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
28 changes: 22 additions & 6 deletions src/bun.js/api/server/NodeHTTPResponse.zig
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,8 @@ extern "C" fn NodeHTTPServer__writeHead_http(
statusMessage: [*]const u8,
statusMessageLength: usize,
headersObjectValue: jsc.JSValue,
shouldKeepAlive: bool,
keepAliveTimeout: u32,
response: *anyopaque,
) void;

Expand All @@ -415,11 +417,13 @@ extern "C" fn NodeHTTPServer__writeHead_https(
statusMessage: [*]const u8,
statusMessageLength: usize,
headersObjectValue: jsc.JSValue,
shouldKeepAlive: bool,
keepAliveTimeout: u32,
response: *anyopaque,
) void;

pub fn writeHead(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSError!jsc.JSValue {
const arguments = callframe.argumentsUndef(3).slice();
const arguments = callframe.argumentsUndef(5).slice();

if (this.isRequestedCompletedOrEnded()) {
return globalObject.ERR(.STREAM_ALREADY_FINISHED, "Stream is already ended", .{}).throw();
Expand All @@ -436,6 +440,8 @@ pub fn writeHead(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, cal
const status_code_value: JSValue = if (arguments.len > 0) arguments[0] else .js_undefined;
const status_message_value: JSValue = if (arguments.len > 1 and arguments[1] != .null) arguments[1] else .js_undefined;
const headers_object_value: JSValue = if (arguments.len > 2 and arguments[2] != .null) arguments[2] else .js_undefined;
const should_keep_alive_value: JSValue = if (arguments.len > 3) arguments[3] else .js_undefined;
const keep_alive_timeout_value: JSValue = if (arguments.len > 4) arguments[4] else .js_undefined;

const status_code: i32 = brk: {
if (!status_code_value.isUndefined()) {
Expand All @@ -461,33 +467,43 @@ pub fn writeHead(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, cal
return error.JSError;
}

const should_keep_alive: bool = if (!should_keep_alive_value.isUndefined())
should_keep_alive_value.toBoolean()
else
true;

const keep_alive_timeout: u32 = if (!keep_alive_timeout_value.isUndefined())
@intCast(keep_alive_timeout_value.toU32())
else
5000;

if (state.isHttpStatusCalled()) {
return globalObject.ERR(.HTTP_HEADERS_SENT, "Stream already started", .{}).throw();
}

do_it: {
if (status_message_slice.len == 0) {
if (HTTPStatusText.get(@intCast(status_code))) |status_message| {
writeHeadInternal(this.raw_response.?, globalObject, status_message, headers_object_value);
writeHeadInternal(this.raw_response.?, globalObject, status_message, headers_object_value, should_keep_alive, keep_alive_timeout);
break :do_it;
}
}

const message = if (status_message_slice.len > 0) status_message_slice.slice() else "HM";
const status_message = bun.handleOom(std.fmt.allocPrint(allocator, "{d} {s}", .{ status_code, message }));
defer allocator.free(status_message);
writeHeadInternal(this.raw_response.?, globalObject, status_message, headers_object_value);
writeHeadInternal(this.raw_response.?, globalObject, status_message, headers_object_value, should_keep_alive, keep_alive_timeout);
break :do_it;
}

return .js_undefined;
}

fn writeHeadInternal(response: uws.AnyResponse, globalObject: *jsc.JSGlobalObject, status_message: []const u8, headers: jsc.JSValue) void {
fn writeHeadInternal(response: uws.AnyResponse, globalObject: *jsc.JSGlobalObject, status_message: []const u8, headers: jsc.JSValue, should_keep_alive: bool, keep_alive_timeout: u32) void {
log("writeHeadInternal({s})", .{status_message});
switch (response) {
.TCP => NodeHTTPServer__writeHead_http(globalObject, status_message.ptr, status_message.len, headers, @ptrCast(response.TCP)),
.SSL => NodeHTTPServer__writeHead_https(globalObject, status_message.ptr, status_message.len, headers, @ptrCast(response.SSL)),
.TCP => NodeHTTPServer__writeHead_http(globalObject, status_message.ptr, status_message.len, headers, should_keep_alive, keep_alive_timeout, @ptrCast(response.TCP)),
.SSL => NodeHTTPServer__writeHead_https(globalObject, status_message.ptr, status_message.len, headers, should_keep_alive, keep_alive_timeout, @ptrCast(response.SSL)),
}
}

Expand Down
71 changes: 67 additions & 4 deletions src/bun.js/bindings/NodeHTTP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ static void writeResponseHeader(uWS::HttpResponse<isSSL>* res, const WTF::String
}

template<bool isSSL>
static void writeFetchHeadersToUWSResponse(WebCore::FetchHeaders& headers, uWS::HttpResponse<isSSL>* res)
static void writeFetchHeadersToUWSResponse(WebCore::FetchHeaders& headers, uWS::HttpResponse<isSSL>* res, bool shouldKeepAlive = true, uint32_t keepAliveTimeout = 5000)
{
auto& internalHeaders = headers.internalHeaders();

Expand All @@ -526,12 +526,18 @@ static void writeFetchHeadersToUWSResponse(WebCore::FetchHeaders& headers, uWS::
}

auto* data = res->getHttpResponseData();
bool hasConnectionHeader = false;

for (const auto& header : internalHeaders.commonHeaders()) {

const auto& name = WebCore::httpHeaderNameString(header.key);
const auto& value = header.value;

// Check if Connection header is already set
if (header.key == WebCore::HTTPHeaderName::Connection) {
hasConnectionHeader = true;
}

// 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 @@ -568,8 +574,29 @@ static void writeFetchHeadersToUWSResponse(WebCore::FetchHeaders& headers, uWS::
const auto& name = header.key;
const auto& value = header.value;

// Check for Connection header in uncommon headers (case-insensitive)
if (name.length() == 10 && WTF::equalIgnoringASCIICase(name, "connection")) {
hasConnectionHeader = true;
}

writeResponseHeader<isSSL>(res, name, value);
}

// Add Connection: keep-alive and Keep-Alive headers if not already set
// This matches Node.js behavior
if (!hasConnectionHeader) {
if (shouldKeepAlive) {
res->writeHeader(std::string_view("Connection", 10), std::string_view("keep-alive", 10));

// Format Keep-Alive header with timeout
char keepAliveValue[32];
uint32_t timeoutSeconds = keepAliveTimeout / 1000;
int len = snprintf(keepAliveValue, sizeof(keepAliveValue), "timeout=%u", timeoutSeconds);
res->writeHeader(std::string_view("Keep-Alive", 10), std::string_view(keepAliveValue, len));
} else {
res->writeHeader(std::string_view("Connection", 10), std::string_view("close", 5));
}
}
}

template<bool isSSL>
Expand All @@ -578,6 +605,8 @@ static void NodeHTTPServer__writeHead(
const char* statusMessage,
size_t statusMessageLength,
JSValue headersObjectValue,
bool shouldKeepAlive,
uint32_t keepAliveTimeout,
uWS::HttpResponse<isSSL>* response)
{
auto& vm = globalObject->vm();
Expand All @@ -589,9 +618,11 @@ static void NodeHTTPServer__writeHead(
}
response->writeStatus(std::string_view(statusMessage, statusMessageLength));

bool hasConnectionHeader = false;

if (headersObject) {
if (auto* fetchHeaders = jsDynamicCast<WebCore::JSFetchHeaders*>(headersObject)) {
writeFetchHeadersToUWSResponse<isSSL>(fetchHeaders->wrapped(), response);
writeFetchHeadersToUWSResponse<isSSL>(fetchHeaders->wrapped(), response, shouldKeepAlive, keepAliveTimeout);
return;
}

Expand All @@ -611,6 +642,12 @@ static void NodeHTTPServer__writeHead(
}

String key = entry.key();

// Check for Connection header (case-insensitive)
if (key.length() == 10 && WTF::equalIgnoringASCIICase(key, "connection")) {
hasConnectionHeader = true;
}

String value = headerValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, false);

Expand All @@ -631,13 +668,35 @@ static void NodeHTTPServer__writeHead(
}

String key = propertyNames[i].string();

// Check for Connection header (case-insensitive)
if (key.length() == 10 && WTF::equalIgnoringASCIICase(key, "connection")) {
hasConnectionHeader = true;
}

String value = headerValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, void());
writeResponseHeader<isSSL>(response, key, value);
}
}
}

// Add Connection: keep-alive and Keep-Alive headers if not already set
// This matches Node.js behavior
if (!hasConnectionHeader) {
if (shouldKeepAlive) {
response->writeHeader(std::string_view("Connection", 10), std::string_view("keep-alive", 10));

// Format Keep-Alive header with timeout
char keepAliveValue[32];
uint32_t timeoutSeconds = keepAliveTimeout / 1000;
int len = snprintf(keepAliveValue, sizeof(keepAliveValue), "timeout=%u", timeoutSeconds);
response->writeHeader(std::string_view("Keep-Alive", 10), std::string_view(keepAliveValue, len));
} else {
response->writeHeader(std::string_view("Connection", 10), std::string_view("close", 5));
}
}

RELEASE_AND_RETURN(scope, void());
}

Expand All @@ -646,19 +705,23 @@ extern "C" void NodeHTTPServer__writeHead_http(
const char* statusMessage,
size_t statusMessageLength,
JSValue headersObjectValue,
bool shouldKeepAlive,
uint32_t keepAliveTimeout,
uWS::HttpResponse<false>* response)
{
return NodeHTTPServer__writeHead<false>(globalObject, statusMessage, statusMessageLength, headersObjectValue, response);
return NodeHTTPServer__writeHead<false>(globalObject, statusMessage, statusMessageLength, headersObjectValue, shouldKeepAlive, keepAliveTimeout, response);
}

extern "C" void NodeHTTPServer__writeHead_https(
JSC::JSGlobalObject* globalObject,
const char* statusMessage,
size_t statusMessageLength,
JSValue headersObjectValue,
bool shouldKeepAlive,
uint32_t keepAliveTimeout,
uWS::HttpResponse<true>* response)
{
return NodeHTTPServer__writeHead<true>(globalObject, statusMessage, statusMessageLength, headersObjectValue, response);
return NodeHTTPServer__writeHead<true>(globalObject, statusMessage, statusMessageLength, headersObjectValue, shouldKeepAlive, keepAliveTimeout, response);
}

extern "C" EncodedJSValue NodeHTTPServer__onRequest_http(
Expand Down
16 changes: 12 additions & 4 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1349,7 +1349,9 @@ ServerResponse.prototype.end = function (chunk, encoding, callback) {
}
if (headerState !== NodeHTTPHeaderState.sent) {
handle.cork(() => {
handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol]);
const shouldKeepAlive = this.shouldKeepAlive;
const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);

Comment on lines +1352 to 1355
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Deduplicate keep-alive param computation

The same shouldKeepAlive/keepAliveTimeout computation repeats in 4 paths. Extract a helper to reduce drift.

Apply this refactor:

@@
-      const shouldKeepAlive = this.shouldKeepAlive;
-      const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
-      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
+      const [ska, kat] = getConnectionParams(this);
+      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], ska, kat);
@@
-      const shouldKeepAlive = this.shouldKeepAlive;
-      const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
-      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
+      const [ska, kat] = getConnectionParams(this);
+      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], ska, kat);
@@
-      const shouldKeepAlive = this.shouldKeepAlive;
-      const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
-      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
+      const [ska, kat] = getConnectionParams(this);
+      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], ska, kat);
@@
-      const shouldKeepAlive = this.shouldKeepAlive;
-      const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
-      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
+      const [ska, kat] = getConnectionParams(this);
+      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], ska, kat);

Add helper (outside the selected ranges):

function getConnectionParams(res: any): [boolean, number] {
  const shouldKeepAlive = res.shouldKeepAlive;
  const keepAliveTimeout = res.socket?.server?.keepAliveTimeout ?? 5000;
  return [shouldKeepAlive, keepAliveTimeout];
}

Also applies to: 1464-1467, 1569-1572, 1638-1641

🤖 Prompt for AI Agents
In src/js/node/_http_server.ts around lines 1352-1355, the keep-alive
computation (shouldKeepAlive and keepAliveTimeout) is duplicated across four
code paths; add a new helper function getConnectionParams(res: any): [boolean,
number] outside the selected ranges that returns [res.shouldKeepAlive,
res.socket?.server?.keepAliveTimeout ?? 5000], then replace the local
computations at 1352-1355 and the other locations (1464-1467, 1569-1572,
1638-1641) to call the helper and destructure its result before calling
handle.writeHead (or equivalent) so all paths reuse the single helper.

Comment on lines +1352 to 1355
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Deduplicate keep‑alive param computation

The same two lines repeat across four sites. Extract a tiny helper to reduce drift.

@@
-      const shouldKeepAlive = this.shouldKeepAlive;
-      const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
-      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
+      const { shouldKeepAlive, keepAliveTimeout } = getKeepAliveParams(this);
+      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
@@
-      const shouldKeepAlive = this.shouldKeepAlive;
-      const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
-      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
+      const { shouldKeepAlive, keepAliveTimeout } = getKeepAliveParams(this);
+      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
@@
-      const shouldKeepAlive = this.shouldKeepAlive;
-      const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
-      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
+      const { shouldKeepAlive, keepAliveTimeout } = getKeepAliveParams(this);
+      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
@@
-      const shouldKeepAlive = this.shouldKeepAlive;
-      const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
-      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
+      const { shouldKeepAlive, keepAliveTimeout } = getKeepAliveParams(this);
+      handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);

Add once near ServerResponse:

function getKeepAliveParams(self: any) {
  return {
    shouldKeepAlive: self.shouldKeepAlive,
    keepAliveTimeout: self.socket?.server?.keepAliveTimeout ?? 5000,
  };
}

Also applies to: 1464-1467, 1569-1572, 1638-1641

🤖 Prompt for AI Agents
In src/js/node/_http_server.ts around lines 1352-1355 (and similarly 1464-1467,
1569-1572, 1638-1641), the computation of shouldKeepAlive and keepAliveTimeout
is duplicated; add a small helper function near ServerResponse named
getKeepAliveParams(self: any) that returns { shouldKeepAlive:
self.shouldKeepAlive, keepAliveTimeout: self.socket?.server?.keepAliveTimeout ??
5000 }, then replace the duplicated two-line blocks with a single call to that
helper and destructure the returned values before calling handle.writeHead
(i.e., const { shouldKeepAlive, keepAliveTimeout } = getKeepAliveParams(this);).

// If handle.writeHead throws, we don't want headersSent to be set to true.
// So we set it here.
Expand Down Expand Up @@ -1459,7 +1461,9 @@ ServerResponse.prototype.write = function (chunk, encoding, callback) {

if (this[headerStateSymbol] !== NodeHTTPHeaderState.sent) {
handle.cork(() => {
handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol]);
const shouldKeepAlive = this.shouldKeepAlive;
const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);

// If handle.writeHead throws, we don't want headersSent to be set to true.
// So we set it here.
Expand Down Expand Up @@ -1562,7 +1566,9 @@ ServerResponse.prototype._send = function (data, encoding, callback, _byteLength

if (this[headerStateSymbol] !== NodeHTTPHeaderState.sent) {
handle.cork(() => {
handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol]);
const shouldKeepAlive = this.shouldKeepAlive;
const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
this[headerStateSymbol] = NodeHTTPHeaderState.sent;
handle.write(data, encoding, callback, strictContentLength(this));
});
Expand Down Expand Up @@ -1629,7 +1635,9 @@ ServerResponse.prototype.flushHeaders = function () {
if (this[headerStateSymbol] === NodeHTTPHeaderState.assigned) {
this[headerStateSymbol] = NodeHTTPHeaderState.sent;

handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol]);
const shouldKeepAlive = this.shouldKeepAlive;
const keepAliveTimeout = this.socket?.server?.keepAliveTimeout ?? 5000;
handle.writeHead(this.statusCode, this.statusMessage, this[headersSymbol], shouldKeepAlive, keepAliveTimeout);
}
handle.flushHeaders();
}
Expand Down
Loading