Bug 13812 - Facet links broken because double-quotes are not URL encoded
Summary: Facet links broken because double-quotes are not URL encoded
Status: RESOLVED DUPLICATE of bug 21526
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-09 19:55 UTC by Barton Chittenden
Modified: 2022-03-24 16:22 UTC (History)
8 users (show)

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


Attachments
Bug 13812 - Facet links broken because double-quotes are not URL encoded. (2.34 KB, patch)
2015-04-06 12:58 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Barton Chittenden 2015-03-09 19:55:36 UTC
This bug is analogous to Bug 13442, but it affects shelf location rather than collection code:

Limiting a search by shelf location is broken, because it wraps double-quotes around the "value" part of the key-value pair in the URL. This breaks the href= part of the anchor tag.

The anchor tag generated for the shelf location faced is

<a href="/cgi-bin/koha/opac-search.pl?&limit=mc-loc%3A"100100ADFICCF"&limit=mc-loc%3A"100200ADFIC"&limit=mc-loc%3A"100250ADFICGN"&limit=mc-loc%3A"100300ADFICHF"&limit=yr%2Cst-numeric%3D2014-&amp;sort_by=acqdate_dsc&amp;limit=au:Paetro%2C%20Maxine%2C" title="Paetro, Maxine,">Paetro, Maxine,</a>

Once again, double-quotes inside the URL are causing a premature close of the href, which results in a broken link.

I think that we need to take a look for code duplication inside C4::Search::buildQuery -- if there's more than one section of code where we're wrapping quotes around the 'value' part of the query, these should be consolidated -- shouldn't we be using URL encoding for URL reserved characters anyway?
Comment 1 Barton Chittenden 2015-03-18 16:39:09 UTC
(In reply to Barton Chittenden from comment #0)
> This bug is analogous to Bug 13442, but it affects shelf location rather
> than collection code:
> 
> Limiting a search by shelf location is broken, because it wraps
> double-quotes around the "value" part of the key-value pair in the URL. This
> breaks the href= part of the anchor tag.
> 
> The anchor tag generated for the shelf location faced is
> 
> <a
> href="/cgi-bin/koha/opac-search.pl?&limit=mc-loc%3A"100100ADFICCF"&limit=mc-
> loc%3A"100200ADFIC"&limit=mc-loc%3A"100250ADFICGN"&limit=mc-
> loc%3A"100300ADFICHF"&limit=yr%2Cst-numeric%3D2014-&amp;
> sort_by=acqdate_dsc&amp;limit=au:Paetro%2C%20Maxine%2C" title="Paetro,
> Maxine,">Paetro, Maxine,</a>
> 
> Once again, double-quotes inside the URL are causing a premature close of
> the href, which results in a broken link.
> 
> I think that we need to take a look for code duplication inside
> C4::Search::buildQuery -- if there's more than one section of code where
> we're wrapping quotes around the 'value' part of the query, these should be
> consolidated -- shouldn't we be using URL encoding for URL reserved
> characters anyway?

I found another example, this time in facets under Subject:

Starting here:

https://library.roseville.ca.us/cgi-bin/koha/opac-detail.pl?biblionumber=35767&query_desc=kw%2Cwrdl%3A%20Fairies

Click on Subject: Faries ... this leads us here:

https://library.roseville.ca.us/cgi-bin/koha/opac-search.pl?q=su:"Fairies."

All of the facets on this page are broken, because the quotes are passed straight in, without being URL encoded. Here's an example from the HTML of that page:

<span class="facet-label"><a href="/cgi-bin/koha/opac-search.pl?q=ccl=su%3A"Fairies."&amp;sort_by=relevance_asc&amp;limit=su-to:Magic" title="Magic">Magic</a></span>
Comment 2 Kyle M Hall 2015-04-06 12:58:16 UTC
Created attachment 37528 [details] [review]
Bug 13812 - Facet links broken because double-quotes are not URL encoded.

This bug is analogous to Bug 13442, but it affects shelf location rather than collection code:

Limiting a search by shelf location is broken, because it wraps double-quotes around the "value" part of the key-value pair in the URL. This breaks the href= part of the anchor tag.

The anchor tag generated for the shelf location faced is

<a href="/cgi-bin/koha/opac-search.pl?&limit=mc-loc%3A"100100ADFICCF"&limit=mc-loc%3A"100200ADFIC"&limit=mc-loc%3A"100250ADFICGN"&limit=mc-loc%3A"100300ADFICHF"&limit=yr%2Cst-numeric%3D2014-&amp;sort_by=acqdate_dsc&amp;limit=au:Paetro%2C%20Maxine%2C" title="Paetro, Maxine,">Paetro, Maxine,</a>

Once again, double-quotes inside the URL are causing a premature close of the href, which results in a broken link.

http://bugs.koha-community.org/show_bug.cgi?id=13821
Comment 3 Kyle M Hall 2015-04-06 13:02:41 UTC
Patch removes quotes. Another possibility would be to escape or encode the quotes, but I'm not well versed enough with the XSLT to do so.
Comment 4 David Cook 2015-04-06 23:31:05 UTC
(In reply to Kyle M Hall from comment #3)
> Patch removes quotes. Another possibility would be to escape or encode the
> quotes, but I'm not well versed enough with the XSLT to do so.

We could register an extension function with XML::LibXSLT, using XML::LibXSLT::register_function(), that allows us to escape or encode quotes. While I think XSLT 2.0 has a uri encoding function, XSLT 1.0 doesn't, so extension functions have to be used.

I thought about adding this to the XSLT handler, but it's a global option, so we can just set it after "use XML::LibXSLT" to make the extension function available. (I wonder a bit about the repercussions while using Plack, but I don't yet know enough about Plack to really speculate on that one. I suppose it depends on the scope in which XML::LibXSLT is loaded.)
Comment 5 Nick Clemens 2015-04-16 19:33:43 UTC
Is tis still valid on master?  I can't recreate, could use a test plan.
Comment 6 David Cook 2015-04-21 01:41:01 UTC
I can't reproduce this on master either. It looks like the problem caused by:

https://library.roseville.ca.us/cgi-bin/koha/opac-search.pl?q=su:%22Fairies.%22

is specific to either 3.16.4 or a modification that Bywater have made to that Koha instance.

Here's a link that I see in the Roseville site for the query https://library.roseville.ca.us/cgi-bin/koha/opac-search.pl?q=su:%22Fairies.%22:

<a href="/cgi-bin/koha/opac-search.pl?q=ccl=su%3A" fairies."&amp;sort_by="relevance_asc&amp;limit=au:Colfer%2C%20Eoin.&quot;" title="Colfer, Eoin.">Colfer, Eoin.</a>

However, my query for http://192.168.1.61:2112/cgi-bin/koha/catalogue/search.pl?q=su:%22Fairies.%22

creates the following link:

<a href="/cgi-bin/koha/catalogue/search.pl?q=ccl=su%3A%22Fairies.%22&amp;sort_by=relevance_dsc&amp;limit=itype:TEST" title="TEST">test</a>

I suspect that the following line in C4::Search::buildQuery() was added after 3.16.4:

$limit_cgi       .= "&limit=" . uri_escape_utf8($this_limit);

Although it looks like the Roseville site is also doing something with lowercasing and possibly whitespace as well...

---

I suspect that the current patch might cause problems if it's used since it removes the TracingQuotesLeft and TracingQuotesRight, which are { and } with ICU, and only " and " with non-ICU.

In any case, this doesn't appear to be a problem in master.
Comment 7 Katrin Fischer 2017-02-23 13:16:48 UTC
I think we see this happen with single quotes in an author name in facets. The name is: Kum'a N'dumbe. A keyword search for the name works, but when you search au:Kum'a N'dumbe - no results. Same when you click on the facet with this name:


/cgi-bin/koha/catalogue/search.pl?idx=kw&q=kamerun&sort_by=relevance_dsc&limit=au:Kum'a N'dumbe, Alexandre
Comment 8 David Cook 2018-10-30 01:34:42 UTC
(In reply to Katrin Fischer from comment #7)
> I think we see this happen with single quotes in an author name in facets.
> The name is: Kum'a N'dumbe. A keyword search for the name works, but when
> you search au:Kum'a N'dumbe - no results. Same when you click on the facet
> with this name:
> 
> 
> /cgi-bin/koha/catalogue/search.
> pl?idx=kw&q=kamerun&sort_by=relevance_dsc&limit=au:Kum'a N'dumbe, Alexandre

Thanks for that info. I'll try to reproduce this one, and see about writing a quick patch...
Comment 9 David Cook 2018-10-30 01:45:40 UTC
(In reply to Katrin Fischer from comment #7)
> I think we see this happen with single quotes in an author name in facets.
> The name is: Kum'a N'dumbe. A keyword search for the name works, but when
> you search au:Kum'a N'dumbe - no results. Same when you click on the facet
> with this name:
> 
> 
> /cgi-bin/koha/catalogue/search.
> pl?idx=kw&q=kamerun&sort_by=relevance_dsc&limit=au:Kum'a N'dumbe, Alexandre

This is the URL that appears for my facets in master:

/cgi-bin/koha/opac-search.pl?idx=kw&amp;q=test&amp;sort_by=relevance_dsc&amp;limit=au:Kum%27a%20N%27dumbe,%20Alexandre

The URL works although it does look like the ampersands are being erroneously escaped as well...
Comment 10 David Cook 2018-10-30 01:55:54 UTC
Just realized that Katrin's comment is from February 2017.

I see there is a html filter on the OPAC template:

<span class="facet-label"><a href="[% local_url | html %]" title="[% facet.facet_title_value | html %]">[% facet.facet_label_value | html %]</a></span>

I have no idea why we're html escaping the URL...

But it does seem to work...
Comment 11 David Cook 2018-10-30 01:58:51 UTC
Ah Jonathan is all over this at #21526
Comment 12 Nick Clemens 2019-06-06 17:16:31 UTC
Seems to be fixed in master

*** This bug has been marked as a duplicate of bug 21526 ***