valkey: accept a plus sign in big number replies and never throw on oversize ones

docs/runtime/redis.mdx

@@ -342,7 +342,7 @@ The client automatically converts Redis responses to JavaScript values:
 - Simple strings are returned as JavaScript strings
 - Null bulk strings and null arrays are returned as `null`
 - Array responses are returned as JavaScript arrays
-- Big number responses (RESP3) are returned as `BigInt`. A payload that is not an integer literal is returned as a string. `getBuffer` returns the payload as a `Buffer`.
+- Big number responses (RESP3) are returned as `BigInt`. A payload that is not an integer literal, or that has too many digits for a `BigInt`, is returned as a string. `getBuffer` returns the payload as a `Buffer`.
 - Error responses throw JavaScript errors with appropriate error codes
 - Boolean responses (RESP3) are returned as JavaScript booleans
 - Map responses (RESP3) are returned as JavaScript objects

packages/bun-types/redis.d.ts

@@ -116,7 +116,7 @@ declare module "bun" {
      * - A simple, bulk or verbatim string becomes a string. Methods that return a Buffer, such as getBuffer, keep the bytes.
      * - An integer becomes a number.
      * - A double becomes a number.
-     * - A big number becomes a bigint. When its payload is not an integer literal it becomes a string.
+     * - A big number becomes a bigint. When its payload is not an integer literal, or has too many digits for a bigint, it becomes a string.
      * - A boolean becomes a boolean.
      * - A null, a null bulk string and a null array become null.
      * - An array becomes an array.

src/jsc/JSValue.rs

@@ -666,23 +666,23 @@ impl JSValue {
     pub fn from_uint64_no_truncate(global: &JSGlobalObject, i: u64) -> JsResult<JSValue> {
         host_fn::from_js_host_call(global, || JSC__JSValue__fromUInt64NoTruncate(global, i))
     }
-    /// A BigInt from a decimal integer literal (optional `-`, then digits).
-    /// Returns `Ok(None)` when `digits` is not such a literal.
-    #[track_caller]
-    pub fn big_int_from_decimal(
-        global: &JSGlobalObject,
-        digits: &[u8],
-    ) -> JsResult<Option<JSValue>> {
-        let unsigned = digits.strip_prefix(b"-").unwrap_or(digits);
-        if unsigned.is_empty() || !unsigned.iter().all(u8::is_ascii_digit) {
-            return Ok(None);
+    /// A BigInt from a decimal integer literal (optional `+` or `-`, then
+    /// digits). Returns `None` when `literal` is not such a literal, or when
+    /// the value does not fit in a BigInt. Never throws.
+    pub fn big_int_from_decimal(global: &JSGlobalObject, literal: &[u8]) -> Option<JSValue> {
+        let (negative, digits) = match literal.split_first() {
+            Some((b'-', digits)) => (true, digits),
+            Some((b'+', digits)) => (false, digits),
+            _ => (false, literal),
+        };
+        if digits.is_empty() || !digits.iter().all(u8::is_ascii_digit) {
+            return None;
         }
-        // Only text StringToBigInt accepts gets here, so empty means it threw (too large).
-        let value = host_fn::from_js_host_call(global, || {
-            // SAFETY: `digits` is a live slice for the duration of the call.
-            unsafe { JSC__JSValue__bigIntFromLatin1(global, digits.as_ptr(), digits.len()) }
-        })?;
-        Ok(Some(value))
+        // SAFETY: `digits` is a live, non-empty slice for the duration of the call.
+        let value = unsafe {
+            JSC__JSValue__bigIntFromDecimalDigits(global, digits.as_ptr(), digits.len(), negative)
+        };
+        (!value.is_empty()).then_some(value)
     }
     /// `JSValue.fromTimevalNoTruncate` — encode a `struct timeval`
     /// as a BigInt (`sec * 1_000_000 + nsec`) without precision loss. May allocate
@@ -2002,10 +2002,11 @@ unsafe extern "C" {
     safe fn JSC__JSValue__dateInstanceFromNumber(global: &JSGlobalObject, n: f64) -> JSValue;
     safe fn JSC__JSValue__fromInt64NoTruncate(global: &JSGlobalObject, i: i64) -> JSValue;
     safe fn JSC__JSValue__fromUInt64NoTruncate(global: &JSGlobalObject, i: u64) -> JSValue;
-    fn JSC__JSValue__bigIntFromLatin1(
+    fn JSC__JSValue__bigIntFromDecimalDigits(
         global: &JSGlobalObject,
-        ptr: *const u8,
+        digits: *const u8,
         len: usize,
+        negative: bool,
     ) -> JSValue;
     safe fn JSC__JSValue__fromTimevalNoTruncate(
         global: &JSGlobalObject,

src/jsc/bindings/bindings.cpp

@@ -4487,11 +4487,15 @@ JSC::EncodedJSValue JSC__JSValue__fromUInt64NoTruncate(JSC::JSGlobalObject* glob
     return JSC::JSValue::encode(JSC::JSBigInt::createFrom(globalObject, val));
 }
 
-// Decimal integer literal (Latin-1) -> BigInt. Returns the empty value when
-// the text is not a valid StringToBigInt input.
-JSC::EncodedJSValue JSC__JSValue__bigIntFromLatin1(JSC::JSGlobalObject* globalObject, const uint8_t* ptr, size_t len)
-{
-    return JSC::JSValue::encode(JSC::JSBigInt::stringToBigInt(globalObject, WTF::StringView(std::span { reinterpret_cast<const char*>(ptr), len })));
+// One or more decimal digits (no sign) -> BigInt. Returns the empty value,
+// without throwing, when the value does not fit in a JSBigInt.
+[[ZIG_EXPORT(nothrow)]] JSC::EncodedJSValue JSC__JSValue__bigIntFromDecimalDigits(JSC::JSGlobalObject* globalObject, const uint8_t* digits, size_t len, bool negative)
+{
+    ASSERT(len > 0);
+    auto sign = negative ? JSC::JSBigInt::ParseIntSign::Signed : JSC::JSBigInt::ParseIntSign::Unsigned;
+    // With no global object to throw into, parseInt reports a too-large value
+    // as the empty value.
+    return JSC::JSValue::encode(JSC::JSBigInt::parseInt(nullptr, JSC::getVM(globalObject), WTF::StringView(std::span { reinterpret_cast<const char*>(digits), len }), 10, JSC::JSBigInt::ErrorParseMode::IgnoreExceptions, sign));
 }
 
 uint64_t JSC__JSValue__toUInt64NoTruncate(JSC::EncodedJSValue val)

src/runtime/valkey_jsc/protocol_jsc.rs

@@ -150,11 +150,12 @@ pub(crate) fn resp_value_to_js_with_options(
 
             Ok(js_obj)
         }
-        // BigInt when the payload is an integer literal; modules and Lua can
-        // put anything after `(`, so other text stays a string.
+        // BigInt when the payload is an integer literal that fits in one;
+        // modules and Lua can put anything after `(`, so other text stays a
+        // string.
         RESPValue::BigNumber(str) => {
             if !options.return_as_buffer
-                && let Some(big) = JSValue::big_int_from_decimal(global, str)?
+                && let Some(big) = JSValue::big_int_from_decimal(global, str)
             {
                 return Ok(big);
             }

test/js/valkey/valkey-incremental-scan.test.ts

@@ -8,7 +8,21 @@ const bulk = (s: string) => `$${Buffer.byteLength(s)}${CRLF}${s}${CRLF}`;
 const HELLO =
   `%3${CRLF}` + bulk("server") + bulk("redis") + bulk("proto") + `:3${CRLF}` + bulk("version") + bulk("7.4.0");
 
-type PerSocket = { buf: Buffer; replied: boolean };
+type PerSocket = { buf: Buffer; replied: boolean; unsent: Buffer | null };
+
+/** Writes `reply`; whatever the socket does not take at once goes out on `drain`. */
+function writeReply(s: Socket<PerSocket>, reply: string) {
+  const bytes = Buffer.from(reply, "latin1");
+  s.data.unsent = s.data.unsent ? Buffer.concat([s.data.unsent, bytes]) : bytes;
+  flushUnsent(s);
+}
+
+function flushUnsent(s: Socket<PerSocket>) {
+  const st = s.data;
+  if (!st.unsent) return;
+  const written = Math.max(0, s.write(st.unsent));
+  st.unsent = written < st.unsent.length ? st.unsent.subarray(written) : null;
+}
 
 /**
  * Mock server: parses the client's RESP command frames
@@ -22,8 +36,9 @@ function createCommandServer(
     port: 0,
     socket: {
       open(s) {
-        s.data = { buf: Buffer.alloc(0), replied: false };
+        s.data = { buf: Buffer.alloc(0), replied: false, unsent: null };
       },
+      drain: flushUnsent,
       error() {},
       close() {},
       data(s, raw) {
@@ -90,16 +105,16 @@ function createReplyServer(
         };
         writeByte(0);
       } else {
-        s.write(reply.slice(0, splitAt));
+        writeReply(s, reply.slice(0, splitAt));
         s.flush();
         if (splitAt < reply.length) {
           // Yield twice so the first write reaches the client's `on_data`
           // before the second is sent.
-          setImmediate(() => setImmediate(() => s.write(reply.slice(splitAt))));
+          setImmediate(() => setImmediate(() => writeReply(s, reply.slice(splitAt))));
         }
       }
     } else {
-      s.write(`+OK${CRLF}`);
+      writeReply(s, `+OK${CRLF}`);
     }
   });
 }
@@ -133,8 +148,10 @@ const FRAMES: [name: string, frame: string, expected: Decoded][] = [
   ],
   ["big number above 2^53", `(9007199254740993${CRLF}`, { value: 9007199254740993n }],
   ["negative big number", `(-42${CRLF}`, { value: -42n }],
+  ["big number with an explicit plus sign", `(+42${CRLF}`, { value: 42n }],
   ["big number above 2^64", `(340282366920938463463374607431768211456${CRLF}`, { value: 2n ** 128n }],
   ["big number with a non-integer payload", `(12abc${CRLF}`, { value: "12abc" }],
+  ["big number with a sign and no digits", `(+${CRLF}`, { value: "+" }],
   [
     "simple error (-ERR)",
     `-ERR unknown command${CRLF}`,
@@ -184,6 +201,19 @@ describe.concurrent("Valkey reply decoding", () => {
     });
   });
 
+  test("big number with more digits than a BigInt can hold resolves as a string", async () => {
+    // JavaScriptCore caps a BigInt at 2^20 bits, about 313,600 decimal digits.
+    // The RESP line limit is far above that.
+    const digits = Buffer.alloc(400_000, "7").toString();
+    const server = createReplyServer(`(${digits}${CRLF}`);
+    await withClient(server, async client => {
+      const value = await client.get("k");
+      expect(typeof value).toBe("string");
+      expect(value).toBe(digits);
+      expect(await client.send("PING", [])).toBe("OK");
+    });
+  });
+
   test.each([
     ["-", `-NOAUTH nope!${CRLF}`],
     ["!", `!12${CRLF}NOAUTH nope!${CRLF}`],