Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Protect Redis config from being manipulated #214

Merged
merged 1 commit into from Oct 18, 2019

Conversation

@lis2
Copy link
Contributor

lis2 commented Oct 18, 2019

Redis gem is allowing to pass custom connector as an option. Later, code is removing that option and initialize custom connector:

options.delete(:connector).new(@options) # https://github.com/redis/redis-rb/blob/master/lib/redis/client.rb#L95

It works for MessageBus when Redis server is running. When it fails and MessageBus is trying to reconnect, because it is a passing reference to Redis, customer connector is not available anymore.

Therefore, I think it would be better to pass a copy of the original object.

Redis gem is allowing to pass custom connector as an option. Later, code is removing that option and initialize custom connector:
```
options.delete(:connector).new(@options) # https://github.com/redis/redis-rb/blob/master/lib/redis/client.rb#L95
```

It works for MessageBus when Redis server is running. When it fails and MessageBus is trying to reconnect, because it is a passing reference to Redis, customer connector is not available anymore.

Therefore, I think it would be better to pass a copy of the original object.
@SamSaffron

This comment has been minimized.

Copy link
Member

SamSaffron commented Oct 18, 2019

This looks good to me, odd that redis are fiddling like this with config, seems unsafe.

@SamSaffron SamSaffron merged commit 983134d into discourse:master Oct 18, 2019
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.