From ce07eca2a2ed5d6fe3545ee86b2ff59c490a1b21 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Fri, 10 May 2024 00:18:48 -0700 Subject: [PATCH] Support overriding `headers` property in `Request` subclass (#10969) --- src/bun.js/webcore/response.zig | 59 +++++++++++++++++++- test/js/web/request/request-subclass.test.ts | 52 ++++++++++++++++- 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/bun.js/webcore/response.zig b/src/bun.js/webcore/response.zig index 5ad6abf097..2be5bcf7b1 100644 --- a/src/bun.js/webcore/response.zig +++ b/src/bun.js/webcore/response.zig @@ -1989,10 +1989,41 @@ pub const Fetch = struct { } } - // TODO: remove second isDisturbed check in useAsAnyBlob - body = request.body.value.useAsAnyBlob(); + // Support headers getter on subclass + // + // class MyRequest extends Request { + // get headers() { + // return {a: "1"}; + // } + // } + // + // fetch(request) + var fetch_headers_to_deref: ?*JSC.FetchHeaders = null; + defer { + if (fetch_headers_to_deref) |fetch_headers| { + fetch_headers.deref(); + } + } - if (request.headers) |head| { + if (get_fetch_headers: { + if (can_use_fast_getters) + break :get_fetch_headers request.headers; + + if (first_arg.fastGet(globalThis, .headers)) |headers_value| { + // Faster path: existing FetchHeaders object: + if (FetchHeaders.cast(headers_value)) |fetch_headers| { + break :get_fetch_headers fetch_headers; + } + + // Slow path: create a new FetchHeaders: + if (FetchHeaders.createFromJS(globalThis, headers_value)) |fetch_headers| { + fetch_headers_to_deref = fetch_headers; + break :get_fetch_headers fetch_headers; + } + } + + break :get_fetch_headers null; + }) |head| { if (head.fastGet(JSC.FetchHeaders.HTTPHeaderName.Host)) |_hostname| { if (hostname) |host| { allocator.free(host); @@ -2001,6 +2032,28 @@ pub const Fetch = struct { } headers = Headers.from(head, allocator, .{ .body = &body }) catch unreachable; } + + // Creating headers can throw. + if (globalThis.hasException()) { + if (hostname) |host| { + allocator.free(host); + hostname = null; + } + return .zero; + } + + // TODO: remove second isDisturbed check in useAsAnyBlob + body = request.body.value.useAsAnyBlob(); + + // Assume that useAsAnyBlob() has already thrown an error if it was going to. + if (globalThis.hasException()) { + if (hostname) |host| { + allocator.free(host); + hostname = null; + } + return .zero; + } + if (request.signal) |signal_| { _ = signal_.ref(); signal = signal_; diff --git a/test/js/web/request/request-subclass.test.ts b/test/js/web/request/request-subclass.test.ts index d738d67f9b..86d393a95a 100644 --- a/test/js/web/request/request-subclass.test.ts +++ b/test/js/web/request/request-subclass.test.ts @@ -12,6 +12,14 @@ test("fetch() calls request.method & request.url getters on subclass", async () return actual_url; }, }); + + Object.defineProperty(this, "headers", { + get() { + return { + "X-My-Header": "123", + }; + }, + }); } // @ts-ignore @@ -22,14 +30,56 @@ test("fetch() calls request.method & request.url getters on subclass", async () const server = Bun.serve({ fetch(req) { - return new Response(req.method); + return new Response(req.method, { headers: req.headers }); }, port: 0, }); const request = new MyRequest("https://example.com", {}, server.url.href); + expect(request.method).toBe("POST"); const response = await fetch(request); expect(await response.text()).toBe("POST"); + expect(response.headers.get("X-My-Header")).toBe("123"); server.stop(true); }); + +test("fetch() with subclass containing invalid HTTP headers throws without crashing", async () => { + class MyRequest extends Request { + constructor(input: string, init?: RequestInit, actual_url?: string) { + super(input, init); + + Object.defineProperty(this, "url", { + get() { + return actual_url; + }, + }); + + Object.defineProperty(this, "headers", { + get() { + return { + "[I am not a valid header]!": "123", + }; + }, + }); + } + + // @ts-ignore + get method() { + return "POST"; + } + } + + const request = new MyRequest("https://example.com", {}, "https://example.com"); + expect(request.method).toBe("POST"); + expect(() => fetch(request)).toThrow("Invalid header name"); + + // quick gc test + for (let i = 0; i < 1e4; i++) { + try { + fetch(request); + } catch (e) {} + } + + expect(() => fetch(request)).toThrow("Invalid header name"); +});