From c024e73e6afdab01ecacc614a261d77062fdf384 Mon Sep 17 00:00:00 2001 From: Seth Flynn Date: Sat, 24 May 2025 02:37:47 -0400 Subject: [PATCH] fix(BunRequest): make `clone()` return a BunRequest (#19813) --- packages/bun-types/bun.d.ts | 2 + src/bun.js/bindings/CookieMap.cpp | 11 +++++- src/bun.js/bindings/CookieMap.h | 2 + src/bun.js/bindings/JSBunRequest.cpp | 58 ++++++++++++++++++++++++++++ src/bun.js/bindings/JSBunRequest.h | 2 + src/bun.js/webcore/Request.zig | 5 +++ test/regression/issue/18547.test.ts | 28 ++++++++++++++ 7 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 test/regression/issue/18547.test.ts diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index dffb3357df..958ffa3658 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -3304,6 +3304,8 @@ declare module "bun" { interface BunRequest extends Request { params: RouterTypes.ExtractRouteParams; readonly cookies: CookieMap; + + clone(): BunRequest; } interface GenericServeOptions { diff --git a/src/bun.js/bindings/CookieMap.cpp b/src/bun.js/bindings/CookieMap.cpp index 68f667f381..ece4833bdc 100644 --- a/src/bun.js/bindings/CookieMap.cpp +++ b/src/bun.js/bindings/CookieMap.cpp @@ -214,7 +214,16 @@ ExceptionOr CookieMap::remove(const CookieStoreDeleteOptions& options) return {}; } -size_t CookieMap::size() const +Ref CookieMap::clone() +{ + auto clone = adoptRef(*new CookieMap()); + clone->m_originalCookies = m_originalCookies; + clone->m_modifiedCookies = m_modifiedCookies; + return clone; +} + +size_t +CookieMap::size() const { size_t size = 0; for (const auto& cookie : m_modifiedCookies) { diff --git a/src/bun.js/bindings/CookieMap.h b/src/bun.js/bindings/CookieMap.h index 66a8c3ee8d..b664acd01d 100644 --- a/src/bun.js/bindings/CookieMap.h +++ b/src/bun.js/bindings/CookieMap.h @@ -38,6 +38,8 @@ public: void set(Ref); + Ref clone(); + ExceptionOr remove(const CookieStoreDeleteOptions& options); JSC::JSValue toJSON(JSC::JSGlobalObject*) const; diff --git a/src/bun.js/bindings/JSBunRequest.cpp b/src/bun.js/bindings/JSBunRequest.cpp index ce05edb03a..5ac2bec994 100644 --- a/src/bun.js/bindings/JSBunRequest.cpp +++ b/src/bun.js/bindings/JSBunRequest.cpp @@ -11,6 +11,7 @@ #include "JSCookieMap.h" #include "Cookie.h" #include "CookieMap.h" +#include "ErrorCode.h" #include "JSDOMExceptionHandling.h" namespace Bun { @@ -18,9 +19,12 @@ namespace Bun { static JSC_DECLARE_CUSTOM_GETTER(jsJSBunRequestGetParams); static JSC_DECLARE_CUSTOM_GETTER(jsJSBunRequestGetCookies); +static JSC_DECLARE_HOST_FUNCTION(jsJSBunRequestClone); + static const HashTableValue JSBunRequestPrototypeValues[] = { { "params"_s, static_cast(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete), NoIntrinsic, { HashTableValue::GetterSetterType, jsJSBunRequestGetParams, nullptr } }, { "cookies"_s, static_cast(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete), NoIntrinsic, { HashTableValue::GetterSetterType, jsJSBunRequestGetCookies, nullptr } }, + { "clone"_s, static_cast(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsJSBunRequestClone, 1 } } }; JSBunRequest* JSBunRequest::create(JSC::VM& vm, JSC::Structure* structure, void* sinkPtr, JSObject* params) @@ -66,6 +70,44 @@ JSObject* JSBunRequest::cookies() const return nullptr; } +extern "C" void* Request__clone(void* internalZigRequestPointer, JSGlobalObject* globalObject); + +JSBunRequest* JSBunRequest::clone(JSC::VM& vm, JSGlobalObject* globalObject) +{ + auto throwScope = DECLARE_THROW_SCOPE(globalObject->vm()); + + auto* structure = createJSBunRequestStructure(vm, defaultGlobalObject(globalObject)); + auto* clone = this->create(vm, structure, Request__clone(this->wrapped(), globalObject), nullptr); + + // Cookies and params are deep copied as they can be changed between the clone and original + if (auto* params = this->params()) { + // TODO: Use JSC's internal `cloneObject()` if/when it's exposed + // https://github.com/oven-sh/WebKit/blob/c5e9b9e327194f520af2c28679adb0ea1fa902ad/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp#L1018-L1099 + auto* prototype = defaultGlobalObject(globalObject)->m_JSBunRequestParamsPrototype.get(globalObject); + auto* paramsClone = JSC::constructEmptyObject(globalObject, prototype); + + auto propertyNames = PropertyNameArray(vm, JSC::PropertyNameMode::Strings, JSC::PrivateSymbolMode::Exclude); + JSObject::getOwnPropertyNames(params, globalObject, propertyNames, JSC::DontEnumPropertiesMode::Exclude); + + for (auto& property : propertyNames) { + auto value = params->get(globalObject, property); + RETURN_IF_EXCEPTION(throwScope, nullptr); + paramsClone->putDirect(vm, property, value); + } + + clone->setParams(paramsClone); + } + + if (auto* wrapper = jsDynamicCast(this->cookies())) { + auto cookieMap = wrapper->protectedWrapped(); + auto cookieMapClone = cookieMap->clone(); + auto cookies = WebCore::toJSNewlyCreated(globalObject, jsCast(globalObject), WTFMove(cookieMapClone)); + clone->setCookies(cookies.getObject()); + } + + RELEASE_AND_RETURN(throwScope, clone); +} + extern "C" void Request__setCookiesOnRequestContext(void* internalZigRequestPointer, CookieMap* cookieMap); void JSBunRequest::setCookies(JSObject* cookies) @@ -203,6 +245,22 @@ JSC_DEFINE_CUSTOM_GETTER(jsJSBunRequestGetCookies, (JSC::JSGlobalObject * global return JSValue::encode(cookies); } +JSC_DEFINE_HOST_FUNCTION(jsJSBunRequestClone, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + auto& vm = globalObject->vm(); + auto throwScope = DECLARE_THROW_SCOPE(vm); + + auto* request = jsDynamicCast(callFrame->thisValue()); + if (!request) { + throwScope.throwException(globalObject, Bun::createInvalidThisError(globalObject, request, "BunRequest")); + RETURN_IF_EXCEPTION(throwScope, {}); + } + + auto clone = request->clone(vm, globalObject); + + RELEASE_AND_RETURN(throwScope, JSValue::encode(clone)); +} + Structure* createJSBunRequestStructure(JSC::VM& vm, Zig::GlobalObject* globalObject) { auto prototypeStructure = JSBunRequestPrototype::createStructure(vm, globalObject, globalObject->JSRequestPrototype()); diff --git a/src/bun.js/bindings/JSBunRequest.h b/src/bun.js/bindings/JSBunRequest.h index 3c41b504ff..0b28b3f391 100644 --- a/src/bun.js/bindings/JSBunRequest.h +++ b/src/bun.js/bindings/JSBunRequest.h @@ -36,6 +36,8 @@ public: JSObject* cookies() const; void setCookies(JSObject* cookies); + JSBunRequest* clone(JSC::VM& vm, JSGlobalObject* globalObject); + private: JSBunRequest(JSC::VM& vm, JSC::Structure* structure, void* sinkPtr); void finishCreation(JSC::VM& vm, JSObject* params); diff --git a/src/bun.js/webcore/Request.zig b/src/bun.js/webcore/Request.zig index e7669b47d1..813530fc9b 100644 --- a/src/bun.js/webcore/Request.zig +++ b/src/bun.js/webcore/Request.zig @@ -66,7 +66,12 @@ pub export fn Request__setTimeout(this: *Request, seconds: JSC.JSValue, globalTh this.setTimeout(seconds.to(c_uint)); } +pub export fn Request__clone(this: *Request, globalThis: *JSC.JSGlobalObject) *Request { + return this.clone(bun.default_allocator, globalThis); +} + comptime { + _ = Request__clone; _ = Request__getUWSRequest; _ = Request__setInternalEventCallback; _ = Request__setTimeout; diff --git a/test/regression/issue/18547.test.ts b/test/regression/issue/18547.test.ts new file mode 100644 index 0000000000..d489177ccd --- /dev/null +++ b/test/regression/issue/18547.test.ts @@ -0,0 +1,28 @@ +import { expect, test } from "bun:test"; + +test("18547", async () => { + using serve = Bun.serve({ + routes: { + "/:foo": request => { + request.cookies.set("sessionToken", "123456"); + + // Ensure cloned requests have the same cookies and params of the original + const clone = request.clone(); + expect(clone.cookies.get("sessionToken")).toEqual("123456"); + expect(clone.params.foo).toEqual("foo"); + + // And that changes made to the clone don't affect the original + clone.cookies.set("sessionToken", "654321"); + expect(request.cookies.get("sessionToken")).toEqual("123456"); + expect(clone.cookies.get("sessionToken")).toEqual("654321"); + + + return new Response("OK"); + }, + }, + }); + + const response = await fetch(`${serve.url}/foo`); + // Or the context of the original request + expect(response.headers.get("set-cookie")).toEqual("sessionToken=123456; Path=/; SameSite=Lax"); +});