Bug 22344

Summary: Limits cannot contain parentheses
Product: Koha Reporter: David Cook <dcook>
Component: Searching - ZebraAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact:
Severity: normal    
Priority: P5 - low    
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6553
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5693
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7031
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:

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.