Bug 703856 - Searching for items with backslashes fails
Summary: Searching for items with backslashes fails
Keywords:
Status: ASSIGNED
Alias: None
Product: RHQ Project
Classification: Other
Component: Core Server
Version: 4.2
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
: ---
Assignee: Nobody
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: jon3
TreeView+ depends on / blocked
 
Reported: 2011-05-11 13:11 UTC by Heiko W. Rupp
Modified: 2024-03-04 13:35 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:
Embargoed:


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

Description Heiko W. Rupp 2011-05-11 13:11:00 UTC
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 13:11:27 UTC
Created attachment 498291 [details]
ss2: No results

Comment 2 Ian Springer 2011-11-08 20:13:02 UTC
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 11:51:58 UTC
Created attachment 533036 [details]
antlr-grammar-fix.diff

Comment 4 Lukas Krejci 2011-11-11 11:52:42 UTC
Created attachment 533037 [details]
partial-search-expression-with-query-params-fix.diff

Comment 5 Lukas Krejci 2011-11-11 11:53:13 UTC
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 15:14:01 UTC
(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.