Bug 30865 - Koha::Biblio->get_components_query should double quote Host-item search
Summary: Koha::Biblio->get_components_query should double quote Host-item search
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on: 11175
Blocks: 31106
  Show dependency treegraph
 
Reported: 2022-05-30 01:57 UTC by David Cook
Modified: 2023-06-08 22:26 UTC (History)
6 users (show)

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


Attachments
Bug 30865: Double-quote Host-item in Koha::Biblio->get_components_query (2.22 KB, patch)
2022-05-30 04:10 UTC, David Cook
Details | Diff | Splinter Review
Bug 30865: Double-quote Host-item in Koha::Biblio->get_components_query (2.44 KB, patch)
2022-06-28 06:56 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2022-05-30 01:57:51 UTC
As noted at https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11175#c522 and https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29418#c11, an unquoted Host-item query returned by Koha::Biblio->get_components_query can cause syntax errors in Zebra.
Comment 1 David Cook 2022-05-30 03:38:49 UTC
Reference on Elasticsearch syntax:
https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html#query-string-syntax

Reference on CCL/Zebra syntax:
https://software.indexdata.com/yaz/doc/tools.html#ccl.syntax

The CCL documentation isn't great and Koha has never handled CCL very well...
Comment 2 David Cook 2022-05-30 04:10:08 UTC
Created attachment 135445 [details] [review]
Bug 30865: Double-quote Host-item in Koha::Biblio->get_components_query

This patch adds double quotes around the term qualified by "Host-item"
in Koha::Biblio->get_components_query().

Without them, reserved charactrs like "=" will cause syntax errors like
"CCL parsing error (10014) Unknown qualifier ZOOM for query:
    Host-item=(MyTitle = Mysubtitle)
    at /usr/share/koha/lib/C4/Search.pm line 245."

Test plan:
0) Don't apply the patch
1) Create biblio with title and subtitle like (MyTitle : MySubtitle)
2) Note the warning "There was an error searching for
analytic records, please see the logs for details." on the detail page

3) Apply the patch
4) koha-plack --restart kohadev

5) Refresh the detail page
6) Note that the warning message is gone

7) prove t/db_dependent/Koha/Biblio.t
Comment 3 Martin Renvoize 2022-06-28 06:56:17 UTC
Created attachment 136669 [details] [review]
Bug 30865: Double-quote Host-item in Koha::Biblio->get_components_query

This patch adds double quotes around the term qualified by "Host-item"
in Koha::Biblio->get_components_query().

Without them, reserved charactrs like "=" will cause syntax errors like
"CCL parsing error (10014) Unknown qualifier ZOOM for query:
    Host-item=(MyTitle = Mysubtitle)
    at /usr/share/koha/lib/C4/Search.pm line 245."

Test plan:
0) Don't apply the patch
1) Create biblio with title and subtitle like (MyTitle : MySubtitle)
2) Note the warning "There was an error searching for analytic records,
   please see the logs for details." on the detail page
3) Apply the patch
4) koha-plack --restart kohadev
5) Refresh the detail page
6) Note that the warning message is gone
7) prove t/db_dependent/Koha/Biblio.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2022-06-28 06:57:40 UTC
Thanks for working on this David, my apologies for not getting to testing it as quickly as I should.

This works great, a much cleaner solution too.

Tested with Zebra and Elastic and as the docs say it works well for both.

In the end, this is a trivial patch so I'm going straight for QA.

QA script happy, Unit tests passing, No regressions and bug is fixed.

Passing QA
Comment 5 Tomás Cohen Arazi 2022-07-01 14:15:10 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 6 Tomás Cohen Arazi 2022-07-05 19:06:38 UTC
(In reply to Martin Renvoize from comment #4)
> Thanks for working on this David, my apologies for not getting to testing it
> as quickly as I should.
> 
> This works great, a much cleaner solution too.
> 
> Tested with Zebra and Elastic and as the docs say it works well for both.
> 
> In the end, this is a trivial patch so I'm going straight for QA.
> 
> QA script happy, Unit tests passing, No regressions and bug is fixed.
> 
> Passing QA

Please look at bug 31106 ASAP.
Comment 7 Lucas Gass 2022-07-29 17:24:48 UTC
Backported to 22.05.x for 22.05.04
Comment 8 Arthur Suzuki 2022-08-12 07:02:27 UTC
backport to 21.11.x for 21.11.11
Comment 9 Victor Grousset/tuxayo 2022-08-17 19:14:40 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.