Handle binary safe string for REQUIREPASS and MASTERAUTH directives#8200
Merged
madolson merged 2 commits intoredis:unstablefrom Dec 17, 2020
QuChen88:fix-masterauth
Merged
Handle binary safe string for REQUIREPASS and MASTERAUTH directives#8200madolson merged 2 commits intoredis:unstablefrom QuChen88:fix-masterauth
madolson merged 2 commits intoredis:unstablefrom
QuChen88:fix-masterauth
Conversation
oranagra
reviewed
Dec 16, 2020
Member
oranagra
left a comment
oranagra
left a comment
There was a problem hiding this comment.
@QuChen88 thank you for this PR.
All the changes seem good to me, except for small improvement request in the test.
p.s. i actually have a big refactoring to sendSynchronousCommand stashed somewhere, which can come instead of the modification you made there, i'll try to push it in after this PR is merged.
oranagra
added
the
state:to-be-merged
Contributor
Author
|
@oranagra Thanks. I can look into the TCL test. Do you have an example of the "wait_for" mechanism on a log message in another TCL test? |
Member
|
@QuChen88 Just grep for |
JimB123
reviewed
Dec 16, 2020
Contributor
JimB123
left a comment
JimB123
left a comment
There was a problem hiding this comment.
A few nits. No showstoppers.
Contributor
Author
|
PR is updated with all comments resolved |
madolson
reviewed
Dec 17, 2020
madolson
approved these changes
Dec 17, 2020
madolson
approved these changes
Dec 17, 2020
linxiang-dev
pushed a commit
to linxiang-dev/redis
that referenced
this pull request
Dec 27, 2020
…edis#8200) * Handle binary safe string for REQUIREPASS and MASTERAUTH directives.
oranagra
added
the
release-notes
Merged
JackieXie168
pushed a commit
to JackieXie168/redis
that referenced
this pull request
Mar 2, 2021
…edis#8200) * Handle binary safe string for REQUIREPASS and MASTERAUTH directives.
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
4 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.
Previously neither server.requirepass or server.masterauth handles binary safe SDS strings. This leads to inconsistencies in client experience around password authentication. This CR makes REQUIREPASS and MASTERAUTH directives handle binary safe strings and added generic SDS argument support in config.c.
i.e. On master
After the fix.
Same goes for MASTERAUTH.
See original related PR #7236