diff --git a/docs/runtime/cookies.mdx b/docs/runtime/cookies.mdx index 6c8618e598a6..cbb640892e38 100644 --- a/docs/runtime/cookies.mdx +++ b/docs/runtime/cookies.mdx @@ -113,6 +113,17 @@ const cookie = new Bun.Cookie("visited", "true"); cookies.set(cookie); ``` +Cookie names are allowed to carry the `__Secure-` and `__Host-` prefixes, which browsers only honor when the cookie meets the prefix's requirements. Bun throws a `TypeError` instead of emitting a cookie every browser would ignore: + +- `__Secure-` requires `secure: true`. +- `__Host-` requires `secure: true`, no `domain`, and a `path` of `"/"`. + +```ts title="prefixed-cookie.ts" icon="/icons/typescript.svg" +cookies.set("__Host-session", "abc123", { secure: true }); + +cookies.set("__Host-session", "abc123"); // TypeError +``` + #### `delete(name: string): void` #### `delete(options: CookieStoreDeleteOptions): void` @@ -131,6 +142,8 @@ cookies.delete({ }); ``` +The expiring cookie has to satisfy the name's prefix requirements too, so `delete()` throws a `TypeError` for the same combinations `set()` rejects. Deleting a `__Host-` cookie with a `domain`, or with a `path` other than `"/"`, would emit a cookie the browser ignores, leaving the original cookie in place. + #### `toJSON(): Record` Converts the cookie map to a serializable format. @@ -379,10 +392,15 @@ const cookie = Bun.Cookie.from("session", "abc123", { ```ts title="types.ts" icon="/icons/typescript.svg" interface CookieInit { + /** + * A name starting with `__Secure-` requires `secure`, and a name starting with `__Host-` + * requires `secure`, no `domain`, and a `path` of `/`. Browsers ignore cookies that use + * one of these prefixes without meeting its requirements. + */ name?: string; value?: string; domain?: string; - /** Defaults to '/'. To allow the browser to set the path, use an empty string. */ + /** Defaults to '/'. Must start with '/'. To allow the browser to set the path, use an empty string. */ path?: string; expires?: number | Date | string; secure?: boolean; diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index 2a58e5f76e4b..45948b8db88a 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -9508,10 +9508,15 @@ declare module "bun" { | [pkg: string, info: Pick]; interface CookieInit { + /** + * A name starting with `__Secure-` requires `secure`, and a name starting with `__Host-` + * requires `secure`, no `domain`, and a `path` of `/`. Browsers ignore cookies that use + * one of these prefixes without meeting its requirements. + */ name?: string; value?: string; domain?: string; - /** Defaults to '/'. To allow the browser to set the path, use an empty string. */ + /** Defaults to '/'. Must start with '/'. To allow the browser to set the path, use an empty string. */ path?: string; expires?: number | Date | string; secure?: boolean; diff --git a/src/jsc/bindings/Cookie.cpp b/src/jsc/bindings/Cookie.cpp index 379aea375f18..3ea094494d00 100644 --- a/src/jsc/bindings/Cookie.cpp +++ b/src/jsc/bindings/Cookie.cpp @@ -34,21 +34,94 @@ Cookie::Cookie(const String& name, const String& value, { } +static ExceptionOr validateCookieAttributes(const String& name, const String& domain, const String& path) +{ + if (!Cookie::isValidCookieName(name)) { + return Exception { TypeError, "Invalid cookie name: contains invalid characters"_s }; + } + if (auto validation = Cookie::validateCookiePath(path); validation.hasException()) { + return validation.releaseException(); + } + if (!Cookie::isValidCookieDomain(domain)) { + return Exception { TypeError, "Invalid cookie domain: contains invalid characters"_s }; + } + return {}; +} + ExceptionOr> Cookie::create(const String& name, const String& value, const String& domain, const String& path, int64_t expires, bool secure, CookieSameSite sameSite, bool httpOnly, double maxAge, bool partitioned) { - if (!isValidCookieName(name)) { - return Exception { TypeError, "Invalid cookie name: contains invalid characters"_s }; + if (auto validation = validateCookieAttributes(name, domain, path); validation.hasException()) { + return validation.releaseException(); } - if (!isValidCookiePath(path)) { - return Exception { TypeError, "Invalid cookie path: contains invalid characters"_s }; + return adoptRef(*new Cookie(name, value, domain, path, expires, secure, sameSite, httpOnly, maxAge, partitioned)); +} + +ExceptionOr> Cookie::create(const CookieInit& init) +{ + if (auto validation = validateCookieAttributes(init.name, init.domain, init.path); validation.hasException()) { + return validation.releaseException(); } - if (!isValidCookieDomain(domain)) { - return Exception { TypeError, "Invalid cookie domain: contains invalid characters"_s }; + if (auto validation = validateNamePrefix(init.name, init.domain, init.path, init.secure); validation.hasException()) { + return validation.releaseException(); } - return adoptRef(*new Cookie(name, value, domain, path, expires, secure, sameSite, httpOnly, maxAge, partitioned)); + return adoptRef(*new Cookie(init.name, init.value, init.domain, init.path, init.expires, init.secure, init.sameSite, init.httpOnly, init.maxAge, init.partitioned)); +} + +// RFC 6265bis 4.1.3: a user agent ignores a cookie whose name carries one of these prefixes +// unless the cookie satisfies the prefix's requirements. Browsers match the prefix +// case-insensitively. +bool Cookie::hasHostPrefix(const String& name) +{ + return name.startsWithIgnoringASCIICase("__Host-"_s); +} + +bool Cookie::hasSecurePrefix(const String& name) +{ + return name.startsWithIgnoringASCIICase("__Secure-"_s); +} + +ExceptionOr Cookie::validatePrefixSecure(const String& name, bool secure) +{ + if (secure) { + return {}; + } + if (hasHostPrefix(name)) { + return Exception { TypeError, "Invalid cookie name: \"__Host-\" prefix requires secure: true"_s }; + } + if (hasSecurePrefix(name)) { + return Exception { TypeError, "Invalid cookie name: \"__Secure-\" prefix requires secure: true"_s }; + } + return {}; +} + +ExceptionOr Cookie::validatePrefixDomain(const String& name, const String& domain) +{ + if (!domain.isEmpty() && hasHostPrefix(name)) { + return Exception { TypeError, "Invalid cookie name: \"__Host-\" prefix does not allow a domain"_s }; + } + return {}; +} + +ExceptionOr Cookie::validatePrefixPath(const String& name, const String& path) +{ + if (path != "/"_s && hasHostPrefix(name)) { + return Exception { TypeError, "Invalid cookie name: \"__Host-\" prefix requires path: \"/\""_s }; + } + return {}; +} + +ExceptionOr Cookie::validateNamePrefix(const String& name, const String& domain, const String& path, bool secure) +{ + if (auto validation = validatePrefixSecure(name, secure); validation.hasException()) { + return validation.releaseException(); + } + if (auto validation = validatePrefixDomain(name, domain); validation.hasException()) { + return validation.releaseException(); + } + return validatePrefixPath(name, path); } String Cookie::serialize(JSC::VM& vm, const std::span> cookies) @@ -230,6 +303,19 @@ bool Cookie::isValidCookiePath(const String& path) return true; } +ExceptionOr Cookie::validateCookiePath(const String& path) +{ + if (!isValidCookiePath(path)) { + return Exception { TypeError, "Invalid cookie path: contains invalid characters"_s }; + } + // RFC 6265 5.2.4: a user agent ignores a Path attribute that does not start with "/" and + // scopes the cookie to the request's directory instead. An empty path omits the attribute. + if (!path.isEmpty() && !path.startsWith('/')) { + return Exception { TypeError, "Invalid cookie path: must start with \"/\""_s }; + } + return {}; +} + static inline bool isValidCharacterInCookieDomain(char16_t c) { return (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-'; diff --git a/src/jsc/bindings/Cookie.h b/src/jsc/bindings/Cookie.h index e7ad07233b4e..cfd7f39ccb2a 100644 --- a/src/jsc/bindings/Cookie.h +++ b/src/jsc/bindings/Cookie.h @@ -42,20 +42,8 @@ class Cookie : public RefCounted { int64_t expires, bool secure, CookieSameSite sameSite, bool httpOnly, double maxAge, bool partitioned); - static ExceptionOr> create(const CookieInit& init) - { - if (!isValidCookieName(init.name)) { - return Exception { TypeError, "Invalid cookie name: contains invalid characters"_s }; - } - if (!isValidCookiePath(init.path)) { - return Exception { TypeError, "Invalid cookie path: contains invalid characters"_s }; - } - if (!isValidCookieDomain(init.domain)) { - return Exception { TypeError, "Invalid cookie domain: contains invalid characters"_s }; - } - - return create(init.name, init.value, init.domain, init.path, init.expires, init.secure, init.sameSite, init.httpOnly, init.maxAge, init.partitioned); - } + // "Set a cookie": additionally enforces the __Secure-/__Host- name prefix rules. + static ExceptionOr> create(const CookieInit& init); static ExceptionOr> parse(StringView cookieString); @@ -72,6 +60,9 @@ class Cookie : public RefCounted { if (!isValidCookieDomain(domain)) { return Exception { TypeError, "Invalid cookie domain: contains invalid characters"_s }; } + if (auto validation = validatePrefixDomain(m_name, domain); validation.hasException()) { + return validation.releaseException(); + } m_domain = domain; return {}; } @@ -79,8 +70,11 @@ class Cookie : public RefCounted { const String& path() const { return m_path; } ExceptionOr setPath(const String& path) { - if (!isValidCookiePath(path)) { - return Exception { TypeError, "Invalid cookie path: contains invalid characters"_s }; + if (auto validation = validateCookiePath(path); validation.hasException()) { + return validation.releaseException(); + } + if (auto validation = validatePrefixPath(m_name, path); validation.hasException()) { + return validation.releaseException(); } m_path = path; return {}; @@ -91,7 +85,14 @@ class Cookie : public RefCounted { bool hasExpiry() const { return m_expires != emptyExpiresAtValue; } bool secure() const { return m_secure; } - void setSecure(bool secure) { m_secure = secure; } + ExceptionOr setSecure(bool secure) + { + if (auto validation = validatePrefixSecure(m_name, secure); validation.hasException()) { + return validation.releaseException(); + } + m_secure = secure; + return {}; + } CookieSameSite sameSite() const { return m_sameSite; } void setSameSite(CookieSameSite sameSite) { m_sameSite = sameSite; } @@ -117,7 +118,19 @@ class Cookie : public RefCounted { static bool isValidCookiePath(const String& path); static bool isValidCookieDomain(const String& domain); + static bool hasHostPrefix(const String& name); + static bool hasSecurePrefix(const String& name); + + static ExceptionOr validateCookiePath(const String& path); + static ExceptionOr validateNamePrefix(const String& name, const String& domain, const String& path, bool secure); + private: + // A cookie's name is immutable, so each attribute the name's prefix constrains is checked + // on its own: at creation, when it is set on a CookieMap, and whenever it is mutated. + static ExceptionOr validatePrefixSecure(const String& name, bool secure); + static ExceptionOr validatePrefixDomain(const String& name, const String& domain); + static ExceptionOr validatePrefixPath(const String& name, const String& path); + Cookie(const String& name, const String& value, const String& domain, const String& path, int64_t expires, bool secure, CookieSameSite sameSite, diff --git a/src/jsc/bindings/CookieMap.cpp b/src/jsc/bindings/CookieMap.cpp index f2396ee543cd..8734b6ca28df 100644 --- a/src/jsc/bindings/CookieMap.cpp +++ b/src/jsc/bindings/CookieMap.cpp @@ -180,29 +180,35 @@ void CookieMap::removeInternal(const String& name) }); } -void CookieMap::set(Ref cookie) +ExceptionOr CookieMap::set(Ref cookie) { + // A Cookie can also reach here straight from Cookie.parse(), which reports what was on + // the wire rather than enforcing the prefix rules, so re-check before it is emitted. + if (auto validation = Cookie::validateNamePrefix(cookie->name(), cookie->domain(), cookie->path(), cookie->secure()); validation.hasException()) { + return validation.releaseException(); + } + removeInternal(cookie->name()); // Add the new cookie m_modifiedCookies.append(WTF::move(cookie)); + return {}; } ExceptionOr CookieMap::remove(const CookieStoreDeleteOptions& options) { - removeInternal(options.name); - String name = options.name; - String domain = options.domain; - String path = options.path; - bool secure = name.startsWithIgnoringASCIICase("__Secure-"_s) || name.startsWithIgnoringASCIICase("__Host-"_s); + // The expiring cookie has to satisfy the prefix rules too, or the user agent ignores it + // and the cookie stays in the browser. + bool secure = Cookie::hasSecurePrefix(name) || Cookie::hasHostPrefix(name); + CookieInit init { name, ""_s, options.domain, options.path, 1, secure, CookieSameSite::Lax, false, std::numeric_limits::quiet_NaN(), false }; - // Add the new cookie - auto cookie_exception = Cookie::create(name, ""_s, domain, path, 1, secure, CookieSameSite::Lax, false, std::numeric_limits::quiet_NaN(), false); + auto cookie_exception = Cookie::create(init); if (cookie_exception.hasException()) { return cookie_exception.releaseException(); } - auto cookie = cookie_exception.releaseReturnValue(); - m_modifiedCookies.append(WTF::move(cookie)); + + removeInternal(name); + m_modifiedCookies.append(cookie_exception.releaseReturnValue()); return {}; } diff --git a/src/jsc/bindings/CookieMap.h b/src/jsc/bindings/CookieMap.h index b664acd01d91..9969df73e512 100644 --- a/src/jsc/bindings/CookieMap.h +++ b/src/jsc/bindings/CookieMap.h @@ -36,7 +36,7 @@ class CookieMap : public RefCounted { bool has(const String& name) const; - void set(Ref); + ExceptionOr set(Ref); Ref clone(); diff --git a/src/jsc/bindings/webcore/JSCookie.cpp b/src/jsc/bindings/webcore/JSCookie.cpp index d8a3041a8507..76e22296bfba 100644 --- a/src/jsc/bindings/webcore/JSCookie.cpp +++ b/src/jsc/bindings/webcore/JSCookie.cpp @@ -776,7 +776,7 @@ JSC_DEFINE_CUSTOM_SETTER(jsCookiePrototypeSetter_secure, (JSGlobalObject * lexic auto& impl = thisObject->wrapped(); auto value = convert(*lexicalGlobalObject, JSValue::decode(encodedValue)); RETURN_IF_EXCEPTION(throwScope, false); - impl.setSecure(value); + WebCore::propagateException(*lexicalGlobalObject, throwScope, impl.setSecure(value)); return true; } diff --git a/src/jsc/bindings/webcore/JSCookieMap.cpp b/src/jsc/bindings/webcore/JSCookieMap.cpp index e6b8ad3b53e9..69a46146f816 100644 --- a/src/jsc/bindings/webcore/JSCookieMap.cpp +++ b/src/jsc/bindings/webcore/JSCookieMap.cpp @@ -379,10 +379,9 @@ static inline JSC::EncodedJSValue jsCookieMapPrototypeFunction_setBody(JSC::JSGl CookieInit cookieInit = {}; // Check if we're setting with a Cookie object directly - if (arg0.isObject() && JSCookie::toWrapped(vm, arg0)) { - auto* cookieImpl = JSCookie::toWrapped(vm, arg0); - if (cookieImpl) - impl.set(Ref(*cookieImpl)); + if (auto* cookieImpl = arg0.isObject() ? JSCookie::toWrapped(vm, arg0) : nullptr) { + WebCore::propagateException(*lexicalGlobalObject, throwScope, impl.set(Ref(*cookieImpl))); + RETURN_IF_EXCEPTION(throwScope, {}); return JSValue::encode(jsUndefined()); } else if (arg0.isObject()) { auto* obj = arg0.getObject(); @@ -418,7 +417,8 @@ static inline JSC::EncodedJSValue jsCookieMapPrototypeFunction_setBody(JSC::JSGl } auto cookie = cookie_exception.releaseReturnValue(); - impl.set(WTF::move(cookie)); + WebCore::propagateException(*lexicalGlobalObject, throwScope, impl.set(WTF::move(cookie))); + RETURN_IF_EXCEPTION(throwScope, {}); return JSValue::encode(jsUndefined()); } diff --git a/test/js/bun/cookie/cookie-map.test.ts b/test/js/bun/cookie/cookie-map.test.ts index ba374b961fd3..81bd6fc156fe 100644 --- a/test/js/bun/cookie/cookie-map.test.ts +++ b/test/js/bun/cookie/cookie-map.test.ts @@ -453,6 +453,89 @@ describe("delete with prefixed cookie names", () => { "id=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; SameSite=Lax", ]); }); + + test("deleting a __Host- cookie with a domain throws and leaves the map untouched", () => { + const map = new Bun.CookieMap("__Host-id=1"); + expect(() => map.delete({ name: "__Host-id", domain: "example.com" })).toThrow( + 'Invalid cookie name: "__Host-" prefix does not allow a domain', + ); + expect(map.get("__Host-id")).toBe("1"); + expect(map.toSetCookieHeaders()).toEqual([]); + }); + + test("deleting a __Host- cookie with a path other than / throws", () => { + const map = new Bun.CookieMap("__Host-id=1"); + expect(() => map.delete("__Host-id", { path: "/admin" })).toThrow( + 'Invalid cookie name: "__Host-" prefix requires path: "/"', + ); + expect(map.get("__Host-id")).toBe("1"); + expect(map.toSetCookieHeaders()).toEqual([]); + }); +}); + +describe("set with prefixed cookie names", () => { + test("setting a __Host- cookie without secure throws", () => { + const map = new Bun.CookieMap(); + expect(() => map.set("__Host-s", "v")).toThrow('Invalid cookie name: "__Host-" prefix requires secure: true'); + expect(map.toSetCookieHeaders()).toEqual([]); + }); + + test("setting a __Secure- cookie without secure throws", () => { + const map = new Bun.CookieMap(); + expect(() => map.set("__Secure-s", "v")).toThrow('Invalid cookie name: "__Secure-" prefix requires secure: true'); + expect(() => map.set({ name: "__Secure-s", value: "v" })).toThrow( + 'Invalid cookie name: "__Secure-" prefix requires secure: true', + ); + expect(map.toSetCookieHeaders()).toEqual([]); + }); + + test("setting a __Host- cookie with a domain throws", () => { + const map = new Bun.CookieMap(); + expect(() => map.set("__Host-s", "v", { secure: true, domain: "example.com" })).toThrow( + 'Invalid cookie name: "__Host-" prefix does not allow a domain', + ); + }); + + test("setting a __Host- cookie with a path other than / throws", () => { + const map = new Bun.CookieMap(); + expect(() => map.set("__Host-s", "v", { secure: true, path: "/admin" })).toThrow( + 'Invalid cookie name: "__Host-" prefix requires path: "/"', + ); + expect(() => map.set("__Host-s", "v", { secure: true, path: "" })).toThrow( + 'Invalid cookie name: "__Host-" prefix requires path: "/"', + ); + }); + + test("a rejected set leaves the previous cookie in place", () => { + const map = new Bun.CookieMap("__Host-s=original"); + expect(() => map.set("__Host-s", "replacement")).toThrow( + 'Invalid cookie name: "__Host-" prefix requires secure: true', + ); + expect(map.get("__Host-s")).toBe("original"); + expect(map.toSetCookieHeaders()).toEqual([]); + }); + + test("setting a valid prefixed cookie works", () => { + const map = new Bun.CookieMap(); + map.set("__Host-s", "v", { secure: true }); + map.set("__Secure-s", "v", { secure: true, domain: "example.com", path: "/admin" }); + expect(map.toSetCookieHeaders()).toEqual([ + "__Host-s=v; Path=/; Secure; SameSite=Lax", + "__Secure-s=v; Domain=example.com; Path=/admin; Secure; SameSite=Lax", + ]); + }); + + test("setting a Cookie object parsed from a header enforces the prefix rules", () => { + const map = new Bun.CookieMap(); + // Cookie.parse reports what was on the wire, so set() is what has to reject it. + expect(() => map.set(Bun.Cookie.parse("__Host-s=v"))).toThrow( + 'Invalid cookie name: "__Host-" prefix requires secure: true', + ); + expect(map.toSetCookieHeaders()).toEqual([]); + + map.set(Bun.Cookie.parse("__Host-s=v; Path=/; Secure")); + expect(map.toSetCookieHeaders()).toEqual(["__Host-s=v; Path=/; Secure; SameSite=Lax"]); + }); }); describe("invalid delete usage", () => { diff --git a/test/js/bun/cookie/cookie.test.ts b/test/js/bun/cookie/cookie.test.ts index d08a7d02f965..36d48e3b111c 100644 --- a/test/js/bun/cookie/cookie.test.ts +++ b/test/js/bun/cookie/cookie.test.ts @@ -474,3 +474,165 @@ describe("cookie name parsing from Cookie header", () => { expect(res.status).toBe(200); }); }); + +// RFC 6265bis 4.1.3: a user agent ignores a cookie whose name carries one of these prefixes +// unless it satisfies the prefix's requirements, so creating one is a mistake worth reporting. +describe("__Secure- and __Host- name prefixes", () => { + test("__Secure- requires secure", () => { + expect(() => new Bun.Cookie("__Secure-a", "1")).toThrow( + 'Invalid cookie name: "__Secure-" prefix requires secure: true', + ); + expect(() => new Bun.Cookie("__Secure-a", "1", { secure: false })).toThrow( + 'Invalid cookie name: "__Secure-" prefix requires secure: true', + ); + expect(new Bun.Cookie("__Secure-a", "1", { secure: true }).toString()).toBe( + "__Secure-a=1; Path=/; Secure; SameSite=Lax", + ); + }); + + test("__Secure- allows a domain and any path", () => { + expect(new Bun.Cookie("__Secure-a", "1", { secure: true, domain: "example.com", path: "/admin" }).toString()).toBe( + "__Secure-a=1; Domain=example.com; Path=/admin; Secure; SameSite=Lax", + ); + }); + + test("__Host- requires secure", () => { + expect(() => new Bun.Cookie("__Host-a", "1")).toThrow( + 'Invalid cookie name: "__Host-" prefix requires secure: true', + ); + expect(new Bun.Cookie("__Host-a", "1", { secure: true }).toString()).toBe( + "__Host-a=1; Path=/; Secure; SameSite=Lax", + ); + }); + + test("__Host- forbids a domain", () => { + expect(() => new Bun.Cookie("__Host-a", "1", { secure: true, domain: "example.com" })).toThrow( + 'Invalid cookie name: "__Host-" prefix does not allow a domain', + ); + }); + + test('__Host- requires a path of "/"', () => { + expect(() => new Bun.Cookie("__Host-a", "1", { secure: true, path: "/admin" })).toThrow( + 'Invalid cookie name: "__Host-" prefix requires path: "/"', + ); + // An empty path omits the attribute entirely, which __Host- does not allow either. + expect(() => new Bun.Cookie("__Host-a", "1", { secure: true, path: "" })).toThrow( + 'Invalid cookie name: "__Host-" prefix requires path: "/"', + ); + }); + + test("the prefix is matched case-insensitively, like browsers do", () => { + expect(() => new Bun.Cookie("__host-a", "1")).toThrow( + 'Invalid cookie name: "__Host-" prefix requires secure: true', + ); + expect(() => new Bun.Cookie("__SECURE-a", "1")).toThrow( + 'Invalid cookie name: "__Secure-" prefix requires secure: true', + ); + }); + + test("a name that only contains the prefix is unaffected", () => { + expect(new Bun.Cookie("x__Host-a", "1").toString()).toBe("x__Host-a=1; Path=/; SameSite=Lax"); + expect(new Bun.Cookie("__Host", "1").toString()).toBe("__Host=1; Path=/; SameSite=Lax"); + }); + + test("the object and Cookie.from forms are checked too", () => { + expect(() => new Bun.Cookie({ name: "__Host-a", value: "1" })).toThrow( + 'Invalid cookie name: "__Host-" prefix requires secure: true', + ); + expect(() => Bun.Cookie.from("__Host-a", "1", { secure: true, domain: "example.com" })).toThrow( + 'Invalid cookie name: "__Host-" prefix does not allow a domain', + ); + }); + + test("Cookie.parse reports what was on the wire without throwing", () => { + const cookie = Bun.Cookie.parse("__Host-a=1"); + expect(cookie.secure).toBe(false); + expect(cookie.toString()).toBe("__Host-a=1; Path=/; SameSite=Lax"); + }); + + test("a cookie in the wire-invalid state from Cookie.parse can be repaired", () => { + const cookie = Bun.Cookie.parse("__Host-a=1"); + cookie.secure = true; + const map = new Bun.CookieMap(); + map.set(cookie); + expect(map.toSetCookieHeaders()).toEqual(["__Host-a=1; Path=/; Secure; SameSite=Lax"]); + }); + + test("the setters cannot mutate a prefixed cookie into a state browsers ignore", () => { + const cookie = new Bun.Cookie("__Host-a", "1", { secure: true }); + expect(() => (cookie.secure = false)).toThrow('Invalid cookie name: "__Host-" prefix requires secure: true'); + expect(() => (cookie.domain = "example.com")).toThrow( + 'Invalid cookie name: "__Host-" prefix does not allow a domain', + ); + expect(() => (cookie.path = "/admin")).toThrow('Invalid cookie name: "__Host-" prefix requires path: "/"'); + expect(cookie.toString()).toBe("__Host-a=1; Path=/; Secure; SameSite=Lax"); + + const secureCookie = new Bun.Cookie("__Secure-a", "1", { secure: true }); + expect(() => (secureCookie.secure = false)).toThrow( + 'Invalid cookie name: "__Secure-" prefix requires secure: true', + ); + // __Secure- constrains nothing but the secure flag. + secureCookie.domain = "example.com"; + secureCookie.path = "/admin"; + expect(secureCookie.toString()).toBe("__Secure-a=1; Domain=example.com; Path=/admin; Secure; SameSite=Lax"); + }); + + test("a cookie already set on a map cannot be mutated into an invalid one", () => { + // CookieMap.set() keeps a reference to the Cookie, so the object stays reachable. + const cookie = new Bun.Cookie("__Host-a", "1", { secure: true }); + const map = new Bun.CookieMap(); + map.set(cookie); + expect(() => (cookie.secure = false)).toThrow('Invalid cookie name: "__Host-" prefix requires secure: true'); + expect(map.toSetCookieHeaders()).toEqual(["__Host-a=1; Path=/; Secure; SameSite=Lax"]); + }); + + test("setters on a cookie without a prefixed name are unaffected", () => { + const cookie = new Bun.Cookie("a", "1", { secure: true }); + cookie.secure = false; + cookie.domain = "example.com"; + cookie.path = "/admin"; + expect(cookie.toString()).toBe("a=1; Domain=example.com; Path=/admin; SameSite=Lax"); + }); + + test("Bun.serve emits a __Host- cookie the browser accepts", async () => { + using server = Bun.serve({ + port: 0, + routes: { + "/": req => { + req.cookies.set("__Host-sid", "s3cret", { secure: true }); + return new Response("ok"); + }, + }, + }); + const res = await fetch(server.url); + expect(res.headers.getSetCookie()).toEqual(["__Host-sid=s3cret; Path=/; Secure; SameSite=Lax"]); + expect(res.status).toBe(200); + }); +}); + +describe("cookie path attribute", () => { + test("a path that does not start with / is rejected", () => { + // RFC 6265 5.2.4: user agents ignore such a Path attribute, and Bun's own parser drops + // it, so the cookie could not even round-trip through Bun.Cookie.parse. + expect(() => new Bun.Cookie("a", "b", { path: "x" })).toThrow('Invalid cookie path: must start with "/"'); + expect(() => new Bun.Cookie("a", "b", { path: "../x" })).toThrow('Invalid cookie path: must start with "/"'); + expect(() => Bun.Cookie.from("a", "b", { path: "x" })).toThrow('Invalid cookie path: must start with "/"'); + expect(() => new Bun.CookieMap().delete("a", { path: "x" })).toThrow('Invalid cookie path: must start with "/"'); + }); + + test("the path setter rejects a relative path", () => { + const cookie = new Bun.Cookie("a", "b", { path: "/x" }); + expect(() => (cookie.path = "x")).toThrow('Invalid cookie path: must start with "/"'); + expect(cookie.path).toBe("/x"); + }); + + test("an empty path omits the attribute", () => { + expect(new Bun.Cookie("a", "b", { path: "" }).toString()).toBe("a=b; SameSite=Lax"); + }); + + test("an absolute path round-trips through Cookie.parse", () => { + const serialized = new Bun.Cookie("a", "b", { path: "/x" }).toString(); + expect(serialized).toBe("a=b; Path=/x; SameSite=Lax"); + expect(Bun.Cookie.parse(serialized).path).toBe("/x"); + }); +}); diff --git a/test/js/bun/util/cookie.test.js b/test/js/bun/util/cookie.test.js index 068b9e8750d4..0a3fc83425a0 100644 --- a/test/js/bun/util/cookie.test.js +++ b/test/js/bun/util/cookie.test.js @@ -574,8 +574,6 @@ describe("cookie.serialize(name, value, options)", function () { "/foo=bar?baz", '/foo"bar"', "/../foo/bar", - "../foo/", - "./", ]; validPaths.forEach(function (path) { @@ -583,6 +581,17 @@ describe("cookie.serialize(name, value, options)", function () { }); }); + // RFC 6265 5.2.4: a user agent ignores a Path attribute that does not start with "/". + it("should throw for a path that does not start with /", function () { + var relativePaths = ["../foo/", "./", "foo"]; + + relativePaths.forEach(function (path) { + expect(cookie.serialize.bind(cookie, "foo", "bar", { path: path })).toThrow( + 'Invalid cookie path: must start with "/"', + ); + }); + }); + it.failing("should throw for invalid value", function () { var invalidPaths = [ "/\n",