Bug 12758 - Failure when loading or parsing XSLT stylesheets over HTTPS
Summary: Failure when loading or parsing XSLT stylesheets over HTTPS
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on: 12973
Blocks:
  Show dependency treegraph
 
Reported: 2014-08-14 08:13 UTC by Marcel de Rooy
Modified: 2017-10-23 14:52 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12758: Workaround for loading stylesheets over https (2.37 KB, patch)
2014-10-02 12:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12758: Workaround for loading stylesheets over https (2.58 KB, patch)
2014-10-02 12:24 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2014-08-14 08:13:57 UTC

    
Comment 1 Marcel de Rooy 2014-08-14 08:14:35 UTC
See also IRC discussion on August 14, 2014
Comment 2 Marcel de Rooy 2014-08-14 08:18:03 UTC
Just to narrow down the problem:
You can use https:etc.xsl in load_xml, but if you include an import href="https:utils.xsl" in that etc.xsl, it fails to load.
Seems to be a problem of XML::LibXML, but would be nice to resolve it one way or the other..
Comment 3 Marcel de Rooy 2014-08-14 08:29:43 UTC
Have sent an email to the cpan author of LibXML..
Comment 4 Marcel de Rooy 2014-10-02 11:46:25 UTC
I have renamed this report to widen its scope.
Actually, I see here two related problems:

1) LibXML does not like https references when loading xml with the location parameter. This can be resolved by reading the file into a string and passing it to load_xml via the string parameter. (A solution within LibXML for https appears not to be expected for now.)

Note btw bthat reading the file might not be working rightaway. If you LWP::UserAgent to get the file contents, it will need LWP::Protocol::https. This dependency was recently added to Koha as optional in the context of OverDrive integration. Should we mark it as required now?

2) If the stylesheet (via local file, http or https) contains an import with a https reference, LibXML can load it (possibly with the workaround under point 1), but LibXSLT will now fail parsing it with: "I/O warning : failed to load external entity". This is harder to resolve; we should not be replacing imports in xslt code ourselves. The problem seems to go down deeper to the libxml2 C library.
Comment 5 Marcel de Rooy 2014-10-02 12:13:21 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2014-10-02 12:24:03 UTC
Created attachment 31975 [details] [review]
Bug 12758: Workaround for loading stylesheets over https

LibXML does not like https references when loading xml with the location
parameter. This can be resolved by reading the file into a string and
passing it to load_xml via the string parameter.

Note that LWP::UserAgent will only load the file if LWP::Protocol::https
is installed. This is an optional perl dependency in Koha. I mark it as
required now.

Also note that if your stylesheet contains an import with a https
reference, this fix is not enough. The load_xml call will not fail, but
the succeeding call to parse_stylesheet will still fail. As a workaround,
make your import file accessible via http.

NOTE: Some code taken from report 12172. With thanks to Martin Renvoize.

Test plan:
Take for example OPACXSLTDetailsDisplay and replace it by a https ref.
Check opac detail display.
Note that replacing OPACXSLTResultsDisplay by some https ref would also test
repeating the call.
Comment 7 Marcel de Rooy 2014-11-17 08:05:52 UTC
Changing the status of this report to an arbitrary Failed QA.
I do not really like the workaround; it does pollute the code. But it works if you look for a workaround for this problem. A solution at the level of LibXML/LibXSLT would be much better.
The Failed QA status still allows this report to be found when 'google' searching in Bugzilla. Only change the status please, if you have a good reason for doing so. Thanks.