Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-1189

QueryParser does not correctly handle escaped characters within quoted strings

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3, 2.3.1
    • Fix Version/s: 2.4
    • Component/s: core/queryparser
    • Labels:
      None
    • Environment:

      Windows Vista Business (x86 and x64) as well as latest Ubuntu server, both cases under Tomcat 6.0.14.
      This shouldn't matter though.

    • Lucene Fields:
      New, Patch Available

      Description

      The Lucene query parser incorrectly handles escaped characters inside quoted strings; specifically, a quoted string that ends with an (escaped) backslash followed by any additional quoted string will not be properly tokenized. Consider the following example:

      (name:"///mike\\\\\\") or (name:"alphonse")

      This is not a contrived example – it derives from an actual bug we've encountered in our system. Running this query will throw an exception, but removing the second clause resolves the problem. After some digging I've found that the problem is with the way quoted strings are processed by the lexer: you'll notice that Mike's name is followed by three escaped backslashes right before the ending quote; looking at the JavaCC code for the query parser highlights the problem:

      QueryParser.jj
      <DEFAULT> TOKEN : {
        <AND:       ("AND" | "&&") >
      | <OR:        ("OR" | "||") >
      | <NOT:       ("NOT" | "!") >
      | <PLUS:      "+" >
      | <MINUS:     "-" >
      | <LPAREN:    "(" >
      | <RPAREN:    ")" >
      | <COLON:     ":" >
      | <STAR:      "*" >
      | <CARAT:     "^" > : Boost
      | <QUOTED:     "\"" (~["\""] | "\\\"")* "\"">
      ...
      

      Take a look at the way the QUOTED token is constructed – there is no lexical processing of the escaped characters within the quoted string itself. In the above query the lexer matches everything from the first quote through all the backslashes, treating the end quote as an escaped character, thus also matching the starting quote of the second term. This causes a lexer error, because the last quote is then considered the start of a new match.

      I've come to understand that the Lucene query handler is supposed to be able to handle unsanitized human input; indeed the lexer above would handle a query like "blah\" without complaining, but that's a "best-guess" approach that results in bugs with legal, automatically generated queries. I've attached a patch that fixes the erroneous behavior but does not maintain leniency with malformed queries; I believe this is the correct approach because the two design goals are fundamentally at odds. I'd appreciate any comments.

      1. QueryParser.jj.patch
        1 kB
        Tomer Gabel
      2. lucene-1189.patch
        28 kB
        Michael Busch

        Activity

        Hide
        michaelbusch Michael Busch added a comment -

        Committed. Thanks, Tomer!

        Show
        michaelbusch Michael Busch added a comment - Committed. Thanks, Tomer!
        Hide
        michaelbusch Michael Busch added a comment -

        Thanks for your patch, Tomer! Your approach certainly seems
        correct to me.

        The file I'm attaching has your fix to QueryParser.jj and also
        a testcase similar to your example that fails before and passes
        after applying the patch.

        I'm planning to commit this in a day or so.

        Show
        michaelbusch Michael Busch added a comment - Thanks for your patch, Tomer! Your approach certainly seems correct to me. The file I'm attaching has your fix to QueryParser.jj and also a testcase similar to your example that fails before and passes after applying the patch. I'm planning to commit this in a day or so.
        Hide
        holograph Tomer Gabel added a comment -

        The patch to correct the query parser behavior.

        Show
        holograph Tomer Gabel added a comment - The patch to correct the query parser behavior.

          People

          • Assignee:
            michaelbusch Michael Busch
            Reporter:
            holograph Tomer Gabel
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development