fix(api): refactors the SQL LIKE pattern escaping logic to use a centralized utility function, ensuring consistent and secure handling of special characters across all database queries.#97
Open
fix(api): refactors the SQL LIKE pattern escaping logic to use a centralized utility function, ensuring consistent and secure handling of special characters across all database queries.#97
Conversation
…ralized utility function, ensuring consistent and secure handling of special characters across all database queries. Signed-off-by: NeatGuyCoding <15627489+NeatGuyCoding@users.noreply.github.com>
…logic Signed-off-by: NeatGuyCoding <15627489+NeatGuyCoding@users.noreply.github.com>
Greptile SummaryRefactored SQL LIKE pattern escaping to use a centralized Major changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant Service
participant Helper
participant Database
Client->>Controller: Search request with keyword
Controller->>Service: search(keyword)
Service->>Helper: escape_like_pattern(keyword)
Note over Helper: Escape backslash first<br/>Then escape % and _
Helper-->>Service: escaped_keyword
Service->>Service: Build SQL query with<br/>escaped_keyword and<br/>ESCAPE '\\'
Service->>Database: Execute LIKE query
Note over Database: Database interprets<br/>\ as escape char<br/>Treats %, _ as literals
Database-->>Service: Query results
Service-->>Controller: Filtered results
Controller-->>Client: Response
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| SELECT TOP {top_k} id, text, meta | ||
| FROM {self.schema}.{self.table_name} | ||
| WHERE text LIKE ? | ||
| WHERE text LIKE ? ESCAPE '|' |
There was a problem hiding this comment.
logic: Wrong ESCAPE character - should be '\' not '|'
The escape character must match what the utility function uses (backslash). Using '|' means the backslash escapes won't be recognized.
Suggested change
| WHERE text LIKE ? ESCAPE '|' | |
| WHERE text LIKE ? ESCAPE '\\' |
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/core/rag/datasource/vdb/iris/iris_vector.py
Line: 298:298
Comment:
**logic:** Wrong ESCAPE character - should be '\\' not '|'
The escape character must match what the utility function uses (backslash). Using '|' means the backslash escapes won't be recognized.
```suggestion
WHERE text LIKE ? ESCAPE '\\'
```
How can I resolve this? If you propose a fix, please make it concise.| case "not contains": | ||
| filters.append(json_field.notlike(f"%{value}%")) | ||
| escaped_value = escape_like_pattern(str(value)) | ||
| filters.append(json_field.notlike(f"%{escaped_value}%")) |
There was a problem hiding this comment.
logic: Missing escape parameter - inconsistent with other cases
All other LIKE calls include escape="\\", but this notlike() call doesn't.
Suggested change
| filters.append(json_field.notlike(f"%{escaped_value}%")) | |
| filters.append(json_field.notlike(f"%{escaped_value}%", escape="\\")) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/core/rag/retrieval/dataset_retrieval.py
Line: 1207:1207
Comment:
**logic:** Missing `escape` parameter - inconsistent with other cases
All other LIKE calls include `escape="\\"`, but this `notlike()` call doesn't.
```suggestion
filters.append(json_field.notlike(f"%{escaped_value}%", escape="\\"))
```
How can I resolve this? If you propose a fix, please make it concise.| if not pattern: | ||
| return pattern | ||
| # Escape backslash first, then percent and underscore | ||
| escapedPattern = pattern.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") |
There was a problem hiding this comment.
syntax: Variable name violates snake_case convention
Per compliance rule #16, Python variables should use snake_case naming.
Suggested change
| escapedPattern = pattern.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") | |
| escaped_pattern = pattern.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") |
Context Used: Context from dashboard - AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/libs/helper.py
Line: 64:64
Comment:
**syntax:** Variable name violates snake_case convention
Per compliance rule #16, Python variables should use snake_case naming.
```suggestion
escaped_pattern = pattern.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
```
**Context Used:** Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=1028e8c2-7220-42d3-94ac-c7ba65fc7240))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| return pattern | ||
| # Escape backslash first, then percent and underscore | ||
| escapedPattern = pattern.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") | ||
| return escapedPattern |
There was a problem hiding this comment.
syntax: Variable name violates snake_case convention
Per compliance rule #16, Python variables should use snake_case naming.
Suggested change
| return escapedPattern | |
| return escaped_pattern |
Context Used: Context from dashboard - AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/libs/helper.py
Line: 65:65
Comment:
**syntax:** Variable name violates snake_case convention
Per compliance rule #16, Python variables should use snake_case naming.
```suggestion
return escaped_pattern
```
**Context Used:** Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=1028e8c2-7220-42d3-94ac-c7ba65fc7240))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+89
to
+93
| from libs.helper import escape_like_pattern | ||
|
|
||
| # Escape special characters in keyword to prevent SQL injection via LIKE wildcards | ||
| keyword_trimmed = keyword[:30] | ||
| escaped_keyword = escape_like_pattern(keyword_trimmed) |
There was a problem hiding this comment.
style: Removed unicode escape encoding without clear justification
The old code used .encode('unicode_escape').decode('utf-8') to handle unicode characters. Verify this removal doesn't break searches with unicode/non-ASCII characters in workflow inputs/outputs. Was unicode escaping intentionally removed, or should it be preserved for non-ASCII character handling?
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/services/workflow_app_service.py
Line: 89:93
Comment:
**style:** Removed unicode escape encoding without clear justification
The old code used `.encode('unicode_escape').decode('utf-8')` to handle unicode characters. Verify this removal doesn't break searches with unicode/non-ASCII characters in workflow inputs/outputs. Was unicode escaping intentionally removed, or should it be preserved for non-ASCII character handling?
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from qodo-benchmark#431