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

Set client name in HELLO handshake during connection initialization #3294

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LINKIWI
Copy link
Contributor

@LINKIWI LINKIWI commented Mar 9, 2025

Context

Connection initialization consists of 3 independent operations, executed sequentially:

  1. Attempt a RESP 3 HELLO handshake (ref: https://github.com/redis/go-redis/blob/v9.7.1/redis.go#L313)
  2. Pipeline request to set additional properties on the connection, including authentication, DB index, read-only mode, and client name (ref: https://github.com/redis/go-redis/blob/v9.7.1/redis.go#L326)
  3. Pipeline request for client library information (ref: https://github.com/redis/go-redis/blob/v9.7.1/redis.go#L362)

The HELLO command supports an optional property, SETNAME, that allows the client name to be set on the connection at handshake-time, thus making any subsequent CLIENT SETNAME unnecesary.

https://redis.io/docs/latest/commands/hello/

Problem statement

We are running go-redis against a proprietary RESP protocol-compliant server that correctly recognizes and manipulates client names on connections.

The problem with the sequence of 3 operations above is that the client name is not known to the server until completion of step 2. This means that, the client identity is unknown at completion of step 1, and unknown at the start of step 2.

This has caused RESP handshakes to be flagged as coming from unknown clients.

Proposal

Since the HELLO command supports supplying the client name inline, simply pass the client-specified name in the HELLO request. This should be supported by any RESP3-compliant servers. This patch is a noop if the client name field is empty.

This does not remove CLIENT SETNAME from the subsequent pipeline; this is needed for backwards compatibility for RESP2 servers.

This patch solves the problem statement described above because the client identity on the connection will be known at the time HELLO completes (assuming a RESP3 client). This allows all subsequent commands (namely, steps 2 and 3 above) to be correctly associated with a client identity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant