Skip to content

node:http2: ignore request() options in respond(), like node #43478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 4 commits into
base: main
Choose a base branch
from
Open

node:http2: ignore request() options in respond(), like node #43478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e0431d
node:http2: respond() reads only the options a response has
robobun Sep 19, 2026
db3a379
node:http2: keep statCheck from changing the options of a file response
robobun Sep 19, 2026
5db8c46
node:http2: cut the new comments in respond() to one line each
robobun Sep 19, 2026
25f7f43
node:http2: check and copy the options of pushStream() like node
robobun Sep 19, 2026
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
60 changes: 33 additions & 27 deletions src/js/node/http2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2957,14 +2957,20 @@ function tryClose(fd) {
function doSendFileFD(options, fd, headers, err, stat) {
const onError = options.onError;
const ownsFd = this[kOwnsFd] === true;
// statCheck is handed `options` and may write to it. node reads these first and never reads endStream.
const respondOptions = {
waitForTrailers: options.waitForTrailers,
sendDate: options.sendDate,
paddingStrategy: options.paddingStrategy,
};
if (err) {
if (ownsFd && err.code !== "EBADF") {
tryClose(fd);
}

if (onError) onError(err);
else {
this.respond(headers, options);
this.respond(headers, respondOptions);
this.destroy(streamErrorFromCode(NGHTTP2_INTERNAL_ERROR));
}
return;
Expand All @@ -2983,7 +2989,7 @@ function doSendFileFD(options, fd, headers, err, stat) {
if (ownsFd) tryClose(fd);
if (onError) onError(err);
else {
this.respond(headers, options);
this.respond(headers, respondOptions);
this.destroy(err);
}
return;
Comment thread
robobun marked this conversation as resolved.
Expand Down Expand Up @@ -3040,7 +3046,7 @@ function doSendFileFD(options, fd, headers, err, stat) {
headers[HTTP2_HEADER_CONTENT_LENGTH] = statOptions.length;
}
try {
this.respond(headers, options);
this.respond(headers, respondOptions);
} catch (err) {
// respond() rejected the headers (e.g. a request pseudo-header in the response): the fd opened
// for the file never reaches a read stream, so close it here before the stream is destroyed.
Expand Down Expand Up @@ -3220,6 +3226,9 @@ class ServerHttp2Stream extends Http2Stream {
if (!this.pushAllowed) {
throw $ERR_HTTP2_PUSH_DISABLED();
}
assertIsObject(options, "options");
// Like node, read a copy so only own enumerable keys count.
options = { ...options };
const session = this[bunHTTP2Session];
const parser = session?.[bunHTTP2Native];
if (!parser) {
Expand Down Expand Up @@ -3303,7 +3312,7 @@ class ServerHttp2Stream extends Http2Stream {
if (headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD) {
pushedStream[kHeadRequest] = true;
pushedStream.end();
} else if (options?.endStream) {
} else if (options.endStream) {
pushedStream.end();
}
}
Expand All @@ -3315,6 +3324,7 @@ class ServerHttp2Stream extends Http2Stream {
throw $ERR_HTTP2_INVALID_STREAM();
}
if (this.headersSent) throw $ERR_HTTP2_HEADERS_SENT();
assertIsObject(options, "options");

if ($isArray(headers)) {
// node rejects the raw-array form here (only respond() accepts it) - same
Expand Down Expand Up @@ -3375,6 +3385,7 @@ class ServerHttp2Stream extends Http2Stream {
throw $ERR_HTTP2_INVALID_STREAM();
}
if (this.headersSent) throw $ERR_HTTP2_HEADERS_SENT();
assertIsObject(options, "options");

if ($isArray(headers)) {
// node rejects the raw-array form here (only respond() accepts it) - same
Expand Down Expand Up @@ -3508,6 +3519,13 @@ class ServerHttp2Stream extends Http2Stream {
if (this.sentTrailers) {
throw $ERR_HTTP2_TRAILERS_ALREADY_SENT();
}
assertIsObject(options, "options");
// Like node, read a copy so only own enumerable keys count. paddingStrategy is a bun extension.
options = { ...options };
const sendDate = options.sendDate;
const paddingStrategy = options.paddingStrategy;
let endStream = !!options.endStream;
let waitForTrailers = !!options.waitForTrailers;

// Raw (flat [name, value, ...] array) headers form: the pairs are encoded
// on the wire in their given order; a default :status is prepended and a
Expand Down Expand Up @@ -3541,8 +3559,7 @@ class ServerHttp2Stream extends Http2Stream {
statusCode = 200;
headers.unshift(HTTP2_HEADER_STATUS, statusCode);
}
const sendDateOption = options?.sendDate;
if (!isDateSet && (sendDateOption == null || sendDateOption)) {
if (!isDateSet && (sendDate == null || sendDate)) {
headers.push(HTTP2_HEADER_DATE, utcDate());
}
rawHeadersList = headers;
Expand Down Expand Up @@ -3598,7 +3615,6 @@ class ServerHttp2Stream extends Http2Stream {
if (statusCode < 100 || statusCode > 599) {
throw $ERR_HTTP2_STATUS_INVALID(statusCode);
}
let endStream = !!options?.endStream;
if (
endStream ||
statusCode === HTTP_STATUS_NO_CONTENT ||
Comment thread
robobun marked this conversation as resolved.
Expand All @@ -3612,13 +3628,10 @@ class ServerHttp2Stream extends Http2Stream {
// onWantTrailers immediately after, whose JS handler calls
// noTrailers → sendData("", true) and emits a spurious DATA frame on
// the already-half-closed stream (RFC 9113 §5.1 violation). Strip
// waitForTrailers here so the native never fires that path; the JS
// guard further down (`options?.waitForTrailers && !endStream`) only
// covers the `_final` side and runs AFTER the native call.
options = { ...options, endStream: true, waitForTrailers: false };
// waitForTrailers so neither the native layer nor `_final` takes the wantTrailers path.
endStream = true;
waitForTrailers = false;
}
const sendDate = options?.sendDate;
if (rawHeadersList === null && (sendDate == null || sendDate)) {
const current_date = headers["date"];
if (current_date == null) {
Expand All @@ -3627,21 +3640,14 @@ class ServerHttp2Stream extends Http2Stream {
}

const wireHeaders = rawHeadersList !== null ? rawHeadersList : headers;
if (typeof options === "undefined") {
session[bunHTTP2Native]?.request(this.id, undefined, wireHeaders, sensitiveNames);
} else {
session[bunHTTP2Native]?.request(this.id, undefined, wireHeaders, sensitiveNames, options);
// Only track waitForTrailers when the HEADERS frame above did NOT end
// the stream. Status codes 204/205/304 and HEAD requests force
// endStream=true earlier in this method, which means the native
// request() already wrote END_STREAM on the HEADERS frame — driving
// the wantTrailers path from `_final` on such a stream would call
// `noTrailers`/`emit("wantTrailers")` on an already-half-closed
// stream and corrupt state. Use optional chaining: `options` may be
// `null` here (typeof null === "object" enters this else branch).
if (options?.waitForTrailers && !endStream) {
this[bunHTTP2WaitForTrailers] = true;
}
// The native writer is shared with request() and acts on parent, weight, exclusive, silent and signal.
session[bunHTTP2Native]?.request(this.id, undefined, wireHeaders, sensitiveNames, {
endStream,
waitForTrailers,
paddingStrategy,
});
if (waitForTrailers) {
this[bunHTTP2WaitForTrailers] = true;
}
this.headersSent = true;
if (onServerStreamFinishChannel.hasSubscribers) {
Expand Down
Loading
Loading