LP#1509479: fix infinite loop bug in QueryParser

An unclosed phrase search that has a modifier can cause QueryParser to
enter an infinite loop, tying up open-ils.storage backends.

Examples of such searches include:

  -"cats and dogs
  subject:+"physical chemistry

This patch fixes the bug by allowing the end of the query string
to terminate a phrase (in addition to a quotation mark).

To test:

[1] Verify that the t/21-QueryParser.t unit test passes
[2] To test in a *development* database, before applying the patch
    run one of the example queries.  Observe that no
    results are returned, and that one of the open-ils.storage
    drones is running at 100% CPU.
[3] Apply the patch and restart the open-ils.storage service.
[4] Try the query again; this time, it should return results
    immediately without causing an open-ils.storage drone to
    peg a CPU.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Dan Scott <dscott@laurentian.ca>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm
index 1b5458d..ba90f18 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm
@@ -1159,7 +1159,7 @@ sub decompose {
             $_ = $';
 
             $last_type = 'CLASS';
-        } elsif (/^\s*($$r{required_re}|$$r{disallowed_re}|$$r{negated_re})?"([^"]+)"/) { # phrase, always anded
+        } elsif (/^\s*($$r{required_re}|$$r{disallowed_re}|$$r{negated_re})?"([^"]+)(?:"|$)/) { # phrase, always anded
             warn '  'x$recursing.'Encountered' . ($1 ? " ['$1' modified]" : '') . " phrase: $2\n" if $self->debug;
 
             my $req_ness = $1 || '';

diff --git a/Open-ILS/src/perlmods/t/21-QueryParser.t b/Open-ILS/src/perlmods/t/21-QueryParser.t
index 55ffd6f..81bf051 100644 (file)
--- a/Open-ILS/src/perlmods/t/21-QueryParser.t
+++ b/Open-ILS/src/perlmods/t/21-QueryParser.t
@@ -60,6 +60,18 @@ is($QParser->superpage(), 1, 'Superpage stays set');
 is($QParser->superpage_size(1000), 1000, 'Superpage size setting works');
 is($QParser->superpage_size(), 1000, 'Superpage size stays set');
 
+init_qp();
+eval {
+    local $SIG{ALRM} = sub { die "timed out!\n" };
+    alarm 1;
+    $QParser->parse('-"unclosed phrase');
+};
+if ($@) {
+    fail('parsing modified unclosed phrase query timed out');
+} else {
+    pass('successfully parsed modified unclosed phrase query');
+}
+
 # It's unfortunate not to be able to use the following tests immediately, but
 # they reflect assumptions that need to be updated in light of new qp_fix code.
 # Also,, canonicalization may not preserve insignificant whitespace nor the