Avoid polluting the original object in case of Object.create (#1799)

Co-authored-by: Laurin Quast <[email protected]>

.changeset/plenty-walls-clap.md

@@ -0,0 +1,5 @@
+---
+'@whatwg-node/server': patch
+---
+
+Avoid polluting the original object in case of \`Object.create\`

packages/server/src/types.ts

@@ -131,7 +131,7 @@ export type ServerAdapterNodeContext = {
   res: NodeResponse;
 };
 
-export type WaitUntilFn = (promise: Promise<unknown>) => void;
+export type WaitUntilFn = (promise: Promise<void> | void) => void;
 
 export type FetchAPI = ReturnType<typeof import('@whatwg-node/fetch').createFetch>;
 

packages/server/src/utils.ts

@@ -527,77 +527,21 @@ export function isolateObject<TIsolatedObject extends object>(
     if (waitUntilPromises == null) {
       return {} as TIsolatedObject;
     }
-    originalCtx = {} as TIsolatedObject;
-  }
-  const extraProps: Partial<TIsolatedObject> = {};
-  const deletedProps = new Set<string | symbol>();
-  return new Proxy(originalCtx, {
-    get(originalCtx, prop) {
-      if (waitUntilPromises != null && prop === 'waitUntil') {
-        return function waitUntil(promise: Promise<unknown>) {
-          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<unknown>) {
+        waitUntilPromises.push(promise.catch(err => console.error(err)));
+      },
+    } as TIsolatedObject;
+  }
+  return completeAssign(
+    Object.create(originalCtx),
+    {
+      waitUntil(promise: Promise<unknown>) {
+        waitUntilPromises?.push(promise.catch(err => console.error(err)));
+      },
     },
-  });
+    originalCtx,
+  );
 }
 
 export interface DeferredPromise<T = void> {

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', () => {

packages/server/test/fetch-event-listener.spec.ts

@@ -1,6 +1,17 @@
 import { CustomEvent } from '@whatwg-node/events';
+import { FetchEvent } from '@whatwg-node/server';
 import { runTestsForEachFetchImpl } from './test-fetch.js';
 
+class PonyfillFetchEvent extends CustomEvent<{}> implements FetchEvent {
+  constructor(
+    public request: Request,
+    public respondWith: FetchEvent['respondWith'],
+    public waitUntil: FetchEvent['waitUntil'],
+  ) {
+    super('fetch');
+  }
+}
+
 describe('FetchEvent listener', () => {
   runTestsForEachFetchImpl(
     (_, { createServerAdapter, fetchAPI: { Request, Response } }) => {
@@ -10,12 +21,12 @@ describe('FetchEvent listener', () => {
         const adapter = createServerAdapter(() => response$);
         const respondWith = jest.fn();
         const waitUntil = jest.fn();
-        const fetchEvent = Object.assign(new CustomEvent('fetch'), {
-          request: new Request('http://localhost:8080'),
+        const fetchEvent = new PonyfillFetchEvent(
+          new Request('http://localhost:8080'),
           respondWith,
           waitUntil,
-        });
-        const returnValue = await adapter(fetchEvent);
+        );
+        const returnValue = adapter(fetchEvent);
         expect(returnValue).toBeUndefined();
         const returnedResponse = await respondWith.mock.calls[0][0];
         expect(returnedResponse).toBe(response);
@@ -25,12 +36,12 @@ describe('FetchEvent listener', () => {
         const adapter = createServerAdapter(handleRequest);
         const respondWith = jest.fn();
         const waitUntil = jest.fn();
-        const fetchEvent = Object.assign(new CustomEvent('fetch'), {
-          request: new Request('http://localhost:8080'),
+        const fetchEvent = new PonyfillFetchEvent(
+          new Request('http://localhost:8080'),
           respondWith,
           waitUntil,
-        });
-        await adapter(fetchEvent);
+        );
+        adapter(fetchEvent);
         expect(handleRequest).toHaveBeenCalledWith(fetchEvent.request, fetchEvent);
       });
       it('should accept additional parameters as server context', async () => {
@@ -40,13 +51,13 @@ describe('FetchEvent listener', () => {
         }>(handleRequest);
         const respondWith = jest.fn();
         const waitUntil = jest.fn();
-        const fetchEvent = Object.assign(new CustomEvent('fetch'), {
-          request: new Request('http://localhost:8080'),
+        const fetchEvent = new PonyfillFetchEvent(
+          new Request('http://localhost:8080'),
           respondWith,
           waitUntil,
-        });
+        );
         const additionalCtx = { foo: 'bar' };
-        await adapter(fetchEvent, additionalCtx);
+        adapter(fetchEvent, additionalCtx);
         expect(handleRequest).toHaveBeenCalledWith(
           fetchEvent.request,
           expect.objectContaining(additionalCtx),

packages/server/test/server-context.spec.ts

@@ -31,6 +31,58 @@ describe('Server Context', () => {
         expect(res.status).toBe(200);
         expect(await res.json()).toEqual({ foo: 'bar', bar: 'baz' });
       });
+      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,
+        });
+      });
+      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('Do not pollute the original object in case of `Object.create` and `Object.defineProperty`', async () => {
+        const serverAdapter = createServerAdapter((_req, context0: any) => {
+          const context1 = Object.create(context0);
+          Object.defineProperty(context1, 'i', { value: 1, configurable: true });
+          const context2 = Object.create(context0);
+          Object.defineProperty(context2, 'i', { value: 2, configurable: true });
+          return Response.json({
+            i1: context1.i,
+            i2: context2.i,
+          });
+        });
+        const res = await serverAdapter.fetch('http://localhost');
+        const resJson = await res.json();
+        expect(resJson).toEqual({
+          i1: 1,
+          i2: 2,
+        });
+      });
     },
     { noLibCurl: true },
   );

packages/server/test/useErrorHandling.spec.ts

@@ -23,7 +23,9 @@ describe('useErrorHandling', () => {
         expect(response.statusText).toBe(errRes.statusText);
         const text = await response.text();
         expect(text).toHaveLength(0);
-        expect(errorHandler).toHaveBeenCalledWith(new Error('Unexpected error'), request, {});
+        expect(errorHandler).toHaveBeenCalledWith(new Error('Unexpected error'), request, {
+          waitUntil: expect.any(Function),
+        });
       });
     },
     { noLibCurl: true },

packages/server/test/utils.spec.ts

@@ -0,0 +1,54 @@
+import { isolateObject } from '../src/utils';
+
+describe('isolateObject', () => {
+  describe('Object.create', () => {
+    test('property assignments', () => {
+      const origin = isolateObject({});
+      const a = Object.create(origin);
+      const b = Object.create(origin);
+      a.a = 1;
+      expect(b.a).toEqual(undefined);
+    });
+    test('property assignments with defineProperty', () => {
+      const origin = isolateObject({});
+      const a = Object.create(origin);
+      const b = Object.create(origin);
+      Object.defineProperty(a, 'a', { value: 1 });
+      expect(b.a).toEqual(undefined);
+    });
+    test('property deletions', () => {
+      const origin = isolateObject({});
+      const a = Object.create(origin);
+      const b = Object.create(origin);
+      b.a = 2;
+      a.a = 1;
+      delete a.a;
+      expect(b.a).toEqual(2);
+    });
+    test('ownKeys', () => {
+      const origin = isolateObject({});
+      const a = Object.create(origin);
+      const b = Object.create(origin);
+      a.a = 1;
+      expect(Object.keys(a)).toEqual(['a']);
+      expect(Object.keys(b)).toEqual([]);
+    });
+    test('hasOwnProperty', () => {
+      const origin = isolateObject({});
+      const a = Object.create(origin);
+      const b = Object.create(origin);
+      a.a = 1;
+      expect(a.hasOwnProperty('a')).toEqual(true);
+      expect(b.hasOwnProperty('a')).toEqual(false);
+    });
+    test('getOwnPropertyDescriptor', () => {
+      const origin = isolateObject({});
+      const a = Object.create(origin);
+      const b = Object.create(origin);
+      a.a = 1;
+      const desc = Object.getOwnPropertyDescriptor(a, 'a');
+      expect(desc?.value).toEqual(1);
+      expect(Object.getOwnPropertyDescriptor(b, 'a')).toEqual(undefined);
+    });
+  });
+});