[v8.x] backport HTTP/2 security release fixups

lib/internal/http2/core.js

@@ -363,23 +363,27 @@ function sessionListenerRemoved(name) {
 // Also keep track of listeners for the Http2Stream instances, as some events
 // are emitted on those objects.
 function streamListenerAdded(name) {
+  const session = this[kSession];
+  if (!session) return;
   switch (name) {
     case 'priority':
-      this[kSession][kNativeFields][kSessionPriorityListenerCount]++;
+      session[kNativeFields][kSessionPriorityListenerCount]++;
       break;
     case 'frameError':
-      this[kSession][kNativeFields][kSessionFrameErrorListenerCount]++;
+      session[kNativeFields][kSessionFrameErrorListenerCount]++;
       break;
   }
 }
 
 function streamListenerRemoved(name) {
+  const session = this[kSession];
+  if (!session) return;
   switch (name) {
     case 'priority':
-      this[kSession][kNativeFields][kSessionPriorityListenerCount]--;
+      session[kNativeFields][kSessionPriorityListenerCount]--;
       break;
     case 'frameError':
-      this[kSession][kNativeFields][kSessionFrameErrorListenerCount]--;
+      session[kNativeFields][kSessionFrameErrorListenerCount]--;
       break;
   }
 }

src/node_http2.cc

@@ -791,8 +791,10 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
   flags_ |= SESSION_STATE_CLOSING;
 
   // Stop reading on the i/o stream
-  if (stream_ != nullptr)
+  if (stream_ != nullptr) {
+    flags_ |= SESSION_STATE_READING_STOPPED;
     stream_->ReadStop();
+  }
 
   // If the socket is not closed, then attempt to send a closing GOAWAY
   // frame. There is no guarantee that this GOAWAY will be received by
@@ -1280,6 +1282,7 @@ inline int Http2Session::OnDataChunkReceived(nghttp2_session* handle,
   // If we are currently waiting for a write operation to finish, we should
   // tell nghttp2 that we want to wait before we process more input data.
   if (session->flags_ & SESSION_STATE_WRITE_IN_PROGRESS) {
+    CHECK_NE(session->flags_ & SESSION_STATE_READING_STOPPED, 0);
     session->flags_ |= SESSION_STATE_NGHTTP2_RECV_PAUSED;
     return NGHTTP2_ERR_PAUSE;
   }
@@ -1626,6 +1629,7 @@ void Http2Session::OnStreamAfterWriteImpl(WriteWrap* w, int status, void* ctx) {
   session->ClearOutgoing(status);
 
   if ((session->flags_ & SESSION_STATE_READING_STOPPED) &&
+      !(session->flags_ & SESSION_STATE_WRITE_IN_PROGRESS) &&
       nghttp2_session_want_read(session->session_)) {
     session->flags_ &= ~SESSION_STATE_READING_STOPPED;
     session->stream_->ReadStart();

test/parallel/test-http2-multistream-destroy-on-read-tls.js

@@ -0,0 +1,47 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const fixtures = require('../common/fixtures');
+const http2 = require('http2');
+
+// Regression test for https://github.com/nodejs/node/issues/29353.
+// Test that it’s okay for an HTTP2 + TLS server to destroy a stream instance
+// while reading it.
+
+const server = http2.createSecureServer({
+  key: fixtures.readKey('agent2-key.pem'),
+  cert: fixtures.readKey('agent2-cert.pem')
+});
+
+const filenames = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+server.on('stream', common.mustCall((stream) => {
+  function write() {
+    stream.write('a'.repeat(10240));
+    stream.once('drain', write);
+  }
+  write();
+}, filenames.length));
+
+server.listen(0, common.mustCall(() => {
+  const client = http2.connect(`https://localhost:${server.address().port}`, {
+    ca: fixtures.readKey('agent2-cert.pem'),
+    servername: 'agent2'
+  });
+
+  let destroyed = 0;
+  for (const entry of filenames) {
+    const stream = client.request({
+      ':path': `/${entry}`
+    });
+    stream.once('data', common.mustCall(() => {
+      stream.destroy();
+
+      if (++destroyed === filenames.length) {
+        client.destroy();
+        server.close();
+      }
+    }));
+  }
+}));

test/parallel/test-http2-stream-removelisteners-after-close.js

@@ -0,0 +1,31 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const http2 = require('http2');
+
+// Regression test for https://github.com/nodejs/node/issues/29457:
+// HTTP/2 stream event listeners can be added and removed after the
+// session has been destroyed.
+
+const server = http2.createServer((req, res) => {
+  res.end('Hi!\n');
+});
+
+server.listen(0, common.mustCall(() => {
+  const client = http2.connect(`http://localhost:${server.address().port}`);
+  const headers = { ':path': '/' };
+  const req = client.request(headers);
+
+  req.on('close', common.mustCall(() => {
+    req.removeAllListeners();
+    req.on('priority', common.mustNotCall());
+    server.close();
+  }));
+
+  req.on('priority', common.mustNotCall());
+  req.on('error', common.mustCall());
+
+  client.destroy();
+}));