diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 282248b5675b..b8557bf037a0 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -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; @@ -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) { @@ -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) { @@ -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) { diff --git a/test/js/node/http/node-http-proxy.js b/test/js/node/http/node-http-proxy.js index 8b82678ae9e4..0fca7f899e19 100644 --- a/test/js/node/http/node-http-proxy.js +++ b/test/js/node/http/node-http-proxy.js @@ -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: { diff --git a/test/js/node/http/node-http-server-abort-events.test.ts b/test/js/node/http/node-http-server-abort-events.test.ts index ac148f126e44..14cd0abe2be9 100644 --- a/test/js/node/http/node-http-server-abort-events.test.ts +++ b/test/js/node/http/node-http-server-abort-events.test.ts @@ -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(); + 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; + }); + 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(); + } +}); diff --git a/test/js/node/http/node-http.test.ts b/test/js/node/http/node-http.test.ts index a204e6d37259..6951284b555e 100644 --- a/test/js/node/http/node-http.test.ts +++ b/test/js/node/http/node-http.test.ts @@ -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(). + 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(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.