Attachment 121436 Details for Bug 28326 – Bug 28326: Add escape_query function to deal with reserved characters

[patch] Bug 28326: Add escape_query function to deal with reserved characters

Bug-28326-Add-escapequery-function-to-deal-with-re.patch (text/plain), 4.81 KB, created by Nick Clemens (kidclamp) on 2021-05-26 15:11:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2021-05-26 15:11:50 UTC

Size: 4.81 KB

patch

obsolete

>From 0d32048f9fc930f0486764571ccd6f56be80ddad Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Fri, 14 May 2021 13:30:46 +0000
>Subject: [PATCH] Bug 28326: Add escape_query function to deal with reserved
> characters
>
>Bug 26313 introduced a search for analytics, which is great, but it started
>sending titles to ES. This, unfortunately, caused problems where the title
>contained reserved ES characters.
>
>As these searches should always be literal title searches, we should escape all the ES
>characters before the search.
>
>We should not make this a standard, however, as then it prevents using the characters for advanced ES
>searching, but we still add a function for future reuse
>
>To test:
>1 - Have ES setup and running and swtich SearchEngine syspref to 'Elasticsearch'
>2 - Edit a record and add some reserved ES characters to the title
>    https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html
>    e.g. De [Crasi] Aristophea.
>3 - You get an error on the detail page loading
>4 - Apply patch
>5 - Reload
>6 - The page successfully loads
>7 - prove -v t/db_dependent/Koha/SearchEngine/Search.t
>---
> Koha/SearchEngine/Elasticsearch/Search.pm | 20 ++++++++++++++++++++
> Koha/SearchEngine/Zebra/Search.pm         | 13 +++++++++++++
> catalogue/detail.pl                       |  8 ++++++--
> t/db_dependent/Koha/SearchEngine/Search.t | 19 ++++++++++++++++++-
> 4 files changed, 57 insertions(+), 3 deletions(-)
>
>diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm
>index ff56ad1174..1a16f7b78b 100644
>--- a/Koha/SearchEngine/Elasticsearch/Search.pm
>+++ b/Koha/SearchEngine/Elasticsearch/Search.pm
>@@ -127,6 +127,26 @@ sub count {
>     return $result->{hits}->{total};
> }
> 
>+=head2 escape_query
>+
>+    my $query = $searcher->escape_query($query);
>+
>+This escapes characters reserved by the search engine. This allows searching for fields
>+that contain special characters.
>+
>+https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html
>+
>+=cut
>+
>+sub escape_query {
>+    my ( $self, $query ) = @_;
>+
>+    $query =~ s/(\+|\-|\=|\&\&|\|\||\!|$|$|\{|\}|\[|\]|\^|\"|\~|\*|\?|\:|\\|\/)/\\$1/g;
>+    $query =~ s/(<|>)/ /g;
>+
>+    return $query;
>+}
>+
> =head2 search_compat
> 
>     my ( $error, $results, $facets ) = $search->search_compat(
>diff --git a/Koha/SearchEngine/Zebra/Search.pm b/Koha/SearchEngine/Zebra/Search.pm
>index 850469ec5d..63fdd0a1fe 100644
>--- a/Koha/SearchEngine/Zebra/Search.pm
>+++ b/Koha/SearchEngine/Zebra/Search.pm
>@@ -53,6 +53,19 @@ sub search {
>     }
> }
> 
>+=head2 escape_query
>+
>+This passes the query back unchanged as we have not found issues with
>+Zebra reserved characters
>+
>+=cut
>+
>+sub escape_query {
>+    my ($self,$query) = @_;
>+
>+    return $query;
>+}
>+
> =head2 search_compat
> 
> This passes straight through to C4::Search::getRecords.
>diff --git a/catalogue/detail.pl b/catalogue/detail.pl
>index 8d06f0da51..6848b56007 100755
>--- a/catalogue/detail.pl
>+++ b/catalogue/detail.pl
>@@ -127,8 +127,12 @@ if ( $xslfile ) {
>     my $searcher = Koha::SearchEngine::Search->new(
>         { index => $Koha::SearchEngine::BIBLIOS_INDEX }
>     );
>-    my $cleaned_title = $biblio->title;
>-    $cleaned_title =~ tr|/||;
>+
>+    my $raw_title = $biblio->title;
>+    my $cleaned_title = $searcher->escape_query( $raw_title );
>+
>+    $cleaned_title =~ tr|/||; # Trim trailing slash from title
>+
>     my $query =
>       ( C4::Context->preference('UseControlNumber') and $record->field('001') )
>       ? 'rcn:'. $record->field('001')->data . ' AND (bib-level:a OR bib-level:b)'
>diff --git a/t/db_dependent/Koha/SearchEngine/Search.t b/t/db_dependent/Koha/SearchEngine/Search.t
>index e3780b34eb..c3af799828 100755
>--- a/t/db_dependent/Koha/SearchEngine/Search.t
>+++ b/t/db_dependent/Koha/SearchEngine/Search.t
>@@ -4,7 +4,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 1;
>+use Test::More tests => 2;
> 
> use MARC::Field;
> use MARC::Record;
>@@ -44,6 +44,23 @@ subtest 'Test extract_biblionumber' => sub {
>     is( $bibno, 4567, 'Extracted biblio number for Zebra' );
> };
> 
>+subtest 'Test escape_query' => sub {
>+    plan tests => 2;
>+
>+    my @engines = ('Zebra','Elasticsearch');
>+    my $expected = {
>+        Zebra => q{+ - = && || > < ! ( ) { } [ ] ^ " ~ * ? : \ /},
>+        Elasticsearch => q{\\+ \\- \\= \\&& \\||     \\! \$ \$ \\{ \\} \\[ \\] \\^ \\" \\~ \\* \\? \\: \\\ \\/}
>+    };
>+
>+    my $query = q{+ - = && || > < ! ( ) { } [ ] ^ " ~ * ? : \ /};
>+    for my $engine ( @engines ){
>+        t::lib::Mocks::mock_preference( 'SearchEngine', $engine );
>+        my $searcher = Koha::SearchEngine::Search->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
>+        is( $searcher->escape_query( $query ), $expected->{$engine},"Query is escaped as expected");
>+    }
>+};
>+
> # -- Helper routine
> sub test_record {
>     my $marc = MARC::Record->new;
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 28326: 120971 | 121000 | 121035 | 121436