FIX: Protect Redis config from being manipulated

FIX: Protect Redis config from being manipulated

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) #

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.

diff --git a/lib/message_bus/backends/redis.rb b/lib/message_bus/backends/redis.rb
index a4751b0..df54493 100644
--- a/lib/message_bus/backends/redis.rb
+++ b/lib/message_bus/backends/redis.rb
@@ -327,7 +327,7 @@ LUA
       def new_redis_connection
       # redis connection used for publishing messages
diff --git a/spec/lib/message_bus/backend_spec.rb b/spec/lib/message_bus/backend_spec.rb
index ec88e2b..989e5fb 100644
--- a/spec/lib/message_bus/backend_spec.rb
+++ b/spec/lib/message_bus/backend_spec.rb
@@ -406,4 +406,12 @@ describe PUB_SUB_CLASS do { |m| }.must_equal ["12"]
+  it 'should not lose redis config' do
+    test_only :redis
+    redis_config = { connector: Redis::Client::Connector }
+    @bus.instance_variable_set(:@redis_config, redis_config)
+    @bus.send(:new_redis_connection)
+    expect(@bus.instance_variable_get(:@redis_config)[:connector]).must_equal Redis::Client::Connector
+  end

GitHub sha: 983134d8

1 Like

Redis can be quite confusing with all its layers of clients, but it does already dup the config:


Hey Daniel, thank you for review, I know it is confusing, I spent half of a day searching for that bug :slight_smile:

In my opinion, we still need that fix.

So here @options = options.dup after that step we got 2 objects. One is @options (dup of options) and second is original options. Then we are passing original options to client @original_client = @client =

Then later in a client again they are making a copy with parse_options but original options are still there

 def initialize(options = {})
      @options = _parse_options(options)

And they are trying to remove attribute from orignal options not @options

elsif options.include?(:connector) && options[:connector].respond_to?(:new)

Does it make sense?


You’re right. Good catch.


This commit appears in #214 which was merged by @SamSaffron.