split raw userinfo before decoding redis URI credentials - #4617
Open
insaf021 wants to merge 2 commits into
Open
Conversation
Collaborator
|
Thanks for the fix — we'll take a closer look shortly. |
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
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.
getUser/getPasswordreaduri.getUserInfo(), which percent-decodes the userinfo before theuser:passwordsplit. A%3Ain the username decodes to a literal:first, sosplit(":", 2)lands on it and the credentials are cut at the wrong point, sending the wrong ACL identity toAUTH.Splitting
uri.getRawUserInfo()keeps the encoded colon as%3A, so only the real separator is a literal:, and each half is decoded after the split.+is kept literal since userinfo is not form data, so passwords already containing+are unaffected.Note
Medium Risk
Touches authentication credential parsing used by all URI-based Jedis connections; behavior changes for URIs with percent-encoded userinfo but fixes incorrect AUTH for those cases.
Overview
Fixes wrong username/password when Redis connection URIs use percent-encoded characters in the userinfo segment (e.g. a colon in the ACL username as
us%3Aer).getUserandgetPasswordnow readgetRawUserInfo(), split on the first literal:, then percent-decode each half via a newdecode()helper. That avoidsgetUserInfo()decoding%3Ainto:before the split, which previously mis-parsed credentials and could send the wrong identity to AUTH. Decoding also treats+as literal in passwords (not form-style space decoding).Adds unit tests for encoded colons in usernames and encoded specials in passwords.
Reviewed by Cursor Bugbot for commit 81845e0. Bugbot is set up for automated code reviews on this repo. Configure here.