Fix ArrayIndexOutOfBoundsException in SearchQueryUtils.getFields() - #50409
Fix ArrayIndexOutOfBoundsException in SearchQueryUtils.getFields()#50409Pons3006 wants to merge 9 commits into
Conversation
… trailing backslash - Issue keycloak#50228 Signed-off-by: Ponshankar <mail4ponz@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses uncaught ArrayIndexOutOfBoundsException risks in SearchQueryUtils.getFields() when parsing malformed search query input (notably trailing escape characters), improving robustness of Admin API-style search parsing.
Changes:
- Add bounds checks before accessing
chars[i + 1]while processing escape sequences in both field-name and value parsing loops. - Treat a trailing escape (
\) as a terminator to avoid out-of-bounds access, rather than crashing. - Add a regression test case for a trailing backslash in a field value.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| services/src/main/java/org/keycloak/utils/SearchQueryUtils.java | Adds bounds checks and terminator handling in escape parsing to prevent ArrayIndexOutOfBoundsException. |
| services/src/test/java/org/keycloak/utils/SearchQueryUtilsTest.java | Adds regression coverage for malformed input ending with a trailing escape in a value. |
Signed-off-by: Ponshankar <mail4ponz@gmail.com>
ahus1
left a comment
There was a problem hiding this comment.
Thank you for the PR. As described in the connected issue, this should rather be a strict parser with a descriptive error message.
In general, it is usually a good idea to first agree on a possible solution in the issue (especially if there are multiple options, and the issue is still marked "triage").
…ak#50228 Signed-off-by: Ponshankar <mail4ponz@gmail.com>
Signed-off-by: Ponshankar <mail4ponz@gmail.com>
|
|
||
| assertInvalidQuery("key:val\\"); | ||
| assertInvalidQuery("key\\"); | ||
| assertInvalidQuery("key:\"val\\"); |
There was a problem hiding this comment.
The original issue lists additional malformed inputs beyond trailing backslashes: "key \", key ", "key \"1\"". It would be good to add test cases for these as well to confirm they're handled gracefully (some may already work via the existing loop bounds, but explicit coverage prevents future fixes on this).
|
Thanks for the contribution @Pons3006! Since this has been idle for a while, I went ahead and opened a new PR preserving your commit with the fix. I just added a few extra test cases on top:
Closing in favor of #51543. |
Closes #50228