From d08e4bae09032e75bdcf19a38fdbb05c452563f9 Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 26 Jan 2026 11:09:03 -0800 Subject: [PATCH] fix(http2): prevent extra empty DATA frame on write()+end() pattern (#26410) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixes an issue where calling `req.write(data)` followed by `req.end()` on an HTTP/2 stream would send **three** DATA frames instead of **two** - This caused AWS ALB and other strict HTTP/2 servers to reject the connection with `NGHTTP2_FRAME_SIZE_ERROR` (error code 6) ## Root Cause The `Http2Stream.end()` method was creating an empty buffer (`Buffer.alloc(0)`) when called without data: ```javascript if (!chunk) { chunk = Buffer.alloc(0); } return super.end(chunk, encoding, callback); ``` This empty buffer was then passed to the Duplex stream's `end()`, which triggered `_write()` with the empty buffer before calling `_final()`. This resulted in: 1. DATA frame with actual data (from `_write`) 2. Empty DATA frame without END_STREAM (from the extra `_write` with empty buffer) 3. Empty DATA frame with END_STREAM (from `_final`) The second empty DATA frame was unnecessary and violated some strict HTTP/2 implementations. ## Fix Remove the unnecessary empty buffer creation. The Duplex stream's `end()` method already handles the no-data case correctly by calling `_final()` directly without calling `_write()`. ## Test plan - [x] Manually verified with ConnectRPC client and AWS ALB endpoint - [x] Added regression test `test/regression/issue/25589-write-end.test.ts` - [x] Existing HTTP/2 tests pass - [x] Existing gRPC tests pass Fixes #25589 🤖 Generated with [Claude Code](https://claude.ai/claude-code) --------- Co-authored-by: Claude Bot Co-authored-by: Claude Opus 4.5 Co-authored-by: Ciro Spaciari Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/js/node/http2.ts | 6 +- test/regression/issue/25589-write-end.test.ts | 201 ++++++++++++++++++ 2 files changed, 204 insertions(+), 3 deletions(-) create mode 100644 test/regression/issue/25589-write-end.test.ts diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 2d58a28447..4f08baf64d 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -1995,10 +1995,10 @@ class Http2Stream extends Duplex { typeof callback == "function" && callback(); return; } - if (!chunk) { - chunk = Buffer.alloc(0); - } this[bunHTTP2StreamStatus] = status | StreamState.EndedCalled; + // Don't create an empty buffer for end() without data - let the Duplex stream + // handle it naturally (just calls _final without _write for empty data). + // Creating an empty buffer here causes an extra empty DATA frame to be sent. return super.end(chunk, encoding, callback); } diff --git a/test/regression/issue/25589-write-end.test.ts b/test/regression/issue/25589-write-end.test.ts new file mode 100644 index 0000000000..417e7eb675 --- /dev/null +++ b/test/regression/issue/25589-write-end.test.ts @@ -0,0 +1,201 @@ +import { expect, test } from "bun:test"; +import http2 from "node:http2"; + +test("http2 write() + end() pattern should only send two DATA frames (local server)", async () => { + // Create a test server that tracks received DATA frames + const receivedDataFrames: Array<{ length: number; flags: number }> = []; + + const server = http2.createServer(); + + server.on("stream", (stream, headers) => { + stream.on("data", chunk => { + // Track that we received data + receivedDataFrames.push({ + length: chunk.length, + flags: 0, // We can't easily get flags here, but we track frame count + }); + }); + + stream.on("end", () => { + // Send response + stream.respond({ ":status": 200 }); + stream.end("OK"); + }); + }); + + await new Promise(resolve => { + server.listen(0, resolve); + }); + + const port = (server.address() as any).port; + + try { + const client = http2.connect(`http://localhost:${port}`); + + const result = await new Promise((resolve, reject) => { + client.on("error", reject); + + const req = client.request({ + ":method": "POST", + ":path": "/test", + }); + + req.on("response", headers => { + expect(headers[":status"]).toBe(200); + }); + + let data = ""; + req.on("data", chunk => { + data += chunk; + }); + + req.on("end", () => { + resolve(data); + client.close(); + }); + + req.on("error", reject); + + // This is the pattern that was causing the bug: + // write() followed by end() should only send 2 DATA frames + const testData = Buffer.from("Hello, World!"); + req.write(testData, "binary", () => { + req.end(); + }); + }); + + expect(result).toBe("OK"); + + // We should receive exactly one data chunk (the test data), + // NOT two empty frames after it + expect(receivedDataFrames.length).toBe(1); + expect(receivedDataFrames[0].length).toBe(13); // "Hello, World!" is 13 bytes + } finally { + server.close(); + } +}); + +test("http2 end() without data should send END_STREAM with no DATA frames", async () => { + const receivedDataFrames: Array<{ length: number }> = []; + + const server = http2.createServer(); + + server.on("stream", (stream, headers) => { + stream.on("data", chunk => { + receivedDataFrames.push({ length: chunk.length }); + }); + + stream.on("end", () => { + stream.respond({ ":status": 200 }); + stream.end("OK"); + }); + }); + + await new Promise(resolve => { + server.listen(0, resolve); + }); + + const port = (server.address() as any).port; + + try { + const client = http2.connect(`http://localhost:${port}`); + + const result = await new Promise((resolve, reject) => { + client.on("error", reject); + + const req = client.request({ + ":method": "POST", + ":path": "/test", + }); + + req.on("response", headers => { + expect(headers[":status"]).toBe(200); + }); + + let data = ""; + req.on("data", chunk => { + data += chunk; + }); + + req.on("end", () => { + resolve(data); + client.close(); + }); + + req.on("error", reject); + + // Just call end() without any data + req.end(); + }); + + expect(result).toBe("OK"); + + // Should receive no data frames (just empty END_STREAM handled by _final) + expect(receivedDataFrames.length).toBe(0); + } finally { + server.close(); + } +}); + +test("http2 end(data) should send data with END_STREAM in one frame", async () => { + const receivedDataFrames: Array<{ length: number }> = []; + + const server = http2.createServer(); + + server.on("stream", (stream, headers) => { + stream.on("data", chunk => { + receivedDataFrames.push({ length: chunk.length }); + }); + + stream.on("end", () => { + stream.respond({ ":status": 200 }); + stream.end("OK"); + }); + }); + + await new Promise(resolve => { + server.listen(0, resolve); + }); + + const port = (server.address() as any).port; + + try { + const client = http2.connect(`http://localhost:${port}`); + + const result = await new Promise((resolve, reject) => { + client.on("error", reject); + + const req = client.request({ + ":method": "POST", + ":path": "/test", + }); + + req.on("response", headers => { + expect(headers[":status"]).toBe(200); + }); + + let data = ""; + req.on("data", chunk => { + data += chunk; + }); + + req.on("end", () => { + resolve(data); + client.close(); + }); + + req.on("error", reject); + + // end(data) should send data with END_STREAM + req.end(Buffer.from("Hello!")); + }); + + expect(result).toBe("OK"); + + // Should receive exactly one data frame + expect(receivedDataFrames.length).toBe(1); + expect(receivedDataFrames[0].length).toBe(6); // "Hello!" is 6 bytes + } finally { + server.close(); + } +});