Bug 28484 - Elasticsearch fails to parse query if exclamation point is in 245$a
Summary: Elasticsearch fails to parse query if exclamation point is in 245$a
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Alex Arnaud
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
: 28326 (view as bug list)
Depends on: 28316
Blocks: 26314 11175
  Show dependency treegraph
 
Reported: 2021-05-28 12:55 UTC by Bohdan
Modified: 2022-06-06 20:27 UTC (History)
10 users (show)

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


Attachments
Bug 28484: ES clean title in biblio detail (1.95 KB, patch)
2021-07-22 11:47 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 28484: ES clean title in biblio detail (2.02 KB, patch)
2021-08-08 23:36 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 28484: ES clean title in biblio detail (2.12 KB, patch)
2021-09-01 15:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28484: ES clean title in biblio detail (2.17 KB, patch)
2021-09-28 20:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28484: (QA follow-up) Add fix on OPAC (1.28 KB, patch)
2021-09-28 20:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 28484: (QA follow-up) POD fix (2.58 KB, patch)
2021-09-28 20:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Bohdan 2021-05-28 12:55:57 UTC
We have Elasticsearch 6.8.15 and the following error happens when trying to enter colon or exclamation point into 245$a as the last symbol

[query_shard_exception] Failed to parse query  [(host-item:osteuropa:)], with: {"index_uuid": ....} called from sub Search::Elasticsearch::Role::Client::Direct::__ANON__ at /usr/share/koha/lib/Koha/SearchEngine/Elasticsearch/Search.pm line 96

'Failed to parse query [(host-item:osteuropa:)]'}],'failed_shards' => [{'shard' => 0,'node' => 'node','reason' => {'type' => 'query_shard_exception','caused_by' => {'caused_by' => {'type' => 'parse_exception','reason' => 'Encountered " ":" ": "" at line 1, column 20.


Error does not happen when entering either of these symbols in 245$b. Are these symbols not properly escaped in that subfield?
Comment 1 John Yorio 2021-06-07 20:05:16 UTC
I'm just confirming this error occurs.

Staff saw Internal Server Error when trying to view a bib record. 

Looking in the log files, I saw the "[query_shard_exception] Failed to parse query [(host-item: ...." message as per Bohdan's original post. 

Editing the bib record by directly bringing it up with addbiblio.pl?biblionumber= worked and I removed the colon at the end of the 245a. The record could then be viewed. Adding the colon back caused the error again.
Comment 2 John Yorio 2021-06-08 13:33:26 UTC
(In reply to John Yorio from comment #1)
> I'm just confirming this error occurs.
> 
> Staff saw Internal Server Error when trying to view a bib record. 
> 
> Looking in the log files, I saw the "[query_shard_exception] Failed to parse
> query [(host-item: ...." message as per Bohdan's original post. 
> 
> Editing the bib record by directly bringing it up with
> addbiblio.pl?biblionumber= worked and I removed the colon at the end of the
> 245a. The record could then be viewed. Adding the colon back caused the
> error again.

I did some further testing.

In my testing of a 245$a ending in a colon, if the colon is preceded by a space, the error does not occur. So, something like "My Title :" works fine, but "My Title:" produces the error.

Exclamation points failed regardless.
Comment 3 Alex Arnaud 2021-07-22 11:47:04 UTC
Created attachment 123045 [details] [review]
Bug 28484: ES clean title in biblio detail

Test plan:
 - Create a biblio with title like "osteuropa:" or "osteuropa!"
 - Go the this biblio detail pages (cgi-bin/koha/catalogue/detail.pl)
   => Error
 - Apply bug 28316 and this one
 - test again
Comment 4 Alex Arnaud 2021-07-22 11:54:12 UTC
Bug 28316 add some escapings for specials characters like exclamation mark or colon.
But this works only for searches. In biblio detail, the query is built in the script, so the cleaning doesn't apply.

The proposed patch clean the biblio title before the call to simple_search_compat.
Comment 5 Alex Buckley 2021-08-08 23:36:24 UTC
Created attachment 123607 [details] [review]
Bug 28484: ES clean title in biblio detail

Test plan:
 - Create a biblio with title like "osteuropa:" or "osteuropa!"
 - Go the this biblio detail pages (cgi-bin/koha/catalogue/detail.pl)
   => Error
 - Apply bug 28316 and this one
 - test again

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 6 Alex Buckley 2021-08-08 23:36:56 UTC
Followed test plan, patch worked as described in kohadevbox running Elasticsearch. So signed off.
Comment 7 Martin Renvoize (ashimema) 2021-09-01 15:04:42 UTC
Created attachment 124379 [details] [review]
Bug 28484: ES clean title in biblio detail

Test plan:
 - Create a biblio with title like "osteuropa:" or "osteuropa!"
 - Go the this biblio detail pages (cgi-bin/koha/catalogue/detail.pl)
   => Error
 - Apply bug 28316 and this one
 - test again

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize (ashimema) 2021-09-01 15:05:18 UTC
Trivial patch after the work carried out by bug 28316.. passing QA
Comment 9 Martin Renvoize (ashimema) 2021-09-01 15:07:44 UTC
*** Bug 28326 has been marked as a duplicate of this bug. ***
Comment 10 Jonathan Druart 2021-09-02 07:00:41 UTC
I don't recreate the work on master, can you provide more details please.

I don't really see the point of the new method this patch adds:

sub clean_search_term {
    my ( $self, $term ) = @_;

    return $term;
}

...
Comment 11 Jonathan Druart 2021-09-02 07:21:19 UTC
(In reply to Jonathan Druart from comment #10)
> I don't really see the point of the new method this patch adds:
> 
> sub clean_search_term {
>     my ( $self, $term ) = @_;
> 
>     return $term;
> }
> 
> ...

Never mind, it's for Zebra.

Still cannot recreate the problem however.
Comment 12 Joonas Kylmälä 2021-09-02 07:33:56 UTC
(In reply to Jonathan Druart from comment #11)
> (In reply to Jonathan Druart from comment #10)
> > I don't really see the point of the new method this patch adds:
> > 
> > sub clean_search_term {
> >     my ( $self, $term ) = @_;
> > 
> >     return $term;
> > }
> > 
> > ...
> 
> Never mind, it's for Zebra.
> 
> Still cannot recreate the problem however.

hmm, seems like the dependency here, bug 28316, does the cleaning already so this seems to just call the cleaning function another time.
Comment 13 Jonathan Druart 2021-09-02 07:53:41 UTC
I don't recreate *on master*:

 - Create a biblio with title like "osteuropa:" or "osteuropa!"
 - Go the this biblio detail pages (cgi-bin/koha/catalogue/detail.pl)
   => Error

There is no error for me.
Comment 14 Jonathan Druart 2021-09-09 09:29:42 UTC
Bug 28316 is waiting for SO, can we get your attention there please?
Comment 15 Martin Renvoize (ashimema) 2021-09-24 10:07:53 UTC
Switching back to PQA as the blocker has been pushed.
Comment 16 Jonathan Druart 2021-09-28 09:05:04 UTC
(In reply to Jonathan Druart from comment #13)
> I don't recreate *on master*:
> 
>  - Create a biblio with title like "osteuropa:" or "osteuropa!"
>  - Go the this biblio detail pages (cgi-bin/koha/catalogue/detail.pl)
>    => Error
> 
> There is no error for me.

For the record, the error appears on the *OPAC* detail page.
Comment 17 Jonathan Druart 2021-09-28 09:07:02 UTC
And I am still getting the error after applying this patch:

Unable to understand your search query, please rephrase and try again.
 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 100

In logs:
    ','type' => 'parse_exception'},'reason' => 'Failed to parse query [(host-item:(osteuropa!))]','index_uuid' => '-iKfKTkGSwCClT_0bcKLdQ','type' => 'query_shard_exception'},'node' => 'PCWsz1DTR6ybJJ3d0wR2YA'}],'t
ype' => 'search_phase_execution_exception','reason' => 'all shards failed'}}}
Comment 18 Jonathan Druart 2021-09-28 09:07:42 UTC
There is also a QA script failure

 FAIL   Koha/SearchEngine/Zebra/QueryBuilder.pm                                                           
   FAIL   pod                                                                                                             *** WARNING: =head2 without preceding higher level  in file Koha/SearchEngine/Zebra/QueryBuilder.pm
Comment 19 Nick Clemens (kidclamp) 2021-09-28 20:36:31 UTC
Created attachment 125433 [details] [review]
Bug 28484: ES clean title in biblio detail

Test plan:
 - Create a biblio with title like "osteuropa:" or "osteuropa!"
 - Go the this biblio detail pages (cgi-bin/koha/catalogue/detail.pl)
   => Error
 - Apply bug 28316 and this one
 - test again

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Nick Clemens (kidclamp) 2021-09-28 20:36:35 UTC
Created attachment 125434 [details] [review]
Bug 28484: (QA follow-up) Add fix on OPAC

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Nick Clemens (kidclamp) 2021-09-28 20:36:39 UTC
Created attachment 125435 [details] [review]
Bug 28484: (QA follow-up) POD fix

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Jonathan Druart 2021-09-29 10:44:17 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 23 Kyle M Hall (khall) 2021-10-08 13:26:33 UTC
Pushed to 21.05.x for 21.05.05