maheshbandal15 commented on code in PR #1156:
URL: https://github.com/apache/ranger/pull/1156#discussion_r3797450298
##########
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceTrie.java:
##########
@@ -679,7 +679,7 @@ private int getMaxDepth() {
}
private Character getLookupChar(char ch) {
- return optIgnoreCase ? Character.toLowerCase(ch) : ch;
+ return optIgnoreCase ?
Character.toLowerCase(Character.toUpperCase(ch)) : ch;
Review Comment:
Thank you all for the review. Thank you @mneethiraj for highlighting the
current test coverage was limited to two-level conversion.
To give some context, the core issue revolves around how certain Unicode
characters are routed and evaluated in the trie during case-insensitive
matching. Using `Character.toLowerCase(Character.toUpperCase(ch))` is the most
robust solution. It perfectly mirrors the matcher’s full two-step case folding
process, resulting in zero BMP mismatches.
Relying solely on toUpperCase(ch) is a narrower fix. While it successfully
closes the originally reported edge cases, it leaves other Unicode ignore-case
pairs potentially mis-routed in the trie, as @vyommani pointed out.
Across the BMP, folding equivalence vs equalsIgnoreCase looks like this:
| Strategy | Mismatched pairs |
|--------|--------|
| Character.toLowerCase(ch) (old trie) | 50 |
| Character.toUpperCase(ch) | 13 |
| Character.toLowerCase(Character.toUpperCase(ch)) | 0 |
So toUpperCase alone still leaves 13 BMP pairs where the matcher says
“equal” but the trie would use different keys.
There are roughly 60 BMP codes. To keep the test class lean and
maintainable, I have opted to include only a representative few that adequately
cover both case-folding scenarios rather than adding all of them to the test.
Request you to review the patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
