From a673c8fe358b1f09f0c8d652d03796ace33cef22 Mon Sep 17 00:00:00 2001 From: bcoe Date: Tue, 21 Apr 2020 15:55:54 -0700 Subject: [PATCH] http2: wait for secureConnect before initializing PR-URL: https://github.com/nodejs/node/pull/32958 Fixes: https://github.com/nodejs/node/issues/32922 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- lib/_tls_wrap.js | 2 + lib/internal/http2/core.js | 2 +- test/internet/test-http2-issue-32922.js | 80 +++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 test/internet/test-http2-issue-32922.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 9957083bb1bf8a..bf3e11f831b80a 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -467,6 +467,7 @@ function TLSSocket(socket, opts) { this._securePending = false; this._newSessionPending = false; this._controlReleased = false; + this.secureConnecting = true; this._SNICallback = null; this.servername = null; this.alpnProtocol = null; @@ -1036,6 +1037,7 @@ function onServerSocketSecure() { if (!this.destroyed && this._releaseControl()) { debug('server emit secureConnection'); + this.secureConnecting = false; this._tlsOptions.server.emit('secureConnection', this); } } diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 3e1bbe3e97638a..e279b0e8f042a9 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1152,7 +1152,7 @@ class Http2Session extends EventEmitter { socket.disableRenegotiation(); const setupFn = setupHandle.bind(this, socket, type, options); - if (socket.connecting) { + if (socket.connecting || socket.secureConnecting) { const connectEvent = socket instanceof tls.TLSSocket ? 'secureConnect' : 'connect'; socket.once(connectEvent, () => { diff --git a/test/internet/test-http2-issue-32922.js b/test/internet/test-http2-issue-32922.js new file mode 100644 index 00000000000000..e11de0286eb7a4 --- /dev/null +++ b/test/internet/test-http2-issue-32922.js @@ -0,0 +1,80 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const http2 = require('http2'); +const net = require('net'); + +const { + HTTP2_HEADER_PATH, +} = http2.constants; + +// Create a normal session, as a control case +function normalSession(cb) { + http2.connect('https://google.com', (clientSession) => { + let error = null; + const req = clientSession.request({ [HTTP2_HEADER_PATH]: '/' }); + req.on('error', (err) => { + error = err; + }); + req.on('response', (_headers) => { + req.on('data', (_chunk) => { }); + req.on('end', () => { + clientSession.close(); + return cb(error); + }); + }); + }); +} +normalSession(common.mustCall(function(err) { + assert.ifError(err); +})); + +// Create a session using a socket that has not yet finished connecting +function socketNotFinished(done) { + const socket2 = net.connect(443, 'google.com'); + http2.connect('https://google.com', { socket2 }, (clientSession) => { + let error = null; + const req = clientSession.request({ [HTTP2_HEADER_PATH]: '/' }); + req.on('error', (err) => { + error = err; + }); + req.on('response', (_headers) => { + req.on('data', (_chunk) => { }); + req.on('end', () => { + clientSession.close(); + socket2.destroy(); + return done(error); + }); + }); + }); +} +socketNotFinished(common.mustCall(function(err) { + assert.ifError(err); +})); + +// Create a session using a socket that has finished connecting +function socketFinished(done) { + const socket = net.connect(443, 'google.com', () => { + http2.connect('https://google.com', { socket }, (clientSession) => { + let error = null; + const req = clientSession.request({ [HTTP2_HEADER_PATH]: '/' }); + req.on('error', (err) => { + error = err; + }); + req.on('response', (_headers) => { + req.on('data', (_chunk) => { }); + req.on('end', () => { + clientSession.close(); + return done(error); + }); + }); + }); + }); +} +socketFinished(common.mustCall(function(err) { + assert.ifError(err); +}));