Bun.Cookie: stop toString() from percent-encoding a parsed value a second time

docs/runtime/cookies.mdx

@@ -324,6 +324,17 @@ console.log(cookie.toString());
 // => "session=abc123; Domain=example.com; Path=/admin; Expires=Wed, 19 Mar 2025 15:03:26 GMT; Secure; HttpOnly; SameSite=Strict"
 ```
 
+`toString()` percent-encodes the value the way [`encodeURIComponent`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent) does. It does not percent-encode the name, `Domain` or `Path`.
+
+```ts title="serialize-cookie-value.ts" icon="/icons/typescript.svg"
+const cookie = new Bun.Cookie("greeting", "hello world; 100%");
+
+console.log(cookie.toString());
+// => "greeting=hello%20world%3B%20100%25; Path=/; SameSite=Lax"
+```
+
+There is one exception. A cookie that was made from a cookie string (`Cookie.parse` or `new Bun.Cookie(cookieString)`), and whose `value` was not assigned since, keeps the value text of that string. This applies when the text is a `cookie-value` of [RFC 6265 section 4.1.1](https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.1) in which two hex digits follow every `%`. A `cookie-value` is printable ASCII other than space, `"`, `,`, `;` and `\`, optionally inside one pair of double quotes. `toString()` percent-encodes every other value.
+
 #### `toJSON(): CookieInit`
 
 Converts the cookie to a plain object suitable for JSON serialization.
@@ -365,6 +376,33 @@ console.log(cookie.secure); // true
 console.log(cookie.sameSite); // "lax"
 ```
 
+`toString()` writes the value of a parsed cookie back as it is in the string. The exact rule is in the `toString()` section above. A cookie string is read as it is. A value that is passed as a value is text, and `toString()` percent-encodes it: `new Bun.Cookie(name, value)`, `Cookie.from`, a `CookieInit` object, `cookies.set(name, value)` and an assignment to `cookie.value`.
+
+```ts title="parse-cookie-value.ts" icon="/icons/typescript.svg"
+const cookie = Bun.Cookie.parse("session=s%3Aabc.def%2Fghi; Path=/; HttpOnly");
+
+console.log(cookie.toString()); // "session=s%3Aabc.def%2Fghi; Path=/; HttpOnly; SameSite=Lax"
+
+// To send a parsed cookie again, append its string to the headers of a response
+const headers = new Headers();
+headers.append("Set-Cookie", cookie.toString());
+
+// or set the Cookie on a CookieMap, which keeps one cookie for each name
+const cookies = new Bun.CookieMap();
+cookies.set(cookie);
+console.log(cookies.toSetCookieHeaders()); // ["session=s%3Aabc.def%2Fghi; Path=/; HttpOnly; SameSite=Lax"]
+
+// A cookie string is read as it is
+console.log(Bun.Cookie.parse(cookie.toString()).toString()); // "session=s%3Aabc.def%2Fghi; Path=/; HttpOnly; SameSite=Lax"
+
+// A value that is passed as a value is text, so it is percent-encoded
+console.log(new Bun.Cookie(cookie.name, cookie.value).toString()); // "session=s%253Aabc.def%252Fghi; Path=/; SameSite=Lax"
+```
+
+A parsed cookie always has a `path` (`/` by default) and a `sameSite` (`lax` by default), and Bun drops the attributes that it does not know. So `toString()` can write attributes that differ from the string that was parsed.
+
+`Bun.CookieMap` percent-decodes the values of a `Cookie` request header. So after a client sends this cookie back, `req.cookies.get("session")` is `"s:abc.def/ghi"`. That text can differ from the `value` of the `Cookie` that was set. An escape that is not valid UTF-8 is read as the Unicode replacement character (`\uFFFD`).
+
 #### `Cookie.from(name: string, value: string, options?: CookieInit): Cookie`
 
 Factory method to create a cookie.

packages/bun-types/bun.d.ts

@@ -11135,7 +11135,10 @@ declare module "bun" {
     readonly name: string;
 
     /**
-     * The value of the cookie
+     * The value of the cookie.
+     *
+     * A value that you assign is text, and {@link Cookie.serialize}
+     * percent-encodes it.
      */
     value: string;
 
@@ -11185,7 +11188,16 @@ declare module "bun" {
     isExpired(): boolean;
 
     /**
-     * Serializes the cookie to a string
+     * Serializes the cookie to a string.
+     *
+     * The value is percent-encoded the way `encodeURIComponent` does, with
+     * one exception. A cookie that was made from a cookie string
+     * ({@link Cookie.parse} or `new Bun.Cookie(cookieString)`), and whose
+     * `value` was not assigned since, keeps the value text of that string.
+     * This applies when the text is a `cookie-value` of RFC 6265 section 4.1.1
+     * in which two hex digits follow every `%`. A `cookie-value` is printable
+     * ASCII other than space, `"`, `,`, `;` and `\`, optionally inside one
+     * pair of double quotes.
      *
      * @example
      * ```ts
@@ -11210,7 +11222,26 @@ declare module "bun" {
     toJSON(): CookieInit;
 
     /**
-     * Parses a serialized cookie string into a `Cookie`
+     * Parses a serialized cookie string into a `Cookie`.
+     *
+     * {@link Cookie.serialize} writes the value of a parsed cookie back as it
+     * is in the string (see there for the rule). A cookie string is read as it
+     * is. A value that is passed as a value is text and is percent-encoded:
+     * `new Bun.Cookie(name, value)`, {@link Cookie.from}, a {@link CookieInit}
+     * object, `CookieMap.set(name, value)` and an assignment to `value`.
+     *
+     * To send a parsed cookie again, use
+     * `headers.append("Set-Cookie", cookie.toString())` or
+     * `cookies.set(cookie)`. A `CookieMap` keeps one cookie for each name.
+     *
+     * A parsed cookie always has a `path` (`/` by default) and a `sameSite`
+     * (`lax` by default), and attributes that Bun does not know are dropped.
+     *
+     * @example
+     * ```ts
+     * const cookie = Bun.Cookie.parse("session=s%3Aabc; Path=/");
+     * cookie.toString(); // "session=s%3Aabc; Path=/; SameSite=Lax"
+     * ```
      * @param cookieString - A serialized cookie string, like `"name=value; Path=/"`
      */
     static parse(cookieString: string): Cookie;

src/jsc/bindings/Cookie.cpp

@@ -149,7 +149,10 @@ ExceptionOr<Ref<Cookie>> Cookie::parse(StringView cookieString)
         }
     }
 
-    return Cookie::create(name, value, domain, path, expires, secure, sameSite, httpOnly, maxAge, partitioned);
+    auto cookie = Cookie::create(name, value, domain, path, expires, secure, sameSite, httpOnly, maxAge, partitioned);
+    if (!cookie.hasException())
+        cookie.returnValue()->m_valueIsWireText = true;
+    return cookie;
 }
 
 bool Cookie::isExpired() const
@@ -234,15 +237,34 @@ bool Cookie::isValidCookieDomain(const String& domain)
     return true;
 }
 
+static inline bool isCookieOctet(char16_t c)
+{
+    return c == 0x21 || (c >= 0x23 && c <= 0x2B) || (c >= 0x2D && c <= 0x3A) || (c >= 0x3C && c <= 0x5B) || (c >= 0x5D && c <= 0x7E);
+}
+template<typename CharacterType>
+static bool isEncodedCookieValue(std::span<const CharacterType> value)
+{
+    // RFC 6265 4.1.1: cookie-value = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
+    if (value.size() >= 2 && value.front() == '"' && value.back() == '"')
+        value = value.subspan(1, value.size() - 2);
+    for (size_t i = 0; i < value.size(); i++) {
+        if (!isCookieOctet(value[i])) return false;
+        // CookieMap reads a '%' that does not start an escape back as U+FFFD, so a value with one stays percent-encoded.
+        if (value[i] == '%' && (value.size() - i < 3 || !isASCIIHexDigit(value[i + 1]) || !isASCIIHexDigit(value[i + 2]))) return false;
+    }
+    return true;
+}
+
 void Cookie::appendTo(JSC::VM& vm, StringBuilder& builder) const
 {
     // Name=Value is the basic format
     builder.append(m_name);
     builder.append('=');
-    auto result = encodeURIComponent(vm, m_value, builder);
-    if (result.hasException()) {
-        // m_value contained unpaired surrogate. oops!
-        // fortunately, this never happens because the string has already had invalid surrogate pairs converted to the replacement character
+    if (m_valueIsWireText && (m_value.is8Bit() ? isEncodedCookieValue(m_value.span8()) : isEncodedCookieValue(m_value.span16()))) {
+        builder.append(m_value);
+    } else {
+        // This fails only for an unpaired surrogate, which m_value never has.
+        encodeURIComponent(vm, m_value, builder);
     }
 
     // Add domain if present

src/jsc/bindings/Cookie.h

@@ -62,7 +62,11 @@ class Cookie : public RefCounted<Cookie> {
     const String& name() const { return m_name; }
 
     const String& value() const { return m_value; }
-    void setValue(const String& value) { m_value = value; }
+    void setValue(const String& value)
+    {
+        m_value = value;
+        m_valueIsWireText = false;
+    }
 
     const String& domain() const { return m_domain; }
     ExceptionOr<void> setDomain(const String& domain)
@@ -133,6 +137,8 @@ class Cookie : public RefCounted<Cookie> {
     bool m_httpOnly = false;
     double m_maxAge = std::numeric_limits<double>::quiet_NaN();
     bool m_partitioned = false;
+    // Set by parse(): m_value is the value text of a cookie string, which appendTo() writes back as it is when it is a valid cookie-value.
+    bool m_valueIsWireText = false;
 };
 
 } // namespace WebCore

test/js/bun/cookie/cookie-map.test.ts

@@ -359,6 +359,33 @@ describe("Bun.Cookie and Bun.CookieMap", () => {
       ]
     `);
   });
+
+  test("CookieMap writes a parsed Cookie back with the value text it has", () => {
+    const map = new Bun.CookieMap();
+    const cookie = Bun.Cookie.parse("sid=s%3Aabc.def%2Fghi; Path=/; HttpOnly");
+    map.set(cookie);
+
+    expect(map.get("sid")).toBe("s%3Aabc.def%2Fghi");
+    expect(map.toJSON()).toEqual({ sid: "s%3Aabc.def%2Fghi" });
+    expect(map.toSetCookieHeaders()).toEqual(["sid=s%3Aabc.def%2Fghi; Path=/; HttpOnly; SameSite=Lax"]);
+
+    // The map holds the Cookie itself. A value that is assigned afterwards is text.
+    cookie.value = "x y";
+    expect(map.toSetCookieHeaders()).toEqual(["sid=x%20y; Path=/; HttpOnly; SameSite=Lax"]);
+  });
+
+  test("CookieMap percent-encodes a value that is set as a string", () => {
+    const cookie = Bun.Cookie.parse("sid=s%3Aabc");
+    const byNameAndValue = new Bun.CookieMap();
+    byNameAndValue.set(cookie.name, cookie.value);
+    const byObject = new Bun.CookieMap();
+    byObject.set(cookie.toJSON());
+
+    expect([byNameAndValue.toSetCookieHeaders(), byObject.toSetCookieHeaders()]).toEqual([
+      ["sid=s%253Aabc; Path=/; SameSite=Lax"],
+      ["sid=s%253Aabc; Path=/; SameSite=Lax"],
+    ]);
+  });
 });
 
 describe("Cookie name field is immutable", () => {