Bug 16804 - Searching can be broken by search terms containing semicolons in XSLT
Summary: Searching can be broken by search terms containing semicolons in XSLT
Status: RESOLVED DUPLICATE of bug 14716
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Liz Rea
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-23 16:36 UTC by Kyle M Hall
Modified: 2020-01-11 14:16 UTC (History)
5 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 16804 - unescaped semicolons in XSLT values can cause searches to fail (6.20 KB, patch)
2017-10-05 02:08 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 16804 - semicolons break series search in 490 (3.27 KB, patch)
2018-01-03 02:30 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 16804 - semicolons break series search in 490 (3.34 KB, patch)
2018-06-14 17:38 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 Kyle M Hall 2016-06-23 16:36:54 UTC
As of CGI version 2.64 ( which was a long time again ) Perl's CGI module began supporting the use of the semicolon (;) as a key value pair separator in addition to the ampersand (&) which Koha currently uses.

This causes an issue where is search string is truncated at the semicolon. Not only can this cause issues with incorrect searches/results, it can cause malformed CCL which causes Koha to return no search results!

Test Plan:
1) Create a record with a 490$a series statement where there is a semicolon at the end
2) Enable OPAC Suppression
3) Search for the record and click the "Series:" link
4) You will get no results

This is because the search:
se,phr:"Report to the Mississippi Legislature ;"
is
converted to
se,phr:"Report to the Mississippi Legislature

When the opac suppression is tacked on, we get malformed CCL:
(se,phr="Report to the Mississippi Legislature ) not Suppress=1

This is not an issue if we simply search for se,phr:"Report to the Mississippi Legislature ;" because the search form properly encodes the semicolon.

The solution appears to be to make sure all URL elements are properly url encoded and converts all ";" to "%3B".

I do notice that in koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slimUtils-MODS31.xsl we have a template url-encode that may prove useful.
Comment 1 Liz Rea 2017-07-04 01:23:44 UTC
Hi Kyle,

Using the MODS url-encode works when you transfer it over to slimUtils, is that what you had in mind or something else?

Seems like it sorts the problem nicely.

Cheers,
Liz
Comment 2 Liz Rea 2017-10-05 02:08:40 UTC
Created attachment 67622 [details] [review]
Bug 16804 - unescaped semicolons in XSLT values can cause searches to fail

This patch adds a url-encode to the MARC21slimUtils, and applies it specifically to a common place where semicolons can appear in searches, the 490 field.

To test:

* have OPACSuppression on
* have a record with a 490 field a la:

490 _a something ;
    _v 123

* go to the opac and search for this record
* click the Series link from the detail

Without this patch, there will be no results.

With this patch, you will get relevant results (possibly the same record back)

There are probably lots of places this could be applied, and also possibly many other solutions.
Comment 3 Katrin Fischer 2017-10-05 05:46:43 UTC
Hi Liz, I think we had a similar approach on another bug, but when I tested with Japanese text the links were totally broken. Could you test your patch with some original script in 490?
Comment 4 Katrin Fischer 2017-10-07 23:17:18 UTC
I've used some Japanese signs in 490 for testing: 日本日本日本;;;

The link is broken:
http://localhost:8080/cgi-bin/koha/opac-search.pl?q=se,phr:%22%3F%3F%3F%3F%3F%3F%3B%3B%3B%22
Comment 5 Katrin Fischer 2017-10-07 23:17:40 UTC
I think an easier solution would be to just remove ; for now as this is the most common problem case.
Comment 6 Liz Rea 2018-01-03 02:30:02 UTC
Created attachment 70241 [details] [review]
Bug 16804 - semicolons break series search in 490

To test:

* have OPACSuppression on
* have a record with a 490 field a la:

490 _a something ;
    _v 123

* go to the opac and search for this record
* click the Series link from the detail

Without this patch, there will be no results.

With this patch, you will get relevant results (possibly the same record back)
Comment 7 Liz Rea 2018-01-03 02:31:05 UTC
Hi Katrin,

I put a chopPunctuation on the href attribute, it'll chop of whatever ending punctuation people feel inclined to put there. What do you think?

Cheers,
Liz
Comment 8 Liz Rea 2018-01-03 02:47:57 UTC
well not *any*, specifically these:


.:,;/ '
Comment 9 Kyle M Hall 2018-06-14 17:38:09 UTC
Created attachment 76064 [details] [review]
Bug 16804 - semicolons break series search in 490

To test:

* have OPACSuppression on
* have a record with a 490 field a la:

490 _a something ;
    _v 123

* go to the opac and search for this record
* click the Series link from the detail

Without this patch, there will be no results.

With this patch, you will get relevant results (possibly the same record back)

Signed-off-by: Joy Nelson <joy@bywatersolutions.com>
Comment 10 Katrin Fischer 2018-06-22 06:27:33 UTC
Why "* have OPACSuppression on"?

Would like to test if bug 14716 also resolves the issue - might be the nicer global fix.
Comment 11 Joy Nelson 2018-06-22 14:56:50 UTC
Katrin,
I did a retest to confirm.  This patch will fix if opac suppress if ON or OFF.  

joy

(In reply to Katrin Fischer from comment #10)
> Why "* have OPACSuppression on"?
> 
> Would like to test if bug 14716 also resolves the issue - might be the nicer
> global fix.
Comment 12 Katrin Fischer 2018-06-25 22:05:33 UTC
I feel bug 18304 could be the better solution here. I tested it also fixes the issue. Linking both bugs for now using 'see also'.
Comment 13 Joy Nelson 2018-06-25 22:06:58 UTC
Can you verify the bug number Katrin? 18304 is an email issue.

(In reply to Katrin Fischer from comment #12)
> I feel bug 18304 could be the better solution here. I tested it also fixes
> the issue. Linking both bugs for now using 'see also'.
Comment 14 Katrin Fischer 2018-06-25 22:07:38 UTC
Sorry, too late here: bug 14716
Comment 15 Liz Rea 2018-06-25 22:11:06 UTC
That's funny, my original solution was basically that url-encode one.
Comment 16 Katrin Fischer 2018-06-25 22:16:49 UTC
Not quite, this is using an exslt function that can do more than the self implemented url-encode that was tried before. Especially original script like Hebrew and Chinese. Try adding a subject 日本 (Japan) in 653. It will work with the new solution, but not with the last one.
Comment 17 Marcel de Rooy 2018-07-06 06:38:09 UTC
Moving this to In Discussion. Leaving it in SO is confusing.
We seem to wait for progress on 14716.
Comment 18 Katrin Fischer 2020-01-11 14:16:55 UTC
Marking resolved fixed as the url-encoding with the exslt function now solves this nicely.

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