Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 26 additions & 14 deletions lib/redis_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ def initialize(config, **)
super
@middlewares = config.middlewares_stack.new(self)
@raw_connection = nil
@prelude_sent = false
@disable_reconnection = false
@retry_attempt = nil
end
Expand Down Expand Up @@ -745,6 +746,7 @@ def ensure_connected(retryable: true)

if @disable_reconnection
@raw_connection.retry_attempt = nil
send_prelude unless @prelude_sent
if block_given?
yield @raw_connection
else
Expand Down Expand Up @@ -798,13 +800,17 @@ def ensure_connected(retryable: true)
def raw_connection
if @raw_connection.nil? || !@raw_connection.revalidate
connect
elsif !@prelude_sent
send_prelude
end

@raw_connection.retry_attempt = @retry_attempt
@raw_connection
end

def connect
@pid = PIDCache.pid
@prelude_sent = false

if @raw_connection&.revalidate
@middlewares.connect(config) do
Expand All @@ -822,6 +828,24 @@ def connect
end
@raw_connection.retry_attempt = @retry_attempt

send_prelude
rescue FailoverError, CannotConnectError => error
error._set_config(config)
raise error
rescue ConnectionError => error
connect_error = CannotConnectError.with_config(error.message, config)
connect_error.set_backtrace(error.backtrace)
raise connect_error
rescue CommandError => error
if error.message.match?(/ERR unknown command ['`]HELLO['`]/)
raise UnsupportedServer,
"redis-client requires Redis 6+ with HELLO command available (#{config.server_url})"
else
raise
end
end

def send_prelude
prelude = config.connection_prelude.dup

if id
Expand All @@ -842,20 +866,8 @@ def connect
end
end
end
rescue FailoverError, CannotConnectError => error
error._set_config(config)
raise error
rescue ConnectionError => error
connect_error = CannotConnectError.with_config(error.message, config)
connect_error.set_backtrace(error.backtrace)
raise connect_error
rescue CommandError => error
if error.message.match?(/ERR unknown command ['`]HELLO['`]/)
raise UnsupportedServer,
"redis-client requires Redis 6+ with HELLO command available (#{config.server_url})"
else
raise
end
Comment on lines -845 to -858
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why you moved these rescues. Now only the send_prelude call in connect is covered.


@prelude_sent = true
end
end

Expand Down
69 changes: 67 additions & 2 deletions test/shared/redis_client_tests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,12 @@ def test_command_missing
end

def test_authentication
@redis.call("ACL", "SETUSER", "AzureDiamond", ">hunter2", "on", "+PING")
@redis.call("ACL", "DELUSER", "AzureDiamond")
@redis.call("ACL", "SETUSER", "AzureDiamond", ">hunter2", "on", "+PING", "+CLIENT")
@redis.call("ACL", "DELUSER", "backup_admin")
@redis.call("ACL", "SETUSER", "backup_admin", ">hunter2", "on", "~*", "&*", "+@all")
backup = new_client(username: "backup_admin", password: "hunter2")
backup.call("ACL", "SETUSER", "default", "off")

client = new_client(username: "AzureDiamond", password: "hunter2")
assert_equal "PONG", client.call("PING")
Expand All @@ -337,10 +342,70 @@ def test_authentication
client.call("GET", "foo")
end

# Wrong password
client = new_client(username: "AzureDiamond", password: "trolilol")
assert_raises RedisClient::AuthenticationError do
error = assert_raises RedisClient::AuthenticationError do
client.call("PING")
end
assert_match(/WRONGPASS invalid username-password pair/, error.message)

# The same error is raised, this shows that the client retried AUTH and didn't fall back to the default user
error = assert_raises RedisClient::AuthenticationError do
client.call("PING")
end
assert_match(/WRONGPASS invalid username-password pair/, error.message)

# Correct password, but user disabled
backup.call("ACL", "SETUSER", "AzureDiamond", "<hunter2", ">trolilol", "off")
Copy link
Member

Choose a reason for hiding this comment

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

Mutating an existing user, and trying to fix it in a rescue isn't good, as it may fail and leave the server in a state that fail other tests.

You can create a different user.

error = assert_raises RedisClient::AuthenticationError do
client.call_once("PING")
end
assert_match(/WRONGPASS invalid username-password pair/, error.message)

# Correct password, user enabled
backup.call("ACL", "SETUSER", "AzureDiamond", "on")
assert_equal "PONG", client.call_once("PING")
assert_match(/user=AzureDiamond/, client.call("CLIENT", "INFO"))

# Wrong username
client = new_client(username: "GreenOpal", password: "trolilol")
error = assert_raises RedisClient::AuthenticationError do
client.call("PING")
end
assert_match(/WRONGPASS invalid username-password pair/, error.message)
ensure
backup.call("ACL", "SETUSER", "default", "on")
end

def test_prelude_failure
client = new_client(db: 100)
error = assert_raises RedisClient::CommandError do
client.call("PING")
end
assert_match(/ERR DB index is out of range/, error.message)

error = assert_raises RedisClient::CommandError do
client.call("PING")
end
assert_match(/ERR DB index is out of range/, error.message)
end

def test_noauth
@redis.call("ACL", "DELUSER", "AzureDiamond")
@redis.call("ACL", "SETUSER", "AzureDiamond", ">hunter2", "on", "~*", "&*", "+@all")
backup = new_client(username: "AzureDiamond", password: "hunter2")
backup.call("ACL", "SETUSER", "default", "off")

client = new_client(protocol: 2)
error = assert_raises RedisClient::CommandError do
client.call("PING")
end
assert_match(/NOAUTH Authentication required/, error.message)

backup.call("ACL", "SETUSER", "default", "on")
client.call("PING")
ensure
backup.call("ACL", "SETUSER", "default", "on")
end

def test_transaction
Expand Down