extract cluster hash tag on encoded bytes in getSlot(String) - #4686
Closed
insaf021 wants to merge 1 commit into
Closed
extract cluster hash tag on encoded bytes in getSlot(String)#4686insaf021 wants to merge 1 commit into
insaf021 wants to merge 1 commit into
Conversation
Collaborator
|
Before reviewing the PR related issue need to be triaged/accepted by the team |
Collaborator
|
Closing as per the comment is related issue |
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.
getSlot(String)locates the{hash tag}by scanning for the{and}characters and only encodes to bytes afterwards. Thebyte[]overload and the server'skeyHashSlot()scan for0x7B/0x7Din the encoded bytes. Under a DBCSSafeEncoder.DEFAULT_CHARSET(GBK, Shift_JIS, Big5) a trail byte can be0x7B/0x7Dwhile the char is not a brace, so the String path picks a different tag (or none) and routes to a slot the server does not own. It also disagrees with the binary API for the same logical key, which reaches routing throughCommandArguments.getKeyHashSlots().Delegating to the byte overload moves brace scanning onto the wire bytes. No-op under UTF-8 since no multibyte sequence contains
0x7B/0x7D; the added regression covers both the GBK mismatch and the UTF-8 parity.Companion issue #4685.
Note
Medium Risk
Changes core cluster routing for all String keys; fix is targeted and covered by regression tests, but mis-routing before was silent and wrong-slot bugs are operationally serious.
Overview
Fixes wrong Redis Cluster slot routing when
SafeEncoder.DEFAULT_CHARSETis a DBCS encoding (GBK, Shift_JIS, Big5).getSlot(String)no longer finds{/}on the JavaStringviaJedisClusterHashTag; it encodes the key first and delegates togetSlot(byte[]), matching RediskeyHashSlot()and the binary API.Under DBCS, trail bytes can be
0x7B/0x7Dwithout being brace characters, so the old String scan could pick a different hash tag (or none) and send commands to the wrong node. UTF-8 behavior is unchanged.Adds
testHashtagGetSlotMatchesEncodedBytesUnderDbcsCharsetto assertgetSlot(String)equalsgetSlot(encoded bytes)for a GBK key and for UTF-8.Reviewed by Cursor Bugbot for commit 79dd2cf. Bugbot is set up for automated code reviews on this repo. Configure here.