From 5bc29974804273df7a8b92c024ffae1f794a0205 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Sat, 18 Nov 2023 22:50:47 -0800 Subject: [PATCH] Micro-optimization: omit instead of delete headers --- src/bun.js/api/server.zig | 22 ++++++++++---- src/bun.js/bindings/bindings.cpp | 50 +++++++++++++++++++++++++------- src/bun.js/bindings/bindings.zig | 10 +++++-- src/bun.js/bindings/headers.h | 3 +- src/bun.js/bindings/headers.zig | 1 - test/js/bun/http/serve.test.ts | 46 +++++++++++++++++++++++++++++ 6 files changed, 111 insertions(+), 21 deletions(-) diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index 5e9abc282a..e9b459b645 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -1812,16 +1812,28 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp server.request_pool_allocator.put(this); } + const headers_to_exclude = brk: { + if (ssl_enabled) { + break :brk [_]JSC.FetchHeaders.HTTPHeaderName{ + .ContentLength, + .TransferEncoding, + }; + } + + break :brk [_]JSC.FetchHeaders.HTTPHeaderName{ + .ContentLength, + .TransferEncoding, + .StrictTransportSecurity, + }; + }; + fn writeHeaders( this: *RequestContext, headers: *JSC.FetchHeaders, ) void { ctxLog("writeHeaders", .{}); - headers.fastRemove(.ContentLength); - headers.fastRemove(.TransferEncoding); - if (!ssl_enabled) headers.fastRemove(.StrictTransportSecurity); if (this.resp) |resp| { - headers.toUWSResponse(ssl_enabled, resp); + headers.toUWSResponse(ssl_enabled, resp, &headers_to_exclude); } } @@ -5054,7 +5066,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp // TODO: should we cork? // we must write the status first so that 200 OK isn't written resp.writeStatus("101 Switching Protocols"); - fetch_headers_to_use.toUWSResponse(comptime ssl_enabled, resp); + fetch_headers_to_use.toUWSResponse(comptime ssl_enabled, resp, &.{}); } } } diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index aef96baad9..4ff3775388 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -99,7 +99,7 @@ #include "JavaScriptCore/InternalFieldTuple.h" template -static void copyToUWS(WebCore::FetchHeaders* headers, UWSResponse* res) +static void copyToUWS(WebCore::FetchHeaders* headers, UWSResponse* res, const std::span exclude) { auto& internalHeaders = headers->internalHeaders(); @@ -107,13 +107,43 @@ static void copyToUWS(WebCore::FetchHeaders* headers, UWSResponse* res) res->writeHeader(std::string_view("set-cookie", 10), std::string_view(value.is8Bit() ? reinterpret_cast(value.characters8()) : value.utf8().data(), value.length())); } - for (const auto& header : internalHeaders.commonHeaders()) { - const auto& name = WebCore::httpHeaderNameString(header.key); - const auto& value = header.value; + bool anyToExclude = false; + for (auto excludeName : exclude) { + if (internalHeaders.contains(excludeName)) { + anyToExclude = true; + break; + } + } - res->writeHeader( - std::string_view(name.is8Bit() ? reinterpret_cast(name.characters8()) : name.utf8().data(), name.length()), - std::string_view(value.is8Bit() ? reinterpret_cast(value.characters8()) : value.utf8().data(), value.length())); + if (!anyToExclude) { + for (const auto& header : internalHeaders.commonHeaders()) { + const auto& name = WebCore::httpHeaderNameString(header.key); + const auto& value = header.value; + + res->writeHeader( + std::string_view(name.is8Bit() ? reinterpret_cast(name.characters8()) : name.utf8().data(), name.length()), + std::string_view(value.is8Bit() ? reinterpret_cast(value.characters8()) : value.utf8().data(), value.length())); + } + } else { + const auto shouldSkip = [exclude](const WebCore::HTTPHeaderName name) -> bool { + for (auto excludeName : exclude) { + if (name == excludeName) { + return true; + } + } + return false; + }; + + for (const auto& header : internalHeaders.commonHeaders()) { + if (!shouldSkip(header.key)) { + const auto& name = WebCore::httpHeaderNameString(header.key); + const auto& value = header.value; + + res->writeHeader( + std::string_view(name.is8Bit() ? reinterpret_cast(name.characters8()) : name.utf8().data(), name.length()), + std::string_view(value.is8Bit() ? reinterpret_cast(value.characters8()) : value.utf8().data(), value.length())); + } + } } for (auto& header : internalHeaders.uncommonHeaders()) { @@ -1072,12 +1102,12 @@ bool WebCore__FetchHeaders__isEmpty(WebCore__FetchHeaders* arg0) return arg0->size() == 0; } -void WebCore__FetchHeaders__toUWSResponse(WebCore__FetchHeaders* arg0, bool is_ssl, void* arg2) +void WebCore__FetchHeaders__toUWSResponse(WebCore__FetchHeaders* arg0, bool is_ssl, void* arg2, const WebCore::HTTPHeaderName* exclusions_list, size_t exclusions_list_length) { if (is_ssl) { - copyToUWS>(arg0, reinterpret_cast*>(arg2)); + copyToUWS>(arg0, reinterpret_cast*>(arg2), std::span { exclusions_list, exclusions_list_length }); } else { - copyToUWS>(arg0, reinterpret_cast*>(arg2)); + copyToUWS>(arg0, reinterpret_cast*>(arg2), std::span { exclusions_list, exclusions_list_length }); } } diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig index 2d528a774a..cdc70ba6ad 100644 --- a/src/bun.js/bindings/bindings.zig +++ b/src/bun.js/bindings/bindings.zig @@ -1205,16 +1205,21 @@ pub const FetchHeaders = opaque { }); } + extern fn WebCore__FetchHeaders__toUWSResponse(arg0: ?*FetchHeaders, arg1: bool, arg2: ?*anyopaque, exclude_ptr: [*]const HTTPHeaderName, exclude_len: usize) void; + pub fn toUWSResponse( headers: *FetchHeaders, is_ssl: bool, uws_response: *anyopaque, + exclusions: []const HTTPHeaderName, ) void { - return shim.cppFn("toUWSResponse", .{ + return WebCore__FetchHeaders__toUWSResponse( headers, is_ssl, uws_response, - }); + exclusions.ptr, + exclusions.len, + ); } const PicoHeaders = extern struct { @@ -1575,7 +1580,6 @@ pub const FetchHeaders = opaque { "put_", "remove", "toJS", - "toUWSResponse", "isEmpty", }; }; diff --git a/src/bun.js/bindings/headers.h b/src/bun.js/bindings/headers.h index a4c91cd968..cbe07e1f9c 100644 --- a/src/bun.js/bindings/headers.h +++ b/src/bun.js/bindings/headers.h @@ -200,8 +200,7 @@ CPP_DECL bool WebCore__FetchHeaders__has(WebCore__FetchHeaders* arg0, const ZigS CPP_DECL bool WebCore__FetchHeaders__isEmpty(WebCore__FetchHeaders* arg0); CPP_DECL void WebCore__FetchHeaders__put_(WebCore__FetchHeaders* arg0, const ZigString* arg1, const ZigString* arg2, JSC__JSGlobalObject* arg3); CPP_DECL void WebCore__FetchHeaders__remove(WebCore__FetchHeaders* arg0, const ZigString* arg1, JSC__JSGlobalObject* arg2); -CPP_DECL JSC__JSValue WebCore__FetchHeaders__toJS(WebCore__FetchHeaders* arg0, JSC__JSGlobalObject* arg1); -CPP_DECL void WebCore__FetchHeaders__toUWSResponse(WebCore__FetchHeaders* arg0, bool arg1, void* arg2); +CPP_DECL JSC__JSValue WebCore__FetchHeaders__toJS(WebCore__FetchHeaders* arg0, JSC__JSGlobalObject* arg1);; CPP_DECL JSC__JSValue SystemError__toErrorInstance(const SystemError* arg0, JSC__JSGlobalObject* arg1); #pragma mark - JSC::JSCell diff --git a/src/bun.js/bindings/headers.zig b/src/bun.js/bindings/headers.zig index a696e86b1a..7fa6535172 100644 --- a/src/bun.js/bindings/headers.zig +++ b/src/bun.js/bindings/headers.zig @@ -129,7 +129,6 @@ pub extern fn WebCore__FetchHeaders__isEmpty(arg0: ?*bindings.FetchHeaders) bool pub extern fn WebCore__FetchHeaders__put_(arg0: ?*bindings.FetchHeaders, arg1: [*c]const ZigString, arg2: [*c]const ZigString, arg3: *bindings.JSGlobalObject) void; pub extern fn WebCore__FetchHeaders__remove(arg0: ?*bindings.FetchHeaders, arg1: [*c]const ZigString, arg2: *bindings.JSGlobalObject) void; pub extern fn WebCore__FetchHeaders__toJS(arg0: ?*bindings.FetchHeaders, arg1: *bindings.JSGlobalObject) JSC__JSValue; -pub extern fn WebCore__FetchHeaders__toUWSResponse(arg0: ?*bindings.FetchHeaders, arg1: bool, arg2: ?*anyopaque) void; pub extern fn SystemError__toErrorInstance(arg0: [*c]const SystemError, arg1: *bindings.JSGlobalObject) JSC__JSValue; pub extern fn JSC__JSCell__getObject(arg0: [*c]bindings.JSCell) [*c]bindings.JSObject; pub extern fn JSC__JSCell__getType(arg0: [*c]bindings.JSCell) u8; diff --git a/test/js/bun/http/serve.test.ts b/test/js/bun/http/serve.test.ts index aef748c0d9..4ae65594d9 100644 --- a/test/js/bun/http/serve.test.ts +++ b/test/js/bun/http/serve.test.ts @@ -1304,3 +1304,49 @@ it("should response with HTTP 413 when request body is larger than maxRequestBod server.stop(true); }); + +describe("strips headers", () => { + const toStrip = ["Strict-Transport-Security", "Content-Length", "Transfer-Encoding"]; + + // all and any permutations of the above + // 3! = 6 + for (let i = 0; i < toStrip.length; i++) { + for (let j = 0; j < toStrip.length; j++) { + for (let k = 0; k < toStrip.length; k++) { + const [first, second, third] = [toStrip[i], toStrip[j], toStrip[k]]; + it(`${first}, ${second}, ${third}`, async () => { + const server = Bun.serve({ + port: 0, + fetch(req, server) { + return new Response("OK", { + headers: { + "X-Test": "123", + "User-Agent": "Custom", + [first]: "123", + [second]: "123", + [third]: "123", + }, + }); + }, + }); + + const resp = await fetch(server.url, { + method: "GET", + }); + + for (const header of [first, second, third]) { + if (header === "Content-Length") { + expect(resp.headers.get(header)).toBe("2"); + } else { + expect(resp.headers.get(header)).toBeNull(); + } + } + expect(resp.headers.get("X-Test")).toBe("123"); + expect(resp.headers.get("User-Agent")).toBe("Custom"); + + server.stop(true); + }); + } + } + } +});