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
49 changes: 26 additions & 23 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3082,9 +3082,6 @@ ServerResponse.prototype.writeContinue = function (cb) {
// But we don't want it for the fetch() response version.
ServerResponse.prototype.end = function (chunk, encoding, callback) {
const handle = this[kHandle];
if (handle?.aborted) {
return this;
}

if ($isCallable(chunk)) {
callback = chunk;
Expand All @@ -3111,20 +3108,13 @@ ServerResponse.prototype.end = function (chunk, encoding, callback) {
}

if (!handle) {
// Read the storage directly - the `socket` getter auto-creates a
// FakeSocket and would make this condition always true.
if (this[fakeSocketSymbol] || this.outputData?.length || !this._header) {
// Standalone response writing through an assigned socket (or buffering
// until one is assigned): use the OutgoingMessage machinery. The
// original chunk passes through (mirroring write()): write_() has its
// own !_hasBody handling, including the rejectNonStandardBodyWrites
// throw, which the clearing below would bypass.
return OutgoingMessagePrototype.end.$call(this, chunk, encoding, callback);
}
if ($isCallable(callback)) {
process.nextTick(callback);
}
return this;
// Standalone response (no native handle): use the OutgoingMessage
// machinery unconditionally so `finished` / `writableEnded` transition and
// the chunk is buffered into outputData like Node.js, regardless of whether
// writeHead() rendered `_header` first. The original chunk passes through
// (mirroring write()): write_() has its own !_hasBody handling, including
// the rejectNonStandardBodyWrites throw.
return OutgoingMessagePrototype.end.$call(this, chunk, encoding, callback);
}

if (this[headerStateSymbol] === NodeHTTPHeaderState.none) {
Expand Down Expand Up @@ -3166,9 +3156,22 @@ ServerResponse.prototype.end = function (chunk, encoding, callback) {

const flags = handle.flags;
if (!!(flags & NodeHTTPResponseFlags.closed_or_completed)) {
// node.js will return true if the handle is closed but the internal state is not
// and will not throw or emit an error
return true;
// The underlying socket is already closed (or the response completed
// natively). Node.js's OutgoingMessage.end() still transitions to
// `finished` and emits 'prefinish' in this state; the `finish` callback
// that would emit 'finish' is dropped by _writeRaw()'s conn.destroyed
// early-return, so 'finish' never fires and the end() callback (registered
// on 'finish') is never called. 'close' is emitted by the socket close
// path.
this._header = " ";
const req = this.req;
if (!req._consuming && !req?._readableState?.resumeScheduled) {
req._dump();
}
this.finished = true;
process.nextTick(markResponseEndedNT, this);
this.emit("prefinish");
return this;
}
const sentState = NodeHTTPHeaderState.sent;
if (headerState !== sentState) {
Expand Down Expand Up @@ -3335,9 +3338,9 @@ ServerResponse.prototype.write = function (chunk, encoding, callback) {

const flags = handle.flags;
if (!!(flags & NodeHTTPResponseFlags.closed_or_completed)) {
// node.js will return true if the handle is closed but the internal state is not
// and will not throw or emit an error
return true;
// Node.js's OutgoingMessage._writeRaw() returns false when the assigned
// socket is destroyed; the write callback is dropped (never invoked).
return false;
}

if (this[headerStateSymbol] !== NodeHTTPHeaderState.sent) {
Expand Down
4 changes: 2 additions & 2 deletions test/js/node/http/node-http-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ export async function run() {
req.pipe(proxyRequest); // Use pipe instead of manual data handling
});

proxyServer.listen(0, "localhost", async () => {
proxyServer.listen(0, "127.0.0.1", async () => {
const address = proxyServer.address();

const options = {
protocol: "http:",
hostname: "localhost",
hostname: "127.0.0.1",
port: address.port,
path: "/", // Change path to /
headers: {
Expand Down
61 changes: 61 additions & 0 deletions test/js/node/http/node-http-server-abort-events.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,64 @@ test("aborted request body emits 'error' ECONNRESET and res 'close' before req '
server.close();
}
});

test("res.write()/end() after req.socket.destroy() inside the handler", async () => {
// The request handler destroys its own socket before writing: write() must
// report false (Node.js's OutgoingMessage._writeRaw returns false for a
// destroyed socket) and end() must still transition to `finished` /
// `writableEnded` and emit 'prefinish', without emitting 'finish'.
const events: string[] = [];
const { promise: resClosed, resolve: resolveResClosed } = Promise.withResolvers<void>();
let writeResult: boolean | undefined;
let endReturnedSelf: boolean | undefined;
let finishedAfterEnd: boolean | undefined;
let writableEndedAfterEnd: boolean | undefined;
let writeCbCalled = false;
let endCbCalled = false;

const server = createServer((req, res) => {
res.on("prefinish", () => events.push("res.prefinish"));
res.on("finish", () => events.push("res.finish"));
res.on("close", () => {
events.push("res.close");
resolveResClosed();
});

req.socket.destroy();
writeResult = res.write("body", () => (writeCbCalled = true));
endReturnedSelf = res.end("done", () => (endCbCalled = true)) === res;
finishedAfterEnd = res.finished;
writableEndedAfterEnd = res.writableEnded;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
try {
server.listen(0, "127.0.0.1");
await once(server, "listening");
const { port } = server.address() as AddressInfo;

const client = connect(port, "127.0.0.1");
client.on("error", () => {});
await once(client, "connect");
client.write("GET / HTTP/1.1\r\nHost: x\r\n\r\n");
await resClosed;

expect({
writeResult,
endReturnedSelf,
finishedAfterEnd,
writableEndedAfterEnd,
writeCbCalled,
endCbCalled,
events,
}).toEqual({
writeResult: false,
endReturnedSelf: true,
finishedAfterEnd: true,
writableEndedAfterEnd: true,
writeCbCalled: false,
endCbCalled: false,
events: ["res.prefinish", "res.close"],
});
} finally {
server.close();
}
});
29 changes: 29 additions & 0 deletions test/js/node/http/node-http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2878,6 +2878,35 @@ it("standalone ServerResponse discards body writes to a no-body response without
expect(out).not.toContain("body");
});

it("standalone ServerResponse end() after writeHead() sets writableEnded (#25632)", async () => {
// With no native handle and no socket assigned, end() after writeHead() must
// still transition to `finished` / `writableEnded` and buffer the rendered
// header block into outputData, like Node.js's OutgoingMessage.end().
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const res = new ServerResponse(new IncomingMessage(null as any));
res.writeHead(403);
res.end("forbidden");
expect({ finished: res.finished, writableEnded: res.writableEnded }).toEqual({
finished: true,
writableEnded: true,
});
const buffered = (res as any).outputData.map((x: any) => String(x.data)).join("");
expect(buffered).toStartWith("HTTP/1.1 403 Forbidden\r\n");
expect(buffered).toEndWith("\r\n\r\nforbidden");

// And with end() alone (no chunk), the end() callback is registered on
// 'finish' (which never fires without a socket), not invoked via nextTick.
const res2 = new ServerResponse(new IncomingMessage(null as any));
let cbCalled = false;
res2.writeHead(200);
res2.end(() => (cbCalled = true));
expect({ finished: res2.finished, writableEnded: res2.writableEnded }).toEqual({
finished: true,
writableEnded: true,
});
await new Promise<void>(r => process.nextTick(r));
expect(cbCalled).toBe(false);
});

it("flushHeaders on a 204 response carries no chunked framing", async () => {
// noBodyStatus must suppress the Transfer-Encoding header in flushHeaders()
// and the terminating chunk in internalEnd(), like the one-shot end() path.
Expand Down
Loading