From 09b5c95b98ec5924feb3f093590b0f169a68bb1e Mon Sep 17 00:00:00 2001 From: nuyb <103496262+wlswo@users.noreply.github.com> Date: Tue, 2 Jun 2026 10:29:56 +0900 Subject: [PATCH] Fix AUTH failure on URIs with empty username in redis-cli (#15255) Partial fix #11085 ## Bug `parseRedisUri()` called `percentDecode(curr, 0)` for URIs of the form `redis://:password@host` and stored the resulting **empty-but-non-NULL** sds into `connInfo->user`. `cliAuth()` only checks `user == NULL` when deciding between legacy `AUTH ` and ACL `AUTH `, so the empty username was sent as `AUTH "" `, which the server rejects with `WRONGPASS`. ## Fix In parseRedisUri(), treat explicitly empty username or password components as NULL rather than empty SDS strings. This allows cliAuth() to fall back to legacy single-argument AUTH when the username is empty, and to skip AUTH entirely when the password is empty. Before replacing URI-parsed credentials, existing connInfo->user and connInfo->auth values are freed to avoid leaks and preserve the expected "later arguments override earlier ones" behavior. As a related cleanup, -a/--pass and --user now also free previously assigned values before reassignment, fixing the same leak pattern when those options are specified multiple times. --- src/cli_common.c | 13 +++-- src/redis-cli.c | 2 + tests/integration/redis-cli.tcl | 93 +++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/src/cli_common.c b/src/cli_common.c index 0c269de9a..6941ee1b3 100644 --- a/src/cli_common.c +++ b/src/cli_common.c @@ -324,11 +324,18 @@ void parseRedisUri(const char *uri, const char* tool_name, cliConnInfo *connInfo /* Extract user info. */ if ((userinfo = strchr(curr,'@'))) { if ((username = strchr(curr, ':')) && username < userinfo) { - connInfo->user = percentDecode(curr, username - curr); + /* Free any value previously set via --user / -a (later + * parameters override earlier ones) and use NULL for an + * explicitly empty component, so cliAuth() falls back to the + * legacy single-argument AUTH (empty username) or skips AUTH + * entirely (empty password) instead of sending an empty ACL + * component, which the server rejects. */ + sdsfree(connInfo->user); + connInfo->user = (username > curr) ? percentDecode(curr, username - curr) : NULL; curr = username + 1; } - - connInfo->auth = percentDecode(curr, userinfo - curr); + sdsfree(connInfo->auth); + connInfo->auth = (userinfo > curr) ? percentDecode(curr, userinfo - curr) : NULL; curr = userinfo + 1; } if (curr == end) return; diff --git a/src/redis-cli.c b/src/redis-cli.c index 75845cbab..9982df3be 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -2735,8 +2735,10 @@ static int parseOptions(int argc, char **argv) { } else if ((!strcmp(argv[i],"-a") || !strcmp(argv[i],"--pass")) && !lastarg) { + sdsfree(config.conn_info.auth); config.conn_info.auth = sdsnew(argv[++i]); } else if (!strcmp(argv[i],"--user") && !lastarg) { + sdsfree(config.conn_info.user); config.conn_info.user = sdsnew(argv[++i]); } else if (!strcmp(argv[i],"-u") && !lastarg) { parseRedisUri(argv[++i],"redis-cli",&config.conn_info,&config.tls); diff --git a/tests/integration/redis-cli.tcl b/tests/integration/redis-cli.tcl index 8a1114a31..714d5452a 100644 --- a/tests/integration/redis-cli.tcl +++ b/tests/integration/redis-cli.tcl @@ -859,4 +859,97 @@ start_server {tags {"cli external:skip"}} { } } +start_server {tags {"cli external:skip"}} { + # Regression for the parseRedisUri() bug where "redis://:password@host" + # produced an empty ACL username and the server replied WRONGPASS. + r config set requirepass "uri-no-username-pass" + set host [srv host] + set port [srv port] + if {$::tls} { + set scheme "rediss" + } else { + set scheme "redis" + } + set tls_args [rediscli_tls_config "tests"] + + test "redis-cli -u with empty username falls back to legacy AUTH" { + set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \ + -u "$scheme://:uri-no-username-pass@$host:$port" PING] + assert_equal "PONG" [exec {*}$cmd] + } + + test "redis-cli -u with explicit username uses ACL AUTH" { + set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \ + -u "$scheme://default:uri-no-username-pass@$host:$port" PING] + assert_equal "PONG" [exec {*}$cmd] + } + + test "redis-cli -u after -a overrides auth without leaking" { + # -a sets connInfo->auth first, then -u must free the previous + # value before assigning the URI-provided one. + set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \ + -a wrong-pass \ + -u "$scheme://default:uri-no-username-pass@$host:$port" PING] + assert_equal "PONG" [exec {*}$cmd] + } + + test "redis-cli -u after --user overrides user without leaking" { + set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \ + --user wronguser \ + -u "$scheme://default:uri-no-username-pass@$host:$port" PING] + assert_equal "PONG" [exec {*}$cmd] + } + + test "redis-cli -u with explicit empty username clears stale --user" { + # A previously set --user must be cleared (not left stale) when + # the URI explicitly empties the username component, otherwise + # cliAuth() would send ACL AUTH with the stale username instead + # of the intended legacy single-argument AUTH. + set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \ + --user wronguser \ + -u "$scheme://:uri-no-username-pass@$host:$port" PING] + assert_equal "PONG" [exec {*}$cmd] + } + + test "redis-cli -u with empty userinfo overrides previously set -a" { + # "redis://@host" has empty userinfo. Since later parameters + # override earlier ones, the URI clears the password supplied via + # -a, so cliAuth() sends no AUTH and the password-protected server + # rejects the command with NOAUTH. + set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \ + -a uri-no-username-pass \ + -u "$scheme://@$host:$port" PING] + catch {exec {*}$cmd 2>@1} e + assert_match "*NOAUTH*" $e + } + + r config set requirepass "" +} + +start_server {tags {"cli external:skip"}} { + # Regression for empty userinfo URIs: "redis://@host" and + # "redis://user:@host" should leave connInfo->auth unset so that + # cliAuth() skips AUTH entirely instead of sending an empty password. + set host [srv host] + set port [srv port] + if {$::tls} { + set scheme "rediss" + } else { + set scheme "redis" + } + set tls_args [rediscli_tls_config "tests"] + + test "redis-cli -u with empty userinfo skips AUTH" { + set cmd [list src/redis-cli {*}$tls_args \ + -u "$scheme://@$host:$port" PING] + assert_equal "PONG" [exec {*}$cmd] + } + + test "redis-cli -u with empty password skips AUTH" { + set cmd [list src/redis-cli {*}$tls_args \ + -u "$scheme://default:@$host:$port" PING] + assert_equal "PONG" [exec {*}$cmd] + } +} + file delete ./.rediscli_history_test