Bug 703856
| Summary: | Searching for items with backslashes fails | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Other] RHQ Project | Reporter: | Heiko W. Rupp <hrupp> | ||||||||||
| Component: | Core Server | Assignee: | Nobody <nobody> | ||||||||||
| Status: | ASSIGNED --- | QA Contact: | |||||||||||
| Severity: | medium | Docs Contact: | |||||||||||
| Priority: | medium | ||||||||||||
| Version: | 4.2 | CC: | 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: | |||||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
| Embargoed: | |||||||||||||
| Bug Depends On: | |||||||||||||
| Bug Blocks: | 678340 | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 498291 [details]
ss2: No results
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. Created attachment 533036 [details]
antlr-grammar-fix.diff
Created attachment 533037 [details]
partial-search-expression-with-query-params-fix.diff
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). (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... |
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 '\'