diff --git a/src/jsc/bindings/NodeHTTP.cpp b/src/jsc/bindings/NodeHTTP.cpp index 9baf0b8d436c..b0f7bbe88495 100644 --- a/src/jsc/bindings/NodeHTTP.cpp +++ b/src/jsc/bindings/NodeHTTP.cpp @@ -778,7 +778,9 @@ static void writeFetchHeadersToUWSResponse(WebCore::FetchHeaders& headers, uWS:: auto& internalHeaders = headers.internalHeaders(); for (auto& value : internalHeaders.getSetCookieHeaders()) { - + if (value.isEmpty()) { + continue; + } if (value.is8Bit()) { const auto valueSpan = value.span8(); res->writeHeader(std::string_view("set-cookie", 10), std::string_view(reinterpret_cast(valueSpan.data()), valueSpan.size())); @@ -795,6 +797,15 @@ static void writeFetchHeadersToUWSResponse(WebCore::FetchHeaders& headers, uWS:: const auto& name = WebCore::httpHeaderNameString(header.key); const auto& value = header.value; + // FetchHeaders strips leading/trailing HTTP whitespace, so an empty value here + // covers `""` and whitespace-only. Treat it as absent: don't write the line and + // don't set the wrote-this-header state bits, so the matching auto-header (Date, + // Content-Type from render_metadata) is emitted exactly once instead of alongside + // an empty duplicate. + if (value.isEmpty()) { + continue; + } + // We have to tell uWS not to automatically insert a TransferEncoding or Date header. // Otherwise, you get this when using Fastify; // @@ -843,7 +854,9 @@ static void writeFetchHeadersToUWSResponse(WebCore::FetchHeaders& headers, uWS:: for (auto& header : internalHeaders.uncommonHeaders()) { const auto& name = header.key; const auto& value = header.value; - + if (value.isEmpty()) { + continue; + } writeResponseHeader(res, name, value); } } @@ -1523,6 +1536,9 @@ static void writeFetchHeadersToH3Response(WebCore::FetchHeaders& headers, uWS::H }; for (auto& value : internalHeaders.getSetCookieHeaders()) { + if (value.isEmpty()) { + continue; + } if (value.is8Bit()) { const auto s = value.span8(); res->writeHeader(std::string_view("set-cookie", 10), std::string_view(reinterpret_cast(s.data()), s.size())); @@ -1533,6 +1549,9 @@ static void writeFetchHeadersToH3Response(WebCore::FetchHeaders& headers, uWS::H } for (const auto& header : internalHeaders.commonHeaders()) { + if (header.value.isEmpty()) { + continue; + } if (header.key == WebCore::HTTPHeaderName::ContentLength) { if (!(data->state & uWS::Http3ResponseData::HTTP_WROTE_CONTENT_LENGTH_HEADER)) { data->state |= uWS::Http3ResponseData::HTTP_WROTE_CONTENT_LENGTH_HEADER; @@ -1548,6 +1567,9 @@ static void writeFetchHeadersToH3Response(WebCore::FetchHeaders& headers, uWS::H } for (auto& header : internalHeaders.uncommonHeaders()) { + if (header.value.isEmpty()) { + continue; + } writeOne(header.key, header.value); } } diff --git a/src/runtime/server/FileRoute.rs b/src/runtime/server/FileRoute.rs index 88691374f5f4..ac6e363dea84 100644 --- a/src/runtime/server/FileRoute.rs +++ b/src/runtime/server/FileRoute.rs @@ -120,10 +120,12 @@ impl FileRoute { bun_core::heap::into_raw(Box::new(FileRoute { ref_count: Cell::new(1), server: Cell::new(opts.server), - has_last_modified_header: headers.get(b"last-modified").is_some(), - has_content_length_header: headers.get(b"content-length").is_some(), - has_content_range_header: headers.get(b"content-range").is_some(), - has_date_header: headers.get(b"date").is_some(), + has_last_modified_header: headers.get(b"last-modified").is_some_and(|v| !v.is_empty()), + has_content_length_header: headers + .get(b"content-length") + .is_some_and(|v| !v.is_empty()), + has_content_range_header: headers.get(b"content-range").is_some_and(|v| !v.is_empty()), + has_date_header: headers.get(b"date").is_some_and(|v| !v.is_empty()), blob, headers, status_code: opts.status_code, @@ -180,10 +182,16 @@ impl FileRoute { return Ok(Some(bun_core::heap::into_raw(Box::new(FileRoute { ref_count: Cell::new(1), server: Cell::new(None), - has_last_modified_header: headers.get(b"last-modified").is_some(), - has_content_length_header: headers.get(b"content-length").is_some(), - has_content_range_header: headers.get(b"content-range").is_some(), - has_date_header: headers.get(b"date").is_some(), + has_last_modified_header: headers + .get(b"last-modified") + .is_some_and(|v| !v.is_empty()), + has_content_length_header: headers + .get(b"content-length") + .is_some_and(|v| !v.is_empty()), + has_content_range_header: headers + .get(b"content-range") + .is_some_and(|v| !v.is_empty()), + has_date_header: headers.get(b"date").is_some_and(|v| !v.is_empty()), blob, headers, status_code, @@ -230,6 +238,9 @@ impl FileRoute { AnyResponse::SSL(s) => { let s = bun_opaque::opaque_deref_mut(s); for (name, value) in names.iter().zip(values) { + if value.length == 0 { + continue; + } s.write_header(sp_slice(*name, buf), sp_slice(*value, buf)); } if let Some(srv) = self.server.get() { @@ -241,6 +252,9 @@ impl FileRoute { AnyResponse::TCP(s) => { let s = bun_opaque::opaque_deref_mut(s); for (name, value) in names.iter().zip(values) { + if value.length == 0 { + continue; + } s.write_header(sp_slice(*name, buf), sp_slice(*value, buf)); } if let Some(srv) = self.server.get() { @@ -252,6 +266,9 @@ impl FileRoute { AnyResponse::H3(s) => { let s = bun_opaque::opaque_deref_mut(s); for (name, value) in names.iter().zip(values) { + if value.length == 0 { + continue; + } s.write_header(sp_slice(*name, buf), sp_slice(*value, buf)); } // tag == .H3 → no alt-svc header diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index d44f8b4afa0f..2b765bc56e29 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -1831,7 +1831,7 @@ where let user_handles_range = if let Some(r) = self.response_weakref.get() { r.status_code() != 200 || r.get_init_headers_mut() - .map(|h| h.fast_has(jsc::HTTPHeaderName::ContentRange)) + .map(|h| h.fast_get(jsc::HTTPHeaderName::ContentRange).is_some()) .unwrap_or(false) } else { false @@ -3630,8 +3630,12 @@ where let mut has_content_disposition = false; let mut has_content_range = false; if let Some(mut headers_) = response.swap_init_headers() { - has_content_disposition = headers_.fast_has(jsc::HTTPHeaderName::ContentDisposition); - has_content_range = headers_.fast_has(jsc::HTTPHeaderName::ContentRange); + has_content_disposition = headers_ + .fast_get(jsc::HTTPHeaderName::ContentDisposition) + .is_some(); + has_content_range = headers_ + .fast_get(jsc::HTTPHeaderName::ContentRange) + .is_some(); // For .slice()-driven ranges, only promote to 206 if the user // also set Content-Range (preserves the old contract). For an // incoming Range: header (sendfile.total > 0) we always 206. diff --git a/src/runtime/server/StaticRoute.rs b/src/runtime/server/StaticRoute.rs index 36c2d66b857e..b2b2fe1b46e3 100644 --- a/src/runtime/server/StaticRoute.rs +++ b/src/runtime/server/StaticRoute.rs @@ -102,7 +102,7 @@ impl StaticRoute { } let cached_blob_size = blob.size(); - let has_date = headers.get(b"date").is_some(); + let has_date = headers.get(b"date").is_some_and(|v| !v.is_empty()); bun_core::heap::into_raw(Box::new(StaticRoute { ref_count: Cell::new(1), blob, @@ -222,13 +222,20 @@ impl StaticRoute { // Consuming the body left a plain `Blob` behind, which no longer implies // the `text/plain` a string body carried. Record it on the response's own // headers so re-registering the same `Response` serves the same type. + // `fast_get` (unlike `put_default`'s `fast_has`) treats an empty value as + // absent, so an explicit `content-type: ""` is overwritten here — a + // Bun-specific choice for the `static:` route API, not at Fetch-spec + // construction time. if was_string { - let text_mime = bun_http_types::MimeType::TEXT; - response.get_or_create_headers(global_this)?.put_default( - HTTPHeaderName::ContentType, - &bun_core::String::ascii(text_mime.value.as_ref()), - global_this, - )?; + let h = response.get_or_create_headers(global_this)?; + if h.fast_get(HTTPHeaderName::ContentType).is_none() { + let text_mime = bun_http_types::MimeType::TEXT; + h.put( + HTTPHeaderName::ContentType, + &bun_core::String::ascii(text_mime.value.as_ref()), + global_this, + )?; + } } let mut headers: Headers = bun_http_jsc::headers_jsc::from_fetch_headers( @@ -244,7 +251,7 @@ impl StaticRoute { } let cached_blob_size = blob.size(); - let has_date = headers.get(b"date").is_some(); + let has_date = headers.get(b"date").is_some_and(|v| !v.is_empty()); return Ok(Some(bun_core::heap::into_raw(Box::new(StaticRoute { ref_count: Cell::new(1), blob, @@ -494,6 +501,9 @@ impl StaticRoute { debug_assert_eq!(names.len(), values.len()); for (name, value) in names.iter().zip(values) { + if value.length == 0 { + continue; + } resp.write_header( &buf[name.offset as usize..][..name.length as usize], &buf[value.offset as usize..][..value.length as usize], diff --git a/test/js/bun/http/bun-serve-headers.test.ts b/test/js/bun/http/bun-serve-headers.test.ts index 46ee49cc1756..eed50d83d934 100644 --- a/test/js/bun/http/bun-serve-headers.test.ts +++ b/test/js/bun/http/bun-serve-headers.test.ts @@ -1,6 +1,183 @@ import { describe, expect, test } from "bun:test"; +import { tempDir } from "harness"; import { once } from "node:events"; import * as net from "node:net"; +import * as path from "node:path"; + +// An empty (or whitespace-only) header value must be treated as absent for the +// well-known headers Bun fills in automatically, so the response head carries +// exactly one Content-Type and exactly one Date (RFC 9110 forbids duplicate +// Content-Type; an origin server MUST send a valid Date). +describe("empty header value does not duplicate auto-headers", () => { + async function readHead(port: number): Promise { + const socket = net.connect(port, "127.0.0.1"); + try { + await once(socket, "connect"); + socket.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + let raw = ""; + await new Promise((resolve, reject) => { + socket.on("data", c => (raw += c.toString("latin1"))); + socket.on("error", reject); + socket.on("close", resolve); + }); + return raw.split("\r\n\r\n")[0]; + } finally { + socket.destroy(); + } + } + + async function rawHead(makeResponse: () => Response): Promise { + using server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + development: false, + fetch: makeResponse, + }); + return await readHead(server.port); + } + + const lines = (head: string, name: string) => head.split("\r\n").filter(l => l.toLowerCase().startsWith(name + ":")); + + for (const [label, value] of [ + ["empty", ""], + ["whitespace", " \t "], + ] as const) { + test(`content-type: ${label}`, async () => { + const head = await rawHead(() => new Response("x", { headers: { "content-type": value } })); + const ct = lines(head, "content-type"); + expect(ct).toHaveLength(1); + expect(ct[0].toLowerCase()).toBe("content-type: text/plain;charset=utf-8"); + }); + + test(`date: ${label}`, async () => { + const head = await rawHead(() => new Response("x", { headers: { date: value } })); + const date = lines(head, "date"); + expect(date).toHaveLength(1); + // auto Date is a valid IMF-fixdate, never an empty value + expect(date[0]).toMatch(/^Date: \S/); + expect(Number.isFinite(new Date(date[0].slice(6)).getTime())).toBe(true); + }); + } + + test("headers.set('content-type', '')", async () => { + const head = await rawHead(() => { + const r = new Response("x"); + r.headers.set("content-type", ""); + return r; + }); + const ct = lines(head, "content-type"); + expect(ct).toHaveLength(1); + expect(ct[0].toLowerCase()).toBe("content-type: text/plain;charset=utf-8"); + }); + + test("Response.json with empty content-type", async () => { + // The Fetch spec's "initialize a response" gates the default Content-Type on + // "header list contains" (key presence), so the Response object keeps the + // empty value; the wire serializer drops it and backfills from the body. + const r = Response.json({ a: 1 }, { headers: { "content-type": "" } }); + expect(r.headers.get("content-type")).toBe(""); + const head = await rawHead(() => Response.json({ a: 1 }, { headers: { "content-type": "" } })); + const ct = lines(head, "content-type"); + expect(ct).toHaveLength(1); + expect(ct[0]).toMatch(/^content-type: \S/i); + }); + + test("both empty at once: one of each", async () => { + const head = await rawHead(() => new Response("x", { headers: { "content-type": "", "date": "" } })); + expect(lines(head, "content-type")).toHaveLength(1); + expect(lines(head, "date")).toHaveLength(1); + }); + + async function rawHeadStatic(response: Response): Promise { + using server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + development: false, + static: { "/": response }, + fetch() { + return new Response("unreachable"); + }, + }); + return await readHead(server.port); + } + + test("static route: content-type empty", async () => { + const head = await rawHeadStatic(new Response("x", { headers: { "content-type": "" } })); + const ct = lines(head, "content-type"); + expect(ct).toHaveLength(1); + expect(ct[0].toLowerCase()).toBe("content-type: text/plain;charset=utf-8"); + }); + + test("static route: date empty", async () => { + const head = await rawHeadStatic(new Response("x", { headers: { date: "" } })); + const date = lines(head, "date"); + expect(date).toHaveLength(1); + expect(date[0]).toMatch(/^Date: \S/); + expect(Number.isFinite(new Date(date[0].slice(6)).getTime())).toBe(true); + }); + + test("static route: both empty", async () => { + const head = await rawHeadStatic(new Response("x", { headers: { "content-type": "", "date": "" } })); + const ct = lines(head, "content-type"); + const date = lines(head, "date"); + expect({ ct: ct.length, date: date.length }).toEqual({ ct: 1, date: 1 }); + expect(ct[0].toLowerCase()).toBe("content-type: text/plain;charset=utf-8"); + expect(date[0]).toMatch(/^Date: \S/); + }); + + test("static route: non-empty user values preserved", async () => { + const head = await rawHeadStatic( + new Response("x", { headers: { "content-type": "text/html", "date": "Sun, 06 Oct 2024 13:37:01 GMT" } }), + ); + expect(lines(head, "content-type")).toEqual(["Content-Type: text/html"]); + expect(lines(head, "date")).toEqual(["Date: Sun, 06 Oct 2024 13:37:01 GMT"]); + }); + + test("file route: date empty", async () => { + using dir = tempDir("serve-file-empty-date", { "a.txt": "x" }); + const head = await rawHeadStatic( + new Response(Bun.file(path.join(String(dir), "a.txt")), { headers: { date: "" } }), + ); + const date = lines(head, "date"); + expect(date).toHaveLength(1); + expect(date[0]).toMatch(/^Date: \S/); + }); + + test("non-empty user values are still preserved", async () => { + const head = await rawHead( + () => new Response("x", { headers: { "content-type": "text/html", "date": "Sun, 06 Oct 2024 13:37:01 GMT" } }), + ); + expect(lines(head, "content-type")).toEqual(["Content-Type: text/html"]); + expect(lines(head, "date")).toEqual(["Date: Sun, 06 Oct 2024 13:37:01 GMT"]); + }); + + test("empty custom header is dropped on both paths", async () => { + for (const head of [ + await rawHead(() => new Response("x", { headers: { "x-custom": "", "content-type": "text/html" } })), + await rawHeadStatic(new Response("x", { headers: { "x-custom": "", "content-type": "text/html" } })), + ]) { + expect(lines(head, "x-custom")).toEqual([]); + expect(lines(head, "content-type")).toEqual(["Content-Type: text/html"]); + } + }); + + test("empty set-cookie is dropped on both paths", async () => { + for (const head of [ + await rawHead(() => new Response("x", { headers: { "set-cookie": "" } })), + await rawHeadStatic(new Response("x", { headers: { "set-cookie": "" } })), + ]) { + expect(lines(head, "set-cookie")).toEqual([]); + } + }); + + test("static route: empty etag is dropped", async () => { + // An empty user etag is not overwritten with the auto content-hash (that + // would leave two snapshot entries and render_precondition reads the first), + // so the write-loop skip drops it and the route simply has no ETag. + const head = await rawHeadStatic(new Response("x", { headers: { etag: "" } })); + expect(lines(head, "etag")).toEqual([]); + }); +}); // https://github.com/oven-sh/bun/issues/9180 test("weird headers", async () => {