Skip to content
Closed
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
36 changes: 13 additions & 23 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3036,9 +3036,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 @@ -3065,20 +3062,7 @@ 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;
return OutgoingMessagePrototype.end.$call(this, chunk, encoding, callback);
}

if (this[headerStateSymbol] === NodeHTTPHeaderState.none) {
Expand Down Expand Up @@ -3120,9 +3104,16 @@ 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;
// Socket already gone: like Node, 'prefinish' fires but 'finish' never does.
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;
Comment thread
robobun marked this conversation as resolved.
Resolved
}
const sentState = NodeHTTPHeaderState.sent;
if (headerState !== sentState) {
Expand Down Expand Up @@ -3289,9 +3280,8 @@ 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;
// Socket already gone: like Node's _writeRaw(), report false and drop the callback.
return false;
}

if (this[headerStateSymbol] !== NodeHTTPHeaderState.sent) {
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 @@ -299,3 +299,64 @@ describe("res.destroy() defers 'close'", () => {
expect(events).toEqual(["destroy()", "destroy() returned (closed: false)", "res.close (closed: true)"]);
});
});

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'.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
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.
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 @@ -2982,6 +2982,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.
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