Bug 22344 - Limits cannot contain parentheses
Summary: Limits cannot contain parentheses
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Zebra (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-15 01:07 UTC by David Cook
Modified: 2022-04-04 23:26 UTC (History)
0 users

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2019-02-15 01:07:25 UTC
I've noticed an authorised value of category LOC can't have parentheses in it. If it does, the search silently fails... I'm guessing it doesn't even make it from the ZOOM library in Koha over the connection to Zebra. 

I'm going to explore in a moment, but people have noticed this and kind of worked around it with #5693 and #7031.
Comment 1 David Cook 2019-02-15 01:15:29 UTC
WARNING: query problem with  (mc-loc='TEST (TEST TEST)') ZOOM error 10014 "CCL parsing error" (addinfo: "')' expected") from diag-set 'ZOOM'
Comment 2 David Cook 2019-02-15 01:22:21 UTC
When I escape the parentheses with a \ in yaz-client, the query works. I'm not sure if the ZOOM library can handle that kind of escaping yet...
Comment 3 David Cook 2019-02-15 01:26:44 UTC
Replacing the single quotes with double quotes also works, which makes some sense. 

It looks like Reed Wade's original fix used double quotes...

Ian Walls's change also had double quotes...

It looks like Kyle's change with 0da0ab0e471 in #13442 probably broke things... which is interesting as he was trying to fix the use of the ccode limit in facet links...
Comment 4 David Cook 2019-02-15 02:10:59 UTC
So I wrote the following code:

#Negative lookbehind regex: matches only if the preceding character is not a backslash
$v =~ s#(?<![\\])\Q(\E#\Q\(\E#g;
$v =~ s#(?<![\\])\Q)\E#\Q\)\E#g;

And I added it into the following block:

          if ( $k !~ /mc-i(tem)?type/ ) {
                # in case the mc-ccode value has complicating chars like ()'s inside it we wrap in quotes
                $this_limit =~ tr/"//d;
                #Negative lookbehind regex: matches only if the preceding character is not a backslash
                $v =~ s#(?<![\\])\Q(\E#\Q\(\E#g;
                $v =~ s#(?<![\\])\Q)\E#\Q\)\E#g;
                $this_limit = $k.":'".$v."'";
            }

That fixed the search. Zebra returned the correct results. However, it did make the $limit_desc not look nice and it updated the $limit_cgi too, so the escaped value continues to propagate through as one uses facets and pages and so on. Fortunately, the regex prevents double-escapes, so nothing breaks, but it's a little ugly. 

Ugly is probably better than broken in any case.
Comment 5 David Cook 2019-02-20 23:46:28 UTC
Although in hindsight... it's possible to have other "invalid" characters in part of query besides just parentheses. In this case it is an asterisk.

WARNING: query problem with (mc-loc='*test') ZOOM error 10014 "CCL parsing error" (addinfo: "Embedded truncation not supported") from diag-set 'ZOOM'.

It's a shame that ZOOM doesn't have an "escape" method for query building but oh well.

I've noticed that (mc-loc='\*test') will actually remove the * character from the query all together - rather than just treating it as a literal. Or maybe there is something in our Zebra indexing/searching configuration that removes that character. I haven't checked.