diff --git a/docs/api/cookie.md b/docs/api/cookie.md index 08be1d5a49..c0e01142a4 100644 --- a/docs/api/cookie.md +++ b/docs/api/cookie.md @@ -5,8 +5,6 @@ Bun provides native APIs for working with HTTP cookies through `Bun.Cookie` and `Bun.CookieMap` provides a Map-like interface for working with collections of cookies. It implements the `Iterable` interface, allowing you to use it with `for...of` loops and other iteration methods. ```ts -import { CookieMap } from "bun"; - // Empty cookie map const cookies = new Bun.CookieMap(); @@ -55,6 +53,8 @@ const server = Bun.serve({ }, }, }); + +console.log("Server listening at: " + server.url); ``` ### Methods @@ -89,7 +89,7 @@ if (cookies.has("session")) { #### `set(cookie: Cookie): void` -Adds or updates a cookie in the map. +Adds or updates a cookie in the map. Cookies default to `{ path: "/", sameSite: "lax" }`. ```ts // Set by name and value @@ -373,9 +373,11 @@ interface CookieInit { name?: string; value?: string; domain?: string; + /** Defaults to '/'. To allow the browser to set the path, use an empty string. */ path?: string; expires?: number | Date | string; secure?: boolean; + /** Defaults to `lax`. */ sameSite?: CookieSameSite; httpOnly?: boolean; partitioned?: boolean; diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index 3c2c9f1ee5..33e2c96ad5 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -7262,9 +7262,11 @@ declare module "bun" { name?: string; value?: string; domain?: string; + /** Defaults to '/'. To allow the browser to set the path, use an empty string. */ path?: string; expires?: number | Date | string; secure?: boolean; + /** Defaults to `lax`. */ sameSite?: CookieSameSite; httpOnly?: boolean; partitioned?: boolean; diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index f8047f3bbf..89e5f80073 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -4291,6 +4291,12 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp this.doWriteStatus(status); } + if (this.cookies) |cookies| { + this.cookies = null; + defer cookies.deref(); + cookies.write(this.server.?.globalThis, ssl_enabled, @ptrCast(this.resp.?)); + } + if (needs_content_type and // do not insert the content type if it is the fallback value // we may not know the content-type when streaming @@ -4347,12 +4353,6 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp } fn doWriteHeaders(this: *RequestContext, headers: *JSC.FetchHeaders) void { - if (this.cookies) |cookies| { - this.cookies = null; - defer cookies.deref(); - cookies.write(this.server.?.globalThis, ssl_enabled, @ptrCast(this.resp.?)); - } - writeHeaders(headers, ssl_enabled, this.resp); } diff --git a/src/bun.js/bindings/Cookie.cpp b/src/bun.js/bindings/Cookie.cpp index 9f7ae23042..c4e08d6591 100644 --- a/src/bun.js/bindings/Cookie.cpp +++ b/src/bun.js/bindings/Cookie.cpp @@ -1,4 +1,5 @@ #include "Cookie.h" +#include "EncodeURIComponent.h" #include "JSCookie.h" #include "helpers.h" #include @@ -8,32 +9,6 @@ #include "HTTPParsers.h" namespace WebCore { -extern "C" JSC::EncodedJSValue Cookie__create(JSDOMGlobalObject* globalObject, const ZigString* name, const ZigString* value, const ZigString* domain, const ZigString* path, double expires, bool secure, int32_t sameSite, bool httpOnly, double maxAge, bool partitioned) -{ - String nameStr = Zig::toString(*name); - String valueStr = Zig::toString(*value); - String domainStr = Zig::toString(*domain); - String pathStr = Zig::toString(*path); - - CookieSameSite sameSiteEnum; - switch (sameSite) { - case 0: - sameSiteEnum = CookieSameSite::Strict; - break; - case 1: - sameSiteEnum = CookieSameSite::Lax; - break; - case 2: - sameSiteEnum = CookieSameSite::None; - break; - default: - sameSiteEnum = CookieSameSite::Strict; - } - - auto result = Cookie::create(nameStr, valueStr, domainStr, pathStr, expires, secure, sameSiteEnum, httpOnly, maxAge, partitioned); - return JSC::JSValue::encode(WebCore::toJSNewlyCreated(globalObject, globalObject, WTFMove(result))); -} - extern "C" WebCore::Cookie* Cookie__fromJS(JSC::EncodedJSValue value) { return WebCoreCast(value); @@ -48,7 +23,7 @@ Cookie::Cookie(const String& name, const String& value, : m_name(name) , m_value(value) , m_domain(domain) - , m_path(path.isEmpty() ? "/"_s : path) + , m_path(path) , m_expires(expires) , m_secure(secure) , m_sameSite(sameSite) @@ -58,22 +33,23 @@ Cookie::Cookie(const String& name, const String& value, { } -Ref Cookie::create(const String& name, const String& value, +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 (!isValidCookiePath(path)) { + return Exception { TypeError, "Invalid cookie path: contains invalid characters"_s }; + } + if (!isValidCookieDomain(domain)) { + return Exception { TypeError, "Invalid cookie domain: contains invalid characters"_s }; + } return adoptRef(*new Cookie(name, value, domain, path, expires, secure, sameSite, httpOnly, maxAge, partitioned)); } -Ref Cookie::from(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) -{ - return create(name, value, domain, path, expires, secure, sameSite, httpOnly, maxAge, partitioned); -} - String Cookie::serialize(JSC::VM& vm, const std::span> cookies) { if (cookies.empty()) @@ -206,12 +182,74 @@ String Cookie::toString(JSC::VM& vm) const return builder.toString(); } +static inline bool isValidCharacterInCookieName(UChar c) +{ + return (c >= 0x21 && c <= 0x3A) || (c == 0x3C) || (c >= 0x3E && c <= 0x7E); +} +bool Cookie::isValidCookieName(const String& name) +{ + // /^[\u0021-\u003A\u003C\u003E-\u007E]+$/ + if (name.length() == 0) return false; // disallow empty name + if (name.is8Bit()) { + for (auto c : name.span8()) { + if (!isValidCharacterInCookieName(c)) return false; + } + } else { + for (auto c : name.span16()) { + if (!isValidCharacterInCookieName(c)) return false; + } + } + return true; +} +static inline bool isValidCharacterInCookiePath(UChar c) +{ + return (c >= 0x20 && c <= 0x3A) || (c >= 0x3D && c <= 0x7E); +} +bool Cookie::isValidCookiePath(const String& path) +{ + // /^[\u0020-\u003A\u003D-\u007E]*$/ + if (path.is8Bit()) { + for (auto c : path.span8()) { + if (!isValidCharacterInCookiePath(c)) return false; + } + } else { + for (auto c : path.span16()) { + if (!isValidCharacterInCookiePath(c)) return false; + } + } + return true; +} + +static inline bool isValidCharacterInCookieDomain(UChar c) +{ + return (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-'; +} +bool Cookie::isValidCookieDomain(const String& domain) +{ + // TODO: /^([.]?[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)([.][a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$/i + // for now, require all characters to be [a-z0-9.-] + if (domain.is8Bit()) { + for (auto c : domain.span8()) { + if (!isValidCharacterInCookieDomain(c)) return false; + } + } else { + for (auto c : domain.span16()) { + if (!isValidCharacterInCookieDomain(c)) return false; + } + } + return true; +} + void Cookie::appendTo(JSC::VM& vm, StringBuilder& builder) const { // Name=Value is the basic format - builder.append(WTF::encodeWithURLEscapeSequences(m_name)); + builder.append(m_name); builder.append('='); - builder.append(WTF::encodeWithURLEscapeSequences(m_value)); + 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 + } // Add domain if present if (!m_domain.isEmpty()) { @@ -219,7 +257,7 @@ void Cookie::appendTo(JSC::VM& vm, StringBuilder& builder) const builder.append(m_domain); } - if (!m_path.isEmpty() && m_path != "/"_s) { + if (!m_path.isEmpty()) { builder.append("; Path="_s); builder.append(m_path); } diff --git a/src/bun.js/bindings/Cookie.h b/src/bun.js/bindings/Cookie.h index 4018b93f36..24a153dc44 100644 --- a/src/bun.js/bindings/Cookie.h +++ b/src/bun.js/bindings/Cookie.h @@ -19,7 +19,7 @@ struct CookieInit { String name = String(); String value = String(); String domain = String(); - String path = String(); + String path = "/"_s; int64_t expires = emptyExpiresAtValue; bool secure = false; @@ -37,35 +37,54 @@ class Cookie : public RefCounted { public: ~Cookie(); static constexpr int64_t emptyExpiresAtValue = std::numeric_limits::min(); - static Ref create(const String& name, const String& value, + static ExceptionOr> 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); - static Ref create(const CookieInit& init) + 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); } static ExceptionOr> parse(StringView cookieString); - static Ref from(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); static String serialize(JSC::VM& vm, const std::span> cookies); const String& name() const { return m_name; } - void setName(const String& name) { m_name = name; } const String& value() const { return m_value; } void setValue(const String& value) { m_value = value; } const String& domain() const { return m_domain; } - void setDomain(const String& domain) { m_domain = domain; } + ExceptionOr setDomain(const String& domain) + { + if (!isValidCookieDomain(domain)) { + return Exception { TypeError, "Invalid cookie domain: contains invalid characters"_s }; + } + m_domain = domain; + return {}; + } const String& path() const { return m_path; } - void setPath(const String& path) { m_path = path; } + ExceptionOr setPath(const String& path) + { + if (!isValidCookiePath(path)) { + return Exception { TypeError, "Invalid cookie path: contains invalid characters"_s }; + } + m_path = path; + return {}; + } int64_t expires() const { return m_expires; } void setExpires(int64_t ms) { m_expires = ms; } @@ -93,6 +112,11 @@ public: JSC::JSValue toJSON(JSC::VM& vm, JSC::JSGlobalObject*) const; size_t memoryCost() const; + static bool isValidCookieName(const String& name); + static bool isValidCookieValue(const String& value); // values are uri component encoded, so this isn't needed + static bool isValidCookiePath(const String& path); + static bool isValidCookieDomain(const String& domain); + private: Cookie(const String& name, const String& value, const String& domain, const String& path, diff --git a/src/bun.js/bindings/CookieMap.cpp b/src/bun.js/bindings/CookieMap.cpp index acc0ff4e5d..08c32a4552 100644 --- a/src/bun.js/bindings/CookieMap.cpp +++ b/src/bun.js/bindings/CookieMap.cpp @@ -196,7 +196,7 @@ void CookieMap::set(Ref cookie) m_modifiedCookies.append(WTFMove(cookie)); } -void CookieMap::remove(const CookieStoreDeleteOptions& options) +ExceptionOr CookieMap::remove(const CookieStoreDeleteOptions& options) { removeInternal(options.name); @@ -205,8 +205,13 @@ void CookieMap::remove(const CookieStoreDeleteOptions& options) String path = options.path; // Add the new cookie - auto cookie = Cookie::create(name, ""_s, domain, path, 1, false, CookieSameSite::Lax, false, std::numeric_limits::quiet_NaN(), false); + auto cookie_exception = Cookie::create(name, ""_s, domain, path, 1, false, CookieSameSite::Lax, false, std::numeric_limits::quiet_NaN(), false); + if (cookie_exception.hasException()) { + return cookie_exception.releaseException(); + } + auto cookie = cookie_exception.releaseReturnValue(); m_modifiedCookies.append(WTFMove(cookie)); + return {}; } size_t CookieMap::size() const @@ -227,13 +232,13 @@ JSC::JSValue CookieMap::toJSON(JSC::JSGlobalObject* globalObject) const // Create an object to hold cookie key-value pairs auto* object = JSC::constructEmptyObject(globalObject); - RETURN_IF_EXCEPTION(scope, JSC::jsNull()); + RETURN_IF_EXCEPTION(scope, {}); // Add modified cookies to the object for (const auto& cookie : m_modifiedCookies) { if (!cookie->value().isEmpty()) { object->putDirect(vm, JSC::Identifier::fromString(vm, cookie->name()), JSC::jsString(vm, cookie->value())); - RETURN_IF_EXCEPTION(scope, JSC::jsNull()); + RETURN_IF_EXCEPTION(scope, {}); } } @@ -242,7 +247,7 @@ JSC::JSValue CookieMap::toJSON(JSC::JSGlobalObject* globalObject) const // Skip if this cookie name was already added from modified cookies if (!object->hasProperty(globalObject, JSC::Identifier::fromString(vm, cookie.key))) { object->putDirect(vm, JSC::Identifier::fromString(vm, cookie.key), JSC::jsString(vm, cookie.value)); - RETURN_IF_EXCEPTION(scope, JSC::jsNull()); + RETURN_IF_EXCEPTION(scope, {}); } } diff --git a/src/bun.js/bindings/CookieMap.h b/src/bun.js/bindings/CookieMap.h index 07ec4200b2..66a8c3ee8d 100644 --- a/src/bun.js/bindings/CookieMap.h +++ b/src/bun.js/bindings/CookieMap.h @@ -38,7 +38,7 @@ public: void set(Ref); - void remove(const CookieStoreDeleteOptions& options); + ExceptionOr remove(const CookieStoreDeleteOptions& options); JSC::JSValue toJSON(JSC::JSGlobalObject*) const; size_t size() const; diff --git a/src/bun.js/bindings/EncodeURIComponent.cpp b/src/bun.js/bindings/EncodeURIComponent.cpp new file mode 100644 index 0000000000..2a2a2bca25 --- /dev/null +++ b/src/bun.js/bindings/EncodeURIComponent.cpp @@ -0,0 +1,101 @@ +#include "EncodeURIComponent.h" + +// from JSGlobalObjectFunctions.cpp + +namespace JSC { + +template +static WebCore::ExceptionOr encode(VM& vm, const WTF::BitSet<256>& doNotEscape, std::span characters, StringBuilder& builder) +{ + auto scope = DECLARE_THROW_SCOPE(vm); + + // 18.2.6.1.1 Runtime Semantics: Encode ( string, unescapedSet ) + // https://tc39.github.io/ecma262/#sec-encode + + auto throwException = [] { + return WebCore::ExceptionOr(WebCore::Exception { WebCore::EncodingError, "String contained an illegal UTF-16 sequence."_s }); + }; + + builder.reserveCapacity(characters.size()); + + // 4. Repeat + auto* end = characters.data() + characters.size(); + for (auto* cursor = characters.data(); cursor != end; ++cursor) { + auto character = *cursor; + + // 4-c. If C is in unescapedSet, then + if (character < doNotEscape.size() && doNotEscape.get(character)) { + // 4-c-i. Let S be a String containing only the code unit C. + // 4-c-ii. Let R be a new String value computed by concatenating the previous value of R and S. + builder.append(static_cast(character)); + continue; + } + + // 4-d-i. If the code unit value of C is not less than 0xDC00 and not greater than 0xDFFF, throw a URIError exception. + if (U16_IS_TRAIL(character)) + return throwException(); + + // 4-d-ii. If the code unit value of C is less than 0xD800 or greater than 0xDBFF, then + // 4-d-ii-1. Let V be the code unit value of C. + char32_t codePoint; + if (!U16_IS_LEAD(character)) + codePoint = character; + else { + // 4-d-iii. Else, + // 4-d-iii-1. Increase k by 1. + ++cursor; + + // 4-d-iii-2. If k equals strLen, throw a URIError exception. + if (cursor == end) + return throwException(); + + // 4-d-iii-3. Let kChar be the code unit value of the code unit at index k within string. + auto trail = *cursor; + + // 4-d-iii-4. If kChar is less than 0xDC00 or greater than 0xDFFF, throw a URIError exception. + if (!U16_IS_TRAIL(trail)) + return throwException(); + + // 4-d-iii-5. Let V be UTF16Decode(C, kChar). + codePoint = U16_GET_SUPPLEMENTARY(character, trail); + } + + // 4-d-iv. Let Octets be the array of octets resulting by applying the UTF-8 transformation to V, and let L be the array size. + LChar utf8OctetsBuffer[U8_MAX_LENGTH]; + unsigned utf8Length = 0; + // We can use U8_APPEND_UNSAFE here since codePoint is either + // 1. non surrogate one, correct code point. + // 2. correct code point generated from validated lead and trail surrogates. + U8_APPEND_UNSAFE(utf8OctetsBuffer, utf8Length, codePoint); + + // 4-d-v. Let j be 0. + // 4-d-vi. Repeat, while j < L + for (unsigned index = 0; index < utf8Length; ++index) { + // 4-d-vi-1. Let jOctet be the value at index j within Octets. + // 4-d-vi-2. Let S be a String containing three code units "%XY" where XY are two uppercase hexadecimal digits encoding the value of jOctet. + // 4-d-vi-3. Let R be a new String value computed by concatenating the previous value of R and S. + builder.append('%'); + builder.append(hex(utf8OctetsBuffer[index], 2)); + } + } + return {}; +} + +static WebCore::ExceptionOr encode(VM& vm, WTF::StringView view, const WTF::BitSet<256>& doNotEscape, StringBuilder& builder) +{ + if (view.is8Bit()) + return encode(vm, doNotEscape, view.span8(), builder); + return encode(vm, doNotEscape, view.span16(), builder); +} + +WebCore::ExceptionOr encodeURIComponent(VM& vm, WTF::StringView source, StringBuilder& builder) +{ + static constexpr auto doNotEscapeWhenEncodingURIComponent = makeLatin1CharacterBitSet( + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789" + "!'()*-._~"); + return encode(vm, source, doNotEscapeWhenEncodingURIComponent, builder); +} + +} diff --git a/src/bun.js/bindings/EncodeURIComponent.h b/src/bun.js/bindings/EncodeURIComponent.h new file mode 100644 index 0000000000..b42d5f15fe --- /dev/null +++ b/src/bun.js/bindings/EncodeURIComponent.h @@ -0,0 +1,12 @@ +#pragma once +#include "root.h" + +#include +#include +#include +#include "ExceptionOr.h" + +namespace JSC { +// errors if the string includes unpaired surrogates +WebCore::ExceptionOr encodeURIComponent(VM& vm, WTF::StringView source, StringBuilder& builder); +} diff --git a/src/bun.js/bindings/ExceptionOr.h b/src/bun.js/bindings/ExceptionOr.h index 9581644f35..aed54f7375 100644 --- a/src/bun.js/bindings/ExceptionOr.h +++ b/src/bun.js/bindings/ExceptionOr.h @@ -35,7 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace WebCore { -template class ExceptionOr { +template class [[nodiscard]] ExceptionOr { public: using ReturnType = T; diff --git a/src/bun.js/bindings/webcore/InternalWritableStream.cpp b/src/bun.js/bindings/webcore/InternalWritableStream.cpp index d63acc71f5..b368692f88 100644 --- a/src/bun.js/bindings/webcore/InternalWritableStream.cpp +++ b/src/bun.js/bindings/webcore/InternalWritableStream.cpp @@ -110,7 +110,7 @@ void InternalWritableStream::lock() arguments.append(guardedObject()); ASSERT(!arguments.hasOverflowed()); - invokeWritableStreamFunction(*globalObject, privateName, arguments); + auto result = invokeWritableStreamFunction(*globalObject, privateName, arguments); if (UNLIKELY(scope.exception())) scope.clearException(); } diff --git a/src/bun.js/bindings/webcore/JSCookie.cpp b/src/bun.js/bindings/webcore/JSCookie.cpp index 79fda89359..78ce7a22e1 100644 --- a/src/bun.js/bindings/webcore/JSCookie.cpp +++ b/src/bun.js/bindings/webcore/JSCookie.cpp @@ -301,12 +301,12 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSCookieDOMConstructor:: RELEASE_AND_RETURN(throwScope, {}); } - auto result = Cookie::parse(cookieString); - if (result.hasException()) { - WebCore::propagateException(lexicalGlobalObject, throwScope, result.releaseException()); + auto cookie_exception = Cookie::parse(cookieString); + if (cookie_exception.hasException()) { + WebCore::propagateException(lexicalGlobalObject, throwScope, cookie_exception.releaseException()); } RETURN_IF_EXCEPTION(throwScope, {}); - auto cookie = result.releaseReturnValue(); + auto cookie = cookie_exception.releaseReturnValue(); auto* globalObject = castedThis->globalObject(); RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(lexicalGlobalObject, globalObject, WTFMove(cookie)))); @@ -323,7 +323,12 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSCookieDOMConstructor:: RETURN_IF_EXCEPTION(throwScope, {}); ASSERT(cookieInit); - auto cookie = Cookie::create(*cookieInit); + auto cookie_exception = Cookie::create(*cookieInit); + if (cookie_exception.hasException()) { + WebCore::propagateException(lexicalGlobalObject, throwScope, cookie_exception.releaseException()); + } + RETURN_IF_EXCEPTION(throwScope, {}); + auto cookie = cookie_exception.releaseReturnValue(); auto* globalObject = castedThis->globalObject(); RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(lexicalGlobalObject, globalObject, WTFMove(cookie)))); } else if (callFrame->argumentCount() >= 2) { @@ -353,7 +358,13 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSCookieDOMConstructor:: RETURN_IF_EXCEPTION(throwScope, {}); } - auto cookie = Cookie::create(cookieInit); + auto cookie_exception = Cookie::create(cookieInit); + if (cookie_exception.hasException()) { + WebCore::propagateException(lexicalGlobalObject, throwScope, cookie_exception.releaseException()); + } + RETURN_IF_EXCEPTION(throwScope, {}); + auto cookie = cookie_exception.releaseReturnValue(); + auto* globalObject = castedThis->globalObject(); RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(lexicalGlobalObject, globalObject, WTFMove(cookie)))); } @@ -513,7 +524,13 @@ JSC_DEFINE_HOST_FUNCTION(jsCookieStaticFunctionParse, (JSGlobalObject * lexicalG RETURN_IF_EXCEPTION(throwScope, {}); if (cookieString.isEmpty()) { - return JSValue::encode(toJSNewlyCreated(lexicalGlobalObject, defaultGlobalObject(lexicalGlobalObject), Cookie::create(CookieInit {}))); + auto cookie_exception = Cookie::create(CookieInit {}); + if (cookie_exception.hasException()) { + WebCore::propagateException(lexicalGlobalObject, throwScope, cookie_exception.releaseException()); + } + RETURN_IF_EXCEPTION(throwScope, {}); + auto cookie = cookie_exception.releaseReturnValue(); + return JSValue::encode(toJSNewlyCreated(lexicalGlobalObject, defaultGlobalObject(lexicalGlobalObject), WTFMove(cookie))); } if (UNLIKELY(!WebCore::isValidHTTPHeaderValue(cookieString))) { @@ -521,15 +538,15 @@ JSC_DEFINE_HOST_FUNCTION(jsCookieStaticFunctionParse, (JSGlobalObject * lexicalG RELEASE_AND_RETURN(throwScope, {}); } - auto result = Cookie::parse(cookieString); - if (result.hasException()) { - WebCore::propagateException(lexicalGlobalObject, throwScope, result.releaseException()); + auto cookie_exception = Cookie::parse(cookieString); + if (cookie_exception.hasException()) { + WebCore::propagateException(lexicalGlobalObject, throwScope, cookie_exception.releaseException()); } - RETURN_IF_EXCEPTION(throwScope, {}); + auto cookie = cookie_exception.releaseReturnValue(); auto* globalObject = defaultGlobalObject(lexicalGlobalObject); - RELEASE_AND_RETURN(throwScope, JSValue::encode(toJSNewlyCreated(lexicalGlobalObject, globalObject, result.releaseReturnValue()))); + RELEASE_AND_RETURN(throwScope, JSValue::encode(toJSNewlyCreated(lexicalGlobalObject, globalObject, WTFMove(cookie)))); } JSC_DEFINE_HOST_FUNCTION(jsCookieStaticFunctionFrom, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) @@ -561,7 +578,12 @@ JSC_DEFINE_HOST_FUNCTION(jsCookieStaticFunctionFrom, (JSGlobalObject * lexicalGl RETURN_IF_EXCEPTION(throwScope, {}); } - auto cookie = Cookie::create(cookieInit); + auto cookie_exception = Cookie::create(cookieInit); + if (cookie_exception.hasException()) { + WebCore::propagateException(lexicalGlobalObject, throwScope, cookie_exception.releaseException()); + } + RETURN_IF_EXCEPTION(throwScope, {}); + auto cookie = cookie_exception.releaseReturnValue(); auto* globalObject = jsCast(lexicalGlobalObject); return JSValue::encode(toJSNewlyCreated(lexicalGlobalObject, globalObject, WTFMove(cookie))); } @@ -649,7 +671,7 @@ JSC_DEFINE_CUSTOM_SETTER(jsCookiePrototypeSetter_domain, (JSGlobalObject * lexic auto& impl = thisObject->wrapped(); auto value = convert(*lexicalGlobalObject, JSValue::decode(encodedValue)); RETURN_IF_EXCEPTION(throwScope, false); - impl.setDomain(value); + WebCore::propagateException(*lexicalGlobalObject, throwScope, impl.setDomain(value)); return true; } @@ -674,7 +696,7 @@ JSC_DEFINE_CUSTOM_SETTER(jsCookiePrototypeSetter_path, (JSGlobalObject * lexical auto& impl = thisObject->wrapped(); auto value = convert(*lexicalGlobalObject, JSValue::decode(encodedValue)); RETURN_IF_EXCEPTION(throwScope, false); - impl.setPath(value); + WebCore::propagateException(*lexicalGlobalObject, throwScope, impl.setPath(value)); return true; } diff --git a/src/bun.js/bindings/webcore/JSCookieMap.cpp b/src/bun.js/bindings/webcore/JSCookieMap.cpp index 5de3a816d2..4abc49ff01 100644 --- a/src/bun.js/bindings/webcore/JSCookieMap.cpp +++ b/src/bun.js/bindings/webcore/JSCookieMap.cpp @@ -173,10 +173,14 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSCookieMapDOMConstructo return {}; } - auto result = CookieMap::create(WTFMove(init)); + auto result_exception = CookieMap::create(WTFMove(init)); + if (result_exception.hasException()) { + WebCore::propagateException(lexicalGlobalObject, throwScope, result_exception.releaseException()); + } RETURN_IF_EXCEPTION(throwScope, {}); + auto result = result_exception.releaseReturnValue(); - RELEASE_AND_RETURN(throwScope, JSValue::encode(toJSNewlyCreated(lexicalGlobalObject, castedThis->globalObject(), result.releaseReturnValue()))); + RELEASE_AND_RETURN(throwScope, JSValue::encode(toJSNewlyCreated(lexicalGlobalObject, castedThis->globalObject(), WTFMove(result)))); } JSC_ANNOTATE_HOST_FUNCTION(JSCookieMapDOMConstructorConstruct, JSCookieMapDOMConstructor::construct); @@ -400,7 +404,13 @@ static inline JSC::EncodedJSValue jsCookieMapPrototypeFunction_setBody(JSC::JSGl } } - auto cookie = Cookie::create(cookieInit); + auto cookie_exception = Cookie::create(cookieInit); + if (cookie_exception.hasException()) { + WebCore::propagateException(lexicalGlobalObject, throwScope, cookie_exception.releaseException()); + } + RETURN_IF_EXCEPTION(throwScope, {}); + auto cookie = cookie_exception.releaseReturnValue(); + impl.set(WTFMove(cookie)); return JSValue::encode(jsUndefined()); @@ -480,7 +490,8 @@ static inline JSC::EncodedJSValue jsCookieMapPrototypeFunction_deleteBody(JSC::J return throwVMError(lexicalGlobalObject, throwScope, createTypeError(lexicalGlobalObject, "Cookie name is required"_s)); } - impl.remove(deleteOptions); + WebCore::propagateException(*lexicalGlobalObject, throwScope, impl.remove(deleteOptions)); + RETURN_IF_EXCEPTION(throwScope, {}); return JSValue::encode(jsUndefined()); } diff --git a/src/bun.js/bindings/webcore/ReadableStream.cpp b/src/bun.js/bindings/webcore/ReadableStream.cpp index c6cf7882e1..51395243b3 100644 --- a/src/bun.js/bindings/webcore/ReadableStream.cpp +++ b/src/bun.js/bindings/webcore/ReadableStream.cpp @@ -148,7 +148,7 @@ std::optional, Ref>> ReadableStrea void ReadableStream::lock() { auto& builtinNames = WebCore::builtinNames(m_globalObject->vm()); - invokeConstructor(*m_globalObject, builtinNames.ReadableStreamDefaultReaderPrivateName(), [this](auto& args, auto&, auto&) { + auto result = invokeConstructor(*m_globalObject, builtinNames.ReadableStreamDefaultReaderPrivateName(), [this](auto& args, auto&, auto&) { args.append(readableStream()); }); } diff --git a/test/js/bun/cookie/cookie-exotic-inputs.test.ts b/test/js/bun/cookie/cookie-exotic-inputs.test.ts index 7c990d112a..c47f5ae79a 100644 --- a/test/js/bun/cookie/cookie-exotic-inputs.test.ts +++ b/test/js/bun/cookie/cookie-exotic-inputs.test.ts @@ -251,7 +251,7 @@ describe("Bun.Cookie.parse with exotic inputs", () => { // URL encoded chars in path "name=value; Path=/path%20with%20spaces", // URL encoded chars in domain (though questionable) - "name=value; Domain=weird%2Edomain.com", + // "name=value; Domain=weird%2Edomain.com", ]; for (const cookieStr of exoticCompliantCases) { diff --git a/test/js/bun/cookie/cookie-map.test.ts b/test/js/bun/cookie/cookie-map.test.ts index 5493e6aabd..5f4b622143 100644 --- a/test/js/bun/cookie/cookie-map.test.ts +++ b/test/js/bun/cookie/cookie-map.test.ts @@ -112,8 +112,8 @@ describe("Bun.Cookie and Bun.CookieMap", () => { const cookie2 = new Bun.Cookie("baz", "qux"); expect(cookie1.serialize() + "\n" + cookie2.serialize()).toMatchInlineSnapshot(` - "foo=bar; SameSite=Lax - baz=qux; SameSite=Lax" + "foo=bar; Path=/; SameSite=Lax + baz=qux; Path=/; SameSite=Lax" `); }); @@ -183,8 +183,8 @@ describe("Bun.Cookie and Bun.CookieMap", () => { expect(map.has("foo")).toBe(true); expect(map.toSetCookieHeaders()).toMatchInlineSnapshot(` [ - "name=value; SameSite=Lax", - "foo=bar; Secure; HttpOnly; Partitioned; SameSite=Lax", + "name=value; Path=/; SameSite=Lax", + "foo=bar; Path=/; Secure; HttpOnly; Partitioned; SameSite=Lax", ] `); @@ -197,8 +197,8 @@ describe("Bun.Cookie and Bun.CookieMap", () => { // Get changes expect(map.toSetCookieHeaders()).toMatchInlineSnapshot(` [ - "foo=bar; Secure; HttpOnly; Partitioned; SameSite=Lax", - "name=; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", + "foo=bar; Path=/; Secure; HttpOnly; Partitioned; SameSite=Lax", + "name=; Path=/; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", ] `); }); @@ -252,7 +252,7 @@ describe("Bun.Cookie and Bun.CookieMap", () => { expect(map.get("session")).toBe("abc123"); expect(map.toSetCookieHeaders()).toMatchInlineSnapshot(` [ - "session=abc123; Max-Age=3600; Secure; HttpOnly; Partitioned; SameSite=Lax", + "session=abc123; Path=/; Max-Age=3600; Secure; HttpOnly; Partitioned; SameSite=Lax", ] `); }); @@ -275,7 +275,7 @@ describe("Cookie name field is immutable", () => { expect(cookieMap.get("name")).toBe("value2"); expect(cookieMap.toSetCookieHeaders()).toMatchInlineSnapshot(` [ - "name=value2; SameSite=Lax", + "name=value2; Path=/; SameSite=Lax", ] `); }); diff --git a/test/js/bun/cookie/cookie.test.ts b/test/js/bun/cookie/cookie.test.ts index c391284895..51c084aee7 100644 --- a/test/js/bun/cookie/cookie.test.ts +++ b/test/js/bun/cookie/cookie.test.ts @@ -1,4 +1,4 @@ -import { test, expect, describe } from "bun:test"; +import { test, expect, describe, afterAll } from "bun:test"; describe("Bun.Cookie validation tests", () => { describe("expires validation", () => { @@ -99,6 +99,7 @@ describe("Bun.Cookie validation tests", () => { }); }); +console.log("describe Bun.serve() cookies"); describe("Bun.serve() cookies", () => { const server = Bun.serve({ port: 0, @@ -125,6 +126,9 @@ describe("Bun.serve() cookies", () => { }, }, }); + afterAll(() => { + server.stop(); + }); test("set-cookie", async () => { const res = await fetch(server.url + "/tester", { @@ -140,7 +144,7 @@ describe("Bun.serve() cookies", () => { `); expect(res.headers.getAll("Set-Cookie")).toMatchInlineSnapshot(` [ - "test=test; SameSite=Lax", + "test=test; Path=/; SameSite=Lax", ] `); }); @@ -162,8 +166,8 @@ describe("Bun.serve() cookies", () => { `); expect(res.headers.getAll("Set-Cookie")).toMatchInlineSnapshot(` [ - "test=test; SameSite=Lax", - "test2=test2; SameSite=Lax", + "test=test; Path=/; SameSite=Lax", + "test2=test2; Path=/; SameSite=Lax", ] `); }); @@ -177,7 +181,7 @@ describe("Bun.serve() cookies", () => { expect(body).toMatchInlineSnapshot(`{}`); expect(res.headers.getAll("Set-Cookie")).toMatchInlineSnapshot(` [ - "test=; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", + "test=; Path=/; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", ] `); }); @@ -203,8 +207,8 @@ describe("Bun.serve() cookies", () => { `); expect(res.headers.getAll("Set-Cookie")).toMatchInlineSnapshot(` [ - "do_modify=c; SameSite=Lax", - "add_cookie=d; SameSite=Lax", + "do_modify=c; Path=/; SameSite=Lax", + "add_cookie=d; Path=/; SameSite=Lax", ] `); }); @@ -234,8 +238,8 @@ describe("Bun.serve() cookies", () => { map.set("do_modify", "FIVE"); expect(map.toSetCookieHeaders()).toMatchInlineSnapshot(` [ - "do_delete=; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", - "do_modify=FIVE; SameSite=Lax", + "do_delete=; Path=/; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", + "do_modify=FIVE; Path=/; SameSite=Lax", ] `); expect(map.toJSON()).toMatchInlineSnapshot(` @@ -246,3 +250,124 @@ describe("Bun.serve() cookies", () => { `); }); }); + +describe("Bun.serve() cookies 2", () => { + const server = Bun.serve({ + port: 0, + routes: { + "/": req => { + // Access request cookies + const cookies = req.cookies; + + // Get a specific cookie + const sessionCookie = cookies.get("session"); + if (sessionCookie != null) { + // console.log(sessionCookie); + } + + // Check if a cookie exists + if (cookies.has("theme")) { + // ... + } + + // Set a cookie, it will be automatically applied to the response + cookies.set("visited", "true"); + + console.log(cookies.toSetCookieHeaders()); + + return new Response("Hello"); + }, + "/redirect": req => { + req.cookies.set("redirected", "true"); + return Response.redirect("/redirect-target"); + }, + }, + }); + afterAll(() => { + server.stop(); + }); + + test("server sets cookie", async () => { + const response = await fetch(server.url, { + headers: { + "Cookie": "abc=def; ghi=jkl", + }, + }); + expect(response.headers.getAll("Set-Cookie")).toMatchInlineSnapshot(` + [ + "visited=true; Path=/; SameSite=Lax", + ] + `); + }); + test("server sets cookie on redirect", async () => { + const response = await fetch(server.url + "/redirect", { + headers: { + "Cookie": "abc=def; ghi=jkl", + }, + redirect: "manual", + }); + expect(response.status).toBe(302); + expect(response.headers.get("Location")).toBe("/redirect-target"); + expect(response.headers.getAll("Set-Cookie")).toMatchInlineSnapshot(` + [ + "redirected=true; Path=/; SameSite=Lax", + ] + `); + }); +}); + +describe("cookie path option", () => { + const server = Bun.serve({ + port: 0, + routes: { + "/x/y": { + GET(r) { + r.cookies.set("user", "a", { maxAge: 3600, path: "/" }); + const cookie = r.cookies.toSetCookieHeaders().at(0); + return new Response("ok", { + headers: { "set-cookie": cookie }, + }); + }, + }, + }, + }); + afterAll(() => server.stop()); + + test("cookie path option", async () => { + const response = await fetch(server.url + "/x/y"); + expect(response.status).toBe(200); + expect(response.headers.getAll("Set-Cookie")).toMatchInlineSnapshot(` + [ + "user=a; Path=/; Max-Age=3600; SameSite=Lax", + "user=a; Path=/; Max-Age=3600; SameSite=Lax", + ] + `); + }); +}); +test("delete cookie path option", () => { + const map = new Bun.CookieMap(); + map.delete("a", { path: "/b" }); + map.delete("b", { path: "" }); + map.delete("c", {}); + map.delete("d", { path: "/" }); + expect(map.toSetCookieHeaders()).toMatchInlineSnapshot(` + [ + "a=; Path=/b; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", + "b=; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", + "c=; Path=/; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", + "d=; Path=/; Expires=Fri, 1 Jan 1970 00:00:00 -0000; SameSite=Lax", + ] + `); +}); +test("delete cookie invalid path option", () => { + const map = new Bun.CookieMap(); + expect(() => map.delete("a", { path: "\n" })).toThrowErrorMatchingInlineSnapshot( + `"Invalid cookie path: contains invalid characters"`, + ); + expect(() => map.delete("a", { domain: "\n" })).toThrowErrorMatchingInlineSnapshot( + `"Invalid cookie domain: contains invalid characters"`, + ); + expect(() => map.delete("\n", {})).toThrowErrorMatchingInlineSnapshot( + `"Invalid cookie name: contains invalid characters"`, + ); +}); diff --git a/test/js/bun/http/bun-serve-cookies.test.ts b/test/js/bun/http/bun-serve-cookies.test.ts index e27d172199..402666895f 100644 --- a/test/js/bun/http/bun-serve-cookies.test.ts +++ b/test/js/bun/http/bun-serve-cookies.test.ts @@ -495,7 +495,7 @@ describe("Direct usage of Bun.Cookie and Bun.CookieMap", () => { expect(fooCookie).toMatchInlineSnapshot(`"bar"`); expect(cookieMap.toSetCookieHeaders()).toMatchInlineSnapshot(` [ - "name=value; SameSite=Lax", + "name=value; Path=/; SameSite=Lax", "foo=bar; Path=/path; Secure; SameSite=Lax", ] `); diff --git a/test/js/bun/util/cookie-server/server.ts b/test/js/bun/util/cookie-server/server.ts deleted file mode 100644 index 25a46cc391..0000000000 --- a/test/js/bun/util/cookie-server/server.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { CookieMap } from "bun"; - -function mainHTML(cookies: CookieMap) { - return ` - - - Hello World. Your cookies are: -
    - ${Array.from(cookies.entries()) - .map( - ([, cookie]) => - `
  • ${Bun.escapeHTML(cookie.name)}: ${Bun.escapeHTML(cookie.value)}
    • `, - ) - .join("\n")} -
  • -
    - -
    -
    • -
- - - `; -} - -Bun.serve({ - port: 3000, - routes: { - "/": req => new Response(mainHTML(req.cookies), { headers: { "content-type": "text/html" } }), - "/update-cookies": req => { - const cookies = req.cookies; - cookies.set("test", "test"); - // return new Response("Cookies updated"); - return Response.redirect("/"); - }, - }, -}); diff --git a/test/js/bun/util/cookie.test.js b/test/js/bun/util/cookie.test.js index dc92a4da3c..a2308fc855 100644 --- a/test/js/bun/util/cookie.test.js +++ b/test/js/bun/util/cookie.test.js @@ -238,11 +238,38 @@ describe("Bun.CookieMap", () => { }); }); +test("cookie invalid surrogate pair", () => { + const cookie = new Bun.Cookie("name", "hello\uD800goodbye", {}); + expect(cookie.value).toBe("hello\uFFFDgoodbye"); + expect(cookie.toString()).toMatchInlineSnapshot(`"name=hello%EF%BF%BDgoodbye; Path=/; SameSite=Lax"`); + + const cookie2 = new Bun.Cookie("name", "abcdefg", {}); + cookie2.value = "hello\uD800goodbye"; + expect(cookie2.value).toBe("hello\uFFFDgoodbye"); + expect(cookie2.toString()).toMatchInlineSnapshot(`"name=hello%EF%BF%BDgoodbye; Path=/; SameSite=Lax"`); +}); + +test("validation errors", () => { + const mycookie = new Bun.Cookie("a", "b"); + expect(() => (mycookie.domain = "ndcla \nkjnc iap!PL)P890u89iop")).toThrow( + /Invalid cookie domain: contains invalid characters/, + ); + expect(mycookie.domain).toBe(null); + expect(() => (mycookie.path = "ndcla \nkjnc iap!PL)P890u89iop")).toThrow( + /Invalid cookie path: contains invalid characters/, + ); + expect(mycookie.path).toBe("/"); + // set name does nothing with no error + mycookie.name = "ndcla \nkjnc iap!PL)P890u89iop"; + expect(mycookie.name).toBe("a"); +}); + const cookie = { parse: str => { return Object.fromEntries(new Bun.CookieMap(str).entries()); }, serialize: (name, value, options) => { + options = { path: "", ...options }; const cookie = new Bun.Cookie(name, value, options); return cookie.toString(); }, @@ -366,8 +393,8 @@ describe("cookie.serialize(name, value)", function () { expect(cookie.serialize("foo", "bar")).toEqual("foo=bar; SameSite=Lax"); }); - it.failing("should URL-encode value", function () { - expect(cookie.serialize("foo", "bar +baz")).toEqual("foo=bar%20%2Bbaz; SameSite=Lax"); + it("should URL-encode value", function () { + expect(cookie.serialize("foo", "bar +baz;")).toEqual("foo=bar%20%2Bbaz%3B; SameSite=Lax"); }); it("should serialize empty value", function () { @@ -378,41 +405,43 @@ describe("cookie.serialize(name, value)", function () { ["foo"], ["foo,bar"], ["foo!bar"], - // ["foo#bar"], + ["foo#bar"], ["foo$bar"], ["foo'bar"], ["foo*bar"], ["foo+bar"], ["foo-bar"], ["foo.bar"], - // ["foo^bar"], + ["foo^bar"], ["foo_bar"], - // ["foo`bar"], - // ["foo|bar"], + ["foo`bar"], + ["foo|bar"], ["foo~bar"], ["foo7bar"], - // ["foo/bar"], - // ["foo@bar"], - // ["foo[bar"], - // ["foo]bar"], - // ["foo:bar"], - // ["foo{bar"], - // ["foo}bar"], - // ['foo"bar'], - // ["foobar"], - // ["foo?bar"], - // ["foo\\bar"], + ["foo/bar"], + ["foo@bar"], + ["foo[bar"], + ["foo]bar"], + ["foo:bar"], + ["foo{bar"], + ["foo}bar"], + ['foo"bar'], + ["foobar"], + ["foo?bar"], + ["foo\\bar"], ])("should serialize name: %s", name => { expect(cookie.serialize(name, "baz")).toEqual(`${name}=baz; SameSite=Lax`); }); - // it.each([["foo\n"], ["foo\u280a"], ["foo=bar"], ["foo;bar"], ["foo bar"], ["foo\tbar"]])( - // "should throw for invalid name: %s", - // name => { - // expect(() => cookie.serialize(name, "bar")).toThrow(/argument name is invalid/); - // }, - // ); + it.each([["foo\n"], ["foo\u280a"], ["foo=bar"], ["foo;bar"], ["foo bar"], ["foo\tbar"], [""]])( + "should throw for invalid name: %s", + name => { + expect(() => cookie.serialize(name, "bar")).toThrow( + /name is required|Invalid cookie name: contains invalid characters/, + ); + }, + ); }); describe("cookie.serialize(name, value, options)", function () { @@ -429,16 +458,18 @@ describe("cookie.serialize(name, value, options)", function () { expect(cookie.serialize("foo", "bar", { domain })).toEqual(`foo=bar; Domain=${domain}; SameSite=Lax`); }); - // it.each([ - // ["example.com\n"], - // ["sub.example.com\u0000"], - // ["my site.org"], - // ["domain..com"], - // ["example.com; Path=/"], - // ["example.com /* inject a comment */"], - // ])("should throw for invalid domain: %s", domain => { - // expect(() => cookie.serialize("foo", "bar", { domain })).toThrow(/option domain is invalid/); - // }); + it.each([ + ["example.com\n"], + ["sub.example.com\u0000"], + ["my site.org"], + // ["domain..com"], // TODO + ["example.com; Path=/"], + ["example.com /* inject a comment */"], + ])("should throw for invalid domain: %s", domain => { + expect(() => cookie.serialize("foo", "bar", { domain })).toThrow( + /Invalid cookie domain: contains invalid characters/, + ); + }); }); describe.skip('with "encode" option', function () {