From 11f18a2ca304ad9b9e37556238407455c4e28c61 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Wed, 13 Nov 2024 16:05:18 +0300 Subject: [PATCH 1/4] Avoid polluting the original object in case of `Object.create` --- .changeset/plenty-walls-clap.md | 5 ++ packages/server/src/utils.ts | 80 +++------------------ packages/server/test/server-context.spec.ts | 34 +++++++++ 3 files changed, 50 insertions(+), 69 deletions(-) create mode 100644 .changeset/plenty-walls-clap.md diff --git a/.changeset/plenty-walls-clap.md b/.changeset/plenty-walls-clap.md new file mode 100644 index 00000000000..c461ed1102a --- /dev/null +++ b/.changeset/plenty-walls-clap.md @@ -0,0 +1,5 @@ +--- +'@whatwg-node/server': patch +--- + +Avoid polluting the original object in case of \`Object.create\` diff --git a/packages/server/src/utils.ts b/packages/server/src/utils.ts index c54b690ef69..014be77d2c8 100644 --- a/packages/server/src/utils.ts +++ b/packages/server/src/utils.ts @@ -527,75 +527,17 @@ export function isolateObject( if (waitUntilPromises == null) { return {} as TIsolatedObject; } - originalCtx = {} as TIsolatedObject; - } - const extraProps: Partial = {}; - const deletedProps = new Set(); - return new Proxy(originalCtx, { - get(originalCtx, prop) { - if (waitUntilPromises != null && prop === 'waitUntil') { - return function waitUntil(promise: Promise) { - waitUntilPromises.push(promise.catch(err => console.error(err))); - }; - } - const extraPropVal = (extraProps as any)[prop]; - if (extraPropVal != null) { - if (typeof extraPropVal === 'function') { - return extraPropVal.bind(extraProps); - } - return extraPropVal; - } - if (deletedProps.has(prop)) { - return undefined; - } - return (originalCtx as any)[prop]; - }, - set(_originalCtx, prop, value) { - (extraProps as any)[prop] = value; - return true; - }, - has(originalCtx, prop) { - if (waitUntilPromises != null && prop === 'waitUntil') { - return true; - } - if (deletedProps.has(prop)) { - return false; - } - if (prop in extraProps) { - return true; - } - return prop in originalCtx; - }, - defineProperty(_originalCtx, prop, descriptor) { - return Reflect.defineProperty(extraProps, prop, descriptor); - }, - deleteProperty(_originalCtx, prop) { - if (prop in extraProps) { - return Reflect.deleteProperty(extraProps, prop); - } - deletedProps.add(prop); - return true; - }, - ownKeys(originalCtx) { - const extraKeys = Reflect.ownKeys(extraProps); - const originalKeys = Reflect.ownKeys(originalCtx); - const deletedKeys = Array.from(deletedProps); - const allKeys = new Set( - extraKeys.concat(originalKeys.filter(keys => !deletedKeys.includes(keys))), - ); - if (waitUntilPromises != null) { - allKeys.add('waitUntil'); - } - return Array.from(allKeys); - }, - getOwnPropertyDescriptor(originalCtx, prop) { - if (prop in extraProps) { - return Reflect.getOwnPropertyDescriptor(extraProps, prop); - } - if (deletedProps.has(prop)) { - return undefined; - } - return Reflect.getOwnPropertyDescriptor(originalCtx, prop); + return { + waitUntil(promise: Promise) { + waitUntilPromises?.push(promise.catch(err => console.error(err))); + }, + } as TIsolatedObject; + } + return Object.create(originalCtx, { + waitUntil: { + value(promise: Promise) { + waitUntilPromises?.push(promise.catch(err => console.error(err))); + }, }, }); } diff --git a/packages/server/test/server-context.spec.ts b/packages/server/test/server-context.spec.ts index 57f0f4034c8..0c72137004a 100644 --- a/packages/server/test/server-context.spec.ts +++ b/packages/server/test/server-context.spec.ts @@ -31,6 +31,40 @@ describe('Server Context', () => { expect(res.status).toBe(200); expect(await res.json()).toEqual({ foo: 'bar', bar: 'baz' }); }); + it('Do not pollute the original object in case of `Object.create`', async () => { + const serverAdapter = createServerAdapter((_req, context0: any) => { + context0.i = 0; + const context1 = Object.create(context0); + context1.i = 1; + const context2 = Object.create(context0); + context2.i = 2; + return Response.json({ + i0: context0.i, + i1: context1.i, + i2: context2.i, + }); + }); + const res = await serverAdapter.fetch('http://localhost'); + const resJson = await res.json(); + expect(resJson).toEqual({ + i0: 0, + i1: 1, + i2: 2, + }); + }); + it('retains the prototype in case of `Object.create`', async () => { + class MyContext {} + const serverAdapter = createServerAdapter((_req, context0: MyContext) => { + return Response.json({ + isMyContext: context0 instanceof MyContext, + }); + }); + const res = await serverAdapter.fetch('http://localhost', new MyContext()); + const resJson = await res.json(); + expect(resJson).toEqual({ + isMyContext: true, + }); + }); }, { noLibCurl: true }, ); From 104012bc963134ef9c02cd2d0f3d86e836b379be Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Wed, 13 Nov 2024 14:04:44 +0100 Subject: [PATCH 2/4] test: add failing test for shared properties --- packages/server/test/utils.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 packages/server/test/utils.spec.ts diff --git a/packages/server/test/utils.spec.ts b/packages/server/test/utils.spec.ts new file mode 100644 index 00000000000..cb286819ad3 --- /dev/null +++ b/packages/server/test/utils.spec.ts @@ -0,0 +1,11 @@ +import { isolateObject } from '../src/utils'; + +describe('isolateObject', () => { + test('Object.create does not share property assignments', () => { + const origin = isolateObject({}); + const a = Object.create(origin); + const b = Object.create(origin); + a.a = 1; + expect(b.a).toEqual(undefined); + }); +}); From 631d899170a4be474752b774f7a4881855ee585a Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Wed, 13 Nov 2024 14:07:47 +0100 Subject: [PATCH 3/4] test: add failing test for shared properties (#1796) From 99dc2fae1faa31a67535d6376eb19f716c02dca8 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Wed, 13 Nov 2024 18:24:22 +0300 Subject: [PATCH 4/4] Fix --- packages/server/src/utils.ts | 106 ++++++++++++++++++++++++++--- packages/server/src/uwebsockets.ts | 16 +++-- 2 files changed, 105 insertions(+), 17 deletions(-) diff --git a/packages/server/src/utils.ts b/packages/server/src/utils.ts index 014be77d2c8..43457bf292a 100644 --- a/packages/server/src/utils.ts +++ b/packages/server/src/utils.ts @@ -527,17 +527,103 @@ export function isolateObject( if (waitUntilPromises == null) { return {} as TIsolatedObject; } - return { - waitUntil(promise: Promise) { - waitUntilPromises?.push(promise.catch(err => console.error(err))); - }, - } as TIsolatedObject; + originalCtx = {} as TIsolatedObject; + } + const extraPropsByReceiver = new WeakMap>(); + const deletedPropsByReceiver = new WeakMap>(); + function getExtraProps(receiver: TIsolatedObject): any { + let extraProps = extraPropsByReceiver.get(receiver); + if (!extraProps) { + extraProps = {}; + extraPropsByReceiver.set(receiver, extraProps); + } + return extraProps; } - return Object.create(originalCtx, { - waitUntil: { - value(promise: Promise) { - waitUntilPromises?.push(promise.catch(err => console.error(err))); - }, + function getDeletedProps(receiver: TIsolatedObject) { + let deletedProps = deletedPropsByReceiver.get(receiver); + if (!deletedProps) { + deletedProps = new Set(); + deletedPropsByReceiver.set(receiver, deletedProps); + } + return deletedProps; + } + return new Proxy(originalCtx, { + get(originalCtx, prop, receiver) { + if (waitUntilPromises != null && prop === 'waitUntil') { + return function waitUntil(promise: Promise) { + waitUntilPromises.push(promise.catch(err => console.error(err))); + }; + } + const extraProps = getExtraProps(receiver); + const extraPropVal = extraProps[prop]; + if (extraPropVal != null) { + if (typeof extraPropVal === 'function') { + return extraPropVal.bind(extraProps); + } + return extraPropVal; + } + const deletedProps = getDeletedProps(receiver); + if (deletedProps.has(prop)) { + return undefined; + } + return (originalCtx as any)[prop]; + }, + set(_originalCtx, prop, value, receiver) { + const extraProps = getExtraProps(receiver); + extraProps[prop] = value; + return true; + }, + has(originalCtx, prop) { + if (waitUntilPromises != null && prop === 'waitUntil') { + return true; + } + const deletedProps = getDeletedProps(originalCtx); + if (deletedProps.has(prop)) { + return false; + } + const extraProps = getExtraProps(originalCtx); + if (prop in extraProps) { + return true; + } + return prop in originalCtx; + }, + defineProperty(originalCtx, prop, descriptor) { + const extraProps = getExtraProps(originalCtx); + return Reflect.defineProperty(extraProps, prop, descriptor); + }, + deleteProperty(originalCtx, prop) { + const extraProps = getExtraProps(originalCtx); + if (prop in extraProps) { + return Reflect.deleteProperty(extraProps, prop); + } + const deletedProps = getDeletedProps(originalCtx); + deletedProps.add(prop); + return true; + }, + ownKeys(originalCtx) { + const extraProps = getExtraProps(originalCtx); + const extraKeys = Reflect.ownKeys(extraProps); + const originalKeys = Reflect.ownKeys(originalCtx); + const deletedProps = getDeletedProps(originalCtx); + const deletedKeys = Array.from(deletedProps); + const allKeys = new Set( + extraKeys.concat(originalKeys.filter(keys => !deletedKeys.includes(keys))), + ); + if (waitUntilPromises != null) { + allKeys.add('waitUntil'); + } + return Array.from(allKeys); + }, + getOwnPropertyDescriptor(originalCtx, prop) { + const extraProps = getExtraProps(originalCtx); + if (prop in extraProps) { + return Reflect.getOwnPropertyDescriptor(extraProps, prop); + } + const deletedProps = getDeletedProps(originalCtx); + if (deletedProps.has(prop)) { + return undefined; + } + return Reflect.getOwnPropertyDescriptor(originalCtx, prop); }, }); } diff --git a/packages/server/src/uwebsockets.ts b/packages/server/src/uwebsockets.ts index 696876eb049..cc07f8745b0 100644 --- a/packages/server/src/uwebsockets.ts +++ b/packages/server/src/uwebsockets.ts @@ -187,10 +187,14 @@ export function getRequestFromUWSRequest({ req, res, fetchAPI, signal }: GetRequ export function createWritableFromUWS(uwsResponse: UWSResponse, fetchAPI: FetchAPI) { return new fetchAPI.WritableStream({ write(chunk) { - uwsResponse.write(chunk); + uwsResponse.cork(() => { + uwsResponse.write(chunk); + }); }, close() { - uwsResponse.end(); + uwsResponse.cork(() => { + uwsResponse.end(); + }); }, }); } @@ -229,13 +233,11 @@ export function sendResponseToUwsOpts( } if (bufferOfRes) { uwsResponse.end(bufferOfRes); + } else if (!fetchResponse.body) { + uwsResponse.end(); } }); - if (bufferOfRes) { - return; - } - if (!fetchResponse.body) { - uwsResponse.end(); + if (bufferOfRes || !fetchResponse.body) { return; } signal.addEventListener('abort', () => {