Bug 703856 - Searching for items with backslashes fails
Searching for items with backslashes fails
Status: ASSIGNED
Product: RHQ Project
Classification: Other
Component: Core Server (Show other bugs)
4.2
Unspecified Unspecified
medium Severity medium (vote)
: ---
: ---
Assigned To: Lukas Krejci
Mike Foley
:
Depends On:
Blocks: jon3
  Show dependency treegraph
 
Reported: 2011-05-11 09:11 EDT by Heiko W. Rupp
Modified: 2015-02-01 18:29 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
ss1: the search autocompleter shows the entry (14.19 KB, image/png)
2011-05-11 09:11 EDT, Heiko W. Rupp
no flags Details
ss2: No results (15.00 KB, image/png)
2011-05-11 09:11 EDT, Heiko W. Rupp
no flags Details
antlr-grammar-fix.diff (2.49 KB, patch)
2011-11-11 06:51 EST, Lukas Krejci
no flags Details | Diff
partial-search-expression-with-query-params-fix.diff (41.65 KB, patch)
2011-11-11 06:52 EST, Lukas Krejci
no flags Details | Diff

  None (edit)
Description Heiko W. Rupp 2011-05-11 09:11:00 EDT
Created attachment 498290 [details]
ss1: the search autocompleter shows the entry

I have an entry with  key c:\fakepath\test.war in inventory 

Searching for it shows it in the list of matching resources (ss1). Then clicking on it , so that the expression ends up in the search bar. Then clicking return shows "no results" (ss2).

Server console shows:

15:07:01,734 ERROR [STDERR] line 1:23 no viable alternative at character '\'
Comment 1 Heiko W. Rupp 2011-05-11 09:11:27 EDT
Created attachment 498291 [details]
ss2: No results
Comment 2 Ian Springer 2011-11-08 15:13:02 EST
The "no viable alternative at character '\'" errors are apparently coming from AntLR, which is used by the search engine impl, so I'm going to reassign this to Lukas, our resident AntLR expert.
Comment 3 Lukas Krejci 2011-11-11 06:51:58 EST
Created attachment 533036 [details]
antlr-grammar-fix.diff
Comment 4 Lukas Krejci 2011-11-11 06:52:42 EST
Created attachment 533037 [details]
partial-search-expression-with-query-params-fix.diff
Comment 5 Lukas Krejci 2011-11-11 06:53:13 EST
The are 2 problems manifesting in this bug:

1) The "no viable alternative" problem actually useful. Because the values are inlined inside the JPQL fragment generated from the search expression, allowing escape characters, semicolons, etc, would allow for SQL injection attacks. But of course by disallowing legal characters in the search strings we limit the usefulness of the search.

2) As already mentioned, we generate the search expression JPQL with the values inlined in them. The grammar is easily modified to allow any characters in the search term but that would allow SQL injection attacks (because values are directly inlined in the search jpql). 

To fix this, we must 1) fix the grammar to allow any characters and 2) use the proper query parameters along with the query text itself. This unfortunately requires refactoring of the code both in the search expression "machinery" but also in the CriteriaQueryGenerator that uses parts of the search expression generation routines.

I'm attaching two patches:

1) antlr-grammar-fix.diff - which modifies the grammar to allow any characters in the search term

2) partial-search-expression-with-query-params-fix.diff - which is a partially finished refactoring to not use just strings when generating the search expression JPQL but use a "SearchFragmentBuilder" class that encapsulates both the query string and the query params (along w/ methods from SearchQueryGenerationUtility that really then belong to this new class).
Comment 6 Charles Crouch 2011-11-11 10:14:01 EST
(9:11:51 AM) lkrejci: ccrouch: my conclusion is annoying but complex to fix.. 
it's not a regression, it's always been like this, so I guess we can push it 
post jon3...

Note You need to log in before you can comment on or make changes to this bug.