Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions src/jsc/bindings/NodeHTTP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<const char*>(valueSpan.data()), valueSpan.size()));
Expand All @@ -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;
}
Comment thread
robobun marked this conversation as resolved.

// We have to tell uWS not to automatically insert a TransferEncoding or Date header.
// Otherwise, you get this when using Fastify;
//
Expand Down Expand Up @@ -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;
}
Comment thread
robobun marked this conversation as resolved.
writeResponseHeader<isSSL>(res, name, value);
}
}
Expand Down Expand Up @@ -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<const char*>(s.data()), s.size()));
Expand All @@ -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;
Expand All @@ -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);
}
}
Expand Down
33 changes: 25 additions & 8 deletions src/runtime/server/FileRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand All @@ -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
Expand Down
10 changes: 7 additions & 3 deletions src/runtime/server/RequestContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
26 changes: 18 additions & 8 deletions src/runtime/server/StaticRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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());
Comment thread
robobun marked this conversation as resolved.
return Ok(Some(bun_core::heap::into_raw(Box::new(StaticRoute {
ref_count: Cell::new(1),
blob,
Expand Down Expand Up @@ -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;
}
Comment thread
robobun marked this conversation as resolved.
resp.write_header(
&buf[name.offset as usize..][..name.length as usize],
&buf[value.offset as usize..][..value.length as usize],
Expand Down
177 changes: 177 additions & 0 deletions test/js/bun/http/bun-serve-headers.test.ts
Original file line number Diff line number Diff line change
@@ -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<string> {
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<void>((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<string> {
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<string> {
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);
}
Comment thread
robobun marked this conversation as resolved.

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/);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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"]);
}
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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 () => {
Expand Down
Loading