Bug 703856

Summary: Searching for items with backslashes fails
Product: [Other] RHQ Project Reporter: Heiko W. Rupp <hrupp>
Component: Core ServerAssignee: Lukas Krejci <lkrejci>
Status: ASSIGNED --- QA Contact: Mike Foley <mfoley>
Severity: medium Docs Contact:
Priority: medium    
Version: 4.2CC: hbrock, hrupp
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
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:
Bug Depends On:    
Bug Blocks: 678340    
Attachments:
Description Flags
ss1: the search autocompleter shows the entry
none
ss2: No results
none
antlr-grammar-fix.diff
none
partial-search-expression-with-query-params-fix.diff none

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...