Skip to content

Conversation

@hwware
Copy link
Contributor

@hwware hwware commented May 17, 2021

Before this commit, when user set sentinel-user/pass to empty string using SENTINEL CONFIG SET command, sentinel will consider "" as a vaild sernanme/password and dump into config file, therefore, the following error happens when start server again using same config file:

*** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
Reading the configuration file, at line 24
>>> 'sentinel sentinel-user'
Unrecognized sentinel configuration statement.

In this commit, when user calling set sentinel-user/pass and pass into an empty string, we set sentinel.sentinel_auth_user/pass to NULL to avoid the above rewite issue.

@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label May 18, 2021
src/sentinel.c Outdated
Comment on lines 3164 to 3165
sentinel.sentinel_auth_user = strcmp(val->ptr,"") ?
sdsnew(val->ptr) : NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about using sdslen() == 0 instead of strcmp? and also maybe sdsdup instead of sdsnew?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @oranagra for your suggestion, updated

@oranagra oranagra added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label May 18, 2021
@oranagra oranagra merged commit ae6f586 into redis:unstable May 23, 2021
@oranagra
Copy link
Member

@hwware looks like sentinel tests are consistently failing now.
please take a look.
https://github.com/redis/redis/runs/2651949881?check_suite_focus=true

@hwware
Copy link
Contributor Author

hwware commented May 24, 2021

hi @oranagra , i will provide a quick fix shortly, thank you!

oranagra pushed a commit that referenced this pull request May 24, 2021
fix for recent breakage from #8958, did a mistake when updating the pr.
oranagra pushed a commit that referenced this pull request Jun 1, 2021
oranagra pushed a commit that referenced this pull request Jun 1, 2021
fix for recent breakage from #8958, did a mistake when updating the pr.

(cherry picked from commit be6ce8a)
@oranagra oranagra mentioned this pull request Jun 1, 2021
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Sep 8, 2021
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Sep 8, 2021
fix for recent breakage from redis#8958, did a mistake when updating the pr.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten

Projects

Development

Successfully merging this pull request may close these issues.

2 participants