redis: send percent-decoded URL credentials byte for byte

RO
2mo ago
+93-47

Problem

  • new RedisClient("redis://jos%E9:pw@host") sends HELLO 3 AUTH default pw: a different ACL user with the same password. redis://user:p%E9ss@host sends AUTH user "". Any userinfo whose percent-decoded bytes are not UTF-8 is silently replaced, with no client-side error.
  • The cause is URL__username / URL__password (src/jsc/bindings/BunString.cpp:690). They call WTF::URL::user() / password(), which percent-decode and then String::fromUTF8() the bytes. That returns a null string for invalid UTF-8. The client then sees an empty username and falls back to default (valkey.rs authenticate()).

Fix

  • URL__username / URL__password now return encodedUser() / encodedPassword(): the userinfo as written, still percent-encoded, the same as JS url.username. The Redis client is their only caller.
  • The client percent-decodes that to raw bytes with a new PercentEncoding::decode_whatwg (src/url/lib.rs) and sends those bytes in HELLO ... AUTH. HELLO and AUTH arguments are bulk strings, so any byte sequence is valid on the wire.
  • A % that is not followed by two hex digits (%zz) stays literal. This is the WHATWG percent-decode rule and the behavior before this change.
  • Verified: test/js/valkey/valkey.test.ts ("sends the percent-decoded credentials ... byte for byte", 3 of 6 rows fail on 1.4.3). Also ran valkey-incremental-scan.test.ts and reliability/connection-failures.test.ts.

Background

  • A Redis URL carries credentials in the userinfo: redis://user:pass@host:port/db. Reserved or non-ASCII bytes are written as %XX. Percent-encoding encodes octets, not characters, so %E9 is the single byte 0xE9 (Latin-1 é), which is not valid UTF-8 on its own.
  • On connect the client sends HELLO 3 AUTH <user> <pass> as a RESP array of bulk strings ($<len>\r\n<bytes>\r\n). Bulk strings are length-prefixed, so they are binary-safe.
  • bun_url::whatwg::URL is a thin Rust handle over WebKit's WTF::URL. Each getter is one extern "C" function in BunString.cpp.
Notes
  • node-redis 5.12.1 and ioredis 5.11.1 throw URIError: URI malformed at construction for %E9, %ff%fe and %zz, because they run decodeURIComponent on the userinfo. Sending the raw bytes was chosen over rejecting: Redis ACL passwords are byte strings, the URL states those bytes without ambiguity, and this adds no new throw path. A Latin-1 é typed by mistake now gets WRONGPASS from the server instead of a silent login as default.
  • ValkeyClient::connection_strings is removed. It was a second copy of username + password + hostname that nothing read (each field is already its own Box<[u8]>), and the block that built it is the one this change rewrites.
  • Wire output of the report's repro after the change:
redis://jos%C3%A9:pw@   -> "HELLO" "3" "AUTH" <hex 6a6f73c3a9> "pw"
redis://jos%E9:pw@      -> "HELLO" "3" "AUTH" <hex 6a6f73e9> "pw"
redis://user:p%E9ss@    -> "HELLO" "3" "AUTH" "user" <hex 70e97373>
redis://user:%ff%fe@    -> "HELLO" "3" "AUTH" "user" <hex fffe>
redis://:%zz@           -> "HELLO" "3" "AUTH" "default" "%zz"