Bug 25373

Summary: &nbsp entity usage no longer works in custom XSLTs
Product: Koha Reporter: Didier Gautheron <didier.gautheron>
Component: Architecture, internals, and plumbingAssignee: Bugs List <koha-bugs>
Status: CLOSED WONTFIX QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: dcook, jonathan.druart
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25381
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 23290    
Bug Blocks:    
Attachments: XSLT patch with entity silently discarded
XSLT patch with parser error

Description Didier Gautheron 2020-05-05 08:32:06 UTC
By default display XSL have :

<!DOCTYPE stylesheet [<!ENTITY nbsp "&#160;" >]>

Since Bug 23290 &nbsp; are replaced with nothing, mangling xsl style sheet output.

Worse if nbsp is inside a xslt:text tag some (all?) xml parsers return an error with an empty output.

Error:

xsltParseTemplateContent: xslt:text content problem
compilation error: file /home/koha/webdatas/public/XSLT/OPACResults.xsl element text
xsltParseTemplateContent: xslt:text content problem
 at /home/koha/src/Koha/XSLT_Handler.pm line 343.
Comment 1 Jonathan Druart 2020-05-05 08:50:27 UTC
How do we recreate the problem?
I do see the detail and result page, using the XSLTs.

Default result XSLT file is:

koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetResults.xsl:<!DOCTYPE stylesheet [<!ENTITY nbsp "&#160;" >]>
Comment 2 Didier Gautheron 2020-05-05 09:17:21 UTC
Created attachment 104353 [details]
XSLT patch with entity silently discarded
Comment 3 Didier Gautheron 2020-05-05 09:18:16 UTC
Created attachment 104354 [details]
XSLT patch with parser error
Comment 4 Didier Gautheron 2020-05-05 09:19:43 UTC
Out of the box XSLT are fine, but upgrading Koha on a site with modified ones aren't, and because ENTITY is defined by default it's often used:

Attached patches tested on 2020-05-05 master, Debian strech (it's a koha-testing-docker box) 

patch bad_output.txt:
Online access: ---> Onlineaccess:

no_output.txt:
xsl parser error if notices have other title fields.
Comment 5 David Cook 2020-05-05 23:18:36 UTC
I think that I understand.

Koha's XSLTS define <!ENTITY nbsp "&#160;" >, but the entity &nbsp is *not* actually used by any Koha XSLTs. 

It seems like Didier uses the &nbsp in custom spreadsheets, and that causes the problem.

It seems to me that this is not a Koha issue. Rather, Didier should stop using the &nbsp entity. 

I'll open a report to remove the ENTITY declaration in the XSLTs, since it's unusable (and even problematic as Didier highlights) now.
Comment 6 David Cook 2020-05-05 23:23:23 UTC
Actually, Didier, what OS are you using and which XML parser?

If I recall correctly, the default XML parser in Debian should never have expanded entities even before Bug 23290. I'm guessing you're not using defaults?
Comment 7 Didier Gautheron 2020-05-06 12:17:34 UTC
(In reply to David Cook from comment #6)
> Actually, Didier, what OS are you using and which XML parser?
> 
> If I recall correctly, the default XML parser in Debian should never have
> expanded entities even before Bug 23290. I'm guessing you're not using
> defaults?

As far as I know plain Debian and Ubuntu.

Adding unsafe works:
<koha_xslt_security> 
   <expand_entities_unsafe>1</expand_entities_unsafe> 
<koha_xslt_security>

Or with 18.11 (prior commit Bug 23290)
Comment 8 Jonathan Druart 2020-05-06 12:42:36 UTC
Yes but it's not recommended, then you are vulnerable to what is described in bug 23290. The easiest seems to replace nsbp in your xsl files.
Comment 9 David Cook 2020-05-07 00:39:31 UTC
(In reply to Jonathan Druart from comment #8)
> Yes but it's not recommended, then you are vulnerable to what is described
> in bug 23290. The easiest seems to replace nsbp in your xsl files.

I agree with Jonathan. I'd recommend not expanding entities as you'll create a security vulnerability in your installation. Just use &#160; directly instead of &nbsp;.
Comment 10 David Cook 2020-05-07 00:39:58 UTC
I'd argue that we should mark this as "WON'T FIX".
Comment 11 Didier Gautheron 2020-05-07 08:48:28 UTC
(In reply to David Cook from comment #9)
> (In reply to Jonathan Druart from comment #8)
> > Yes but it's not recommended, then you are vulnerable to what is described
> > in bug 23290. The easiest seems to replace nsbp in your xsl files.
> 
> I agree with Jonathan. I'd recommend not expanding entities as you'll create
> a security vulnerability in your installation. Just use &#160; directly
> instead of &nbsp;.

Hi,
WON'T FIX make sense.

Are you going to remove ENTITY in xls or should I make a report?

Removing  [<!ENTITY nbsp "&#160;" >] and using &#160; will do for us, thanks.

No authority on XSTL files, libraries change them, moreover in french typography you add a space before many punctuation marks, like colon. Cf. (broken!) Koha french manual translation.
https://koha-community.org/manual/18.05/fr/html/systempreferences.html

BTW where do we fix this one? It's an eyesore :)

Regards
Didier
Comment 12 David Cook 2020-05-08 00:09:52 UTC
(In reply to didier from comment #11)
> Hi,
> WON'T FIX make sense.
> 
> Are you going to remove ENTITY in xls or should I make a report?
> 

I've already reported it as Bug 25381. I'm rather busy at the moment, so it would be great if someone else wrote the batch, but if they don't I will eventually.

> Removing  [<!ENTITY nbsp "&#160;" >] and using &#160; will do for us, thanks.
> 

Perfect :)

> No authority on XSTL files, libraries change them, moreover in french
> typography you add a space before many punctuation marks, like colon. Cf.
> (broken!) Koha french manual translation.
> https://koha-community.org/manual/18.05/fr/html/systempreferences.html
> 

Ah, that's interesting. It's been many years since I've had to write in French, so I must have forgotten if I ever knew that heh.

> BTW where do we fix this one? It's an eyesore :)
> 

Honestly, I'm not 100% sure. I think the git repo can be viewed at http://git.koha-community.org/gitweb/?p=kohadocs.git;a=summary. 

It looks like this might be the translation file needed to change: http://git.koha-community.org/gitweb/?p=kohadocs.git;a=blob;f=locales/fr/LC_MESSAGES/systempreferences.po;h=90e7d11b50a945a9757c374adb96663054e1dadf;hb=HEAD

I think you can use that git repo, and then open a "Documentation" issue on this Bugzilla.

I think Caroline Cyr La Rose from inLibro might be the expert on that one these days.