Commit 0d6d38f1e53 for nodejs
commit 0d6d38f1e5370e9bdd289b6d87da0e57e6f942f4
Author: Matteo Collina <hello@matteocollina.com>
Date: Mon Nov 9 12:25:14 2026 +0100
http2: retain streams during pending writes
Native file responses create write requests without a JS reference to
the Http2Stream wrapper. Resetting a stream while its socket write is
pending could let GC delete the stream before WriteWrap::OnDone().
Keep a strong stream reference with each outgoing DATA buffer until the
underlying write completes.
Assisted-by: codex:gpt-5.6-sol
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs-private/node-private/pull/1040
Refs: https://hackerone.com/reports/4150891
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
CVE-ID: CVE-2026-123874
diff --git a/src/node_http2.cc b/src/node_http2.cc
index 6b63f06150a..3505d96eba6 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -2184,6 +2184,7 @@ int Http2Session::OnSendData(
if (write.buf.len <= length) {
// This write does not suffice by itself, so we can consume it completely.
length -= write.buf.len;
+ write.stream = stream;
session->PushOutgoingBuffer(std::move(write));
stream->queue_.pop();
continue;
@@ -2191,8 +2192,11 @@ int Http2Session::OnSendData(
// Slice off `length` bytes of the first write in the queue. Retain the
// originating request until the slice is no longer used by the socket.
- session->PushOutgoingBuffer(NgHttp2StreamWrite{
- nullptr, write.buffer_owner, uv_buf_init(write.buf.base, length)});
+ session->PushOutgoingBuffer(
+ NgHttp2StreamWrite{nullptr,
+ write.buffer_owner,
+ stream,
+ uv_buf_init(write.buf.base, length)});
write.buf.base += length;
write.buf.len -= length;
break;
@@ -2290,8 +2294,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
bool Http2Session::HasWritesOnSocketForStream(Http2Stream* stream) {
for (const NgHttp2StreamWrite& wr : outgoing_buffers_) {
- if (wr.req_wrap && WriteWrap::FromObject(wr.req_wrap)->stream() == stream)
- return true;
+ if (wr.stream.get() == stream) return true;
}
return false;
}
@@ -3621,6 +3624,7 @@ void NgHttp2StreamWrite::MemoryInfo(MemoryTracker* tracker) const {
if (req_wrap)
tracker->TrackField("req_wrap", req_wrap);
if (buffer_owner) tracker->TrackField("buffer_owner", buffer_owner);
+ if (stream) tracker->TrackField("stream", stream);
tracker->TrackField("buf", buf);
}
diff --git a/src/node_http2.h b/src/node_http2.h
index 24bc6bb128e..48dab953770 100644
--- a/src/node_http2.h
+++ b/src/node_http2.h
@@ -163,11 +163,14 @@ struct Http2RcBufferPointerTraits {
using Http2Headers = NgHeaders<Http2HeadersTraits>;
using Http2RcBufferPointer = NgRcBufPointer<Http2RcBufferPointerTraits>;
+class Http2Stream;
+
struct NgHttp2StreamWrite : public MemoryRetainer {
// Completed once all buffers from the request have been consumed.
BaseObjectPtr<AsyncWrap> req_wrap;
// Retains the request's buffers without completing it.
BaseObjectPtr<AsyncWrap> buffer_owner;
+ BaseObjectPtr<Http2Stream> stream;
uv_buf_t buf;
inline explicit NgHttp2StreamWrite(uv_buf_t buf_) : buf(buf_) {}
@@ -177,6 +180,14 @@ struct NgHttp2StreamWrite : public MemoryRetainer {
: req_wrap(std::move(req_wrap)),
buffer_owner(std::move(buffer_owner)),
buf(buf_) {}
+ inline NgHttp2StreamWrite(BaseObjectPtr<AsyncWrap> req_wrap,
+ BaseObjectPtr<AsyncWrap> buffer_owner,
+ BaseObjectPtr<Http2Stream> stream,
+ uv_buf_t buf_)
+ : req_wrap(std::move(req_wrap)),
+ buffer_owner(std::move(buffer_owner)),
+ stream(std::move(stream)),
+ buf(buf_) {}
void MemoryInfo(MemoryTracker* tracker) const override;
SET_MEMORY_INFO_NAME(NgHttp2StreamWrite)
@@ -189,7 +200,6 @@ typedef uint32_t(*get_setting)(nghttp2_session* session,
class Http2Ping;
class Http2Session;
class Http2Settings;
-class Http2Stream;
class Origins;
// This scope should be present when any call into nghttp2 that may schedule
diff --git a/test/parallel/test-http2-file-reset-pending-write.js b/test/parallel/test-http2-file-reset-pending-write.js
new file mode 100644
index 00000000000..6d9e5411ed7
--- /dev/null
+++ b/test/parallel/test-http2-file-reset-pending-write.js
@@ -0,0 +1,87 @@
+// Flags: --expose-gc
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+ common.skip('missing crypto');
+
+const assert = require('assert');
+const fs = require('fs');
+const http2 = require('http2');
+const { duplexPair } = require('stream');
+const { setImmediate } = require('timers/promises');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+const filename = tmpdir.resolve('http2-file');
+fs.writeFileSync(filename, Buffer.alloc(1024 * 1024));
+
+async function test(useFD) {
+ const [clientSide, serverSide] = duplexPair();
+ let reset = false;
+ let pendingWrite;
+ const originalWrite = serverSide._write;
+ serverSide._write = common.mustCallAtLeast(function(buf, enc, cb) {
+ originalWrite.call(this, buf, enc, common.mustCall(async () => {
+ // Deliver the data, but allow the client to reset the stream before
+ // completing the corresponding native write.
+ await setImmediate();
+ if (reset) {
+ assert.strictEqual(pendingWrite, undefined);
+ pendingWrite = cb;
+ } else {
+ cb();
+ }
+ }));
+ });
+
+ const server = http2.createServer();
+ const fd = useFD ? fs.openSync(filename, 'r') : undefined;
+ let streamClosed;
+ const closed = new Promise((resolve) => { streamClosed = resolve; });
+ server.on('stream', common.mustCall((stream) => {
+ stream.on('close', common.mustCall(streamClosed));
+ if (useFD)
+ stream.respondWithFD(fd);
+ else
+ stream.respondWithFile(filename);
+ }));
+ server.emit('connection', serverSide);
+
+ const client = http2.connect('http://localhost', {
+ createConnection: () => clientSide,
+ settings: { initialWindowSize: 1024 * 1024 },
+ });
+ client.on('connect', common.mustCall(() => {
+ client.setLocalWindowSize(1024 * 1024);
+ }));
+ const req = client.request();
+ req.once('data', common.mustCall(() => {
+ reset = true;
+ req.close(http2.constants.NGHTTP2_CANCEL);
+ }));
+ req.resume();
+
+ await closed;
+ // Let deferred native stream cleanup and unpipe callbacks run, then collect
+ // the wrapper while its file write is still awaiting completion.
+ for (let i = 0; i < 10; i++) {
+ await setImmediate();
+ global.gc();
+ }
+ assert.strictEqual(typeof pendingWrite, 'function');
+ reset = false;
+ pendingWrite();
+ pendingWrite = undefined;
+ await setImmediate();
+
+ client.destroy();
+ server.close();
+ if (useFD)
+ fs.closeSync(fd);
+}
+
+(async () => {
+ await test(false);
+ await test(true);
+})().then(common.mustCall());