From ab2b066fc117345aa3c0973498ab2d1d361c646b Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Thu, 5 Nov 2020 23:56:00 +0100 Subject: [PATCH] http2: delay session.receive() by a tick PR-URL: https://github.com/nodejs/node/pull/35985 Reviewed-By: Matteo Collina Reviewed-By: Rich Trott --- lib/internal/http2/core.js | 23 ++++++---- .../test-http2-connect-tls-with-delay.js | 46 +++++++------------ 2 files changed, 30 insertions(+), 39 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7260f59868e63a..afd7cd28aafec2 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -3105,16 +3105,21 @@ function connect(authority, options, listener) { if (typeof listener === 'function') session.once('connect', listener); - debug('Http2Session connect', options.createConnection); - // Socket already has some buffered data - emulate receiving it - // https://github.com/nodejs/node/issues/35475 - if (socket && socket.readableLength) { - let buf; - while ((buf = socket.read()) !== null) { - debug(`Http2Session connect: injecting ${buf.length} already in buffer`); - session[kHandle].receive(buf); + // Process data on the next tick - a remoteSettings handler may be attached. + // https://github.com/nodejs/node/issues/35981 + process.nextTick(() => { + debug('Http2Session connect', options.createConnection); + // Socket already has some buffered data - emulate receiving it + // https://github.com/nodejs/node/issues/35475 + if (socket && socket.readableLength) { + let buf; + while ((buf = socket.read()) !== null) { + debug(`Http2Session connect: ${buf.length} bytes already in buffer`); + session[kHandle].receive(buf); + } } - } + }); + return session; } diff --git a/test/parallel/test-http2-connect-tls-with-delay.js b/test/parallel/test-http2-connect-tls-with-delay.js index 3e2e8a46a3662a..0b3753ae383642 100644 --- a/test/parallel/test-http2-connect-tls-with-delay.js +++ b/test/parallel/test-http2-connect-tls-with-delay.js @@ -4,11 +4,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); -if (!common.hasMultiLocalhost()) - common.skip('platform-specific test.'); - const http2 = require('http2'); -const assert = require('assert'); const tls = require('tls'); const fixtures = require('../common/fixtures'); @@ -16,15 +12,9 @@ const serverOptions = { key: fixtures.readKey('agent1-key.pem'), cert: fixtures.readKey('agent1-cert.pem') }; -const server = http2.createSecureServer(serverOptions, (req, res) => { - console.log(`Connect from: ${req.connection.remoteAddress}`); - assert.strictEqual(req.connection.remoteAddress, '127.0.0.2'); - req.on('end', common.mustCall(() => { - res.writeHead(200, { 'Content-Type': 'text/plain' }); - res.end(`You are from: ${req.connection.remoteAddress}`); - })); - req.resume(); +const server = http2.createSecureServer(serverOptions, (req, res) => { + res.end(); }); server.listen(0, '127.0.0.1', common.mustCall(() => { @@ -32,33 +22,29 @@ server.listen(0, '127.0.0.1', common.mustCall(() => { ALPNProtocols: ['h2'], host: '127.0.0.1', servername: 'localhost', - localAddress: '127.0.0.2', port: server.address().port, rejectUnauthorized: false }; - console.log('Server ready', server.address().port); - const socket = tls.connect(options, async () => { - - console.log('TLS Connected!'); - - setTimeout(() => { - + socket.once('readable', () => { const client = http2.connect( 'https://localhost:' + server.address().port, { ...options, createConnection: () => socket } ); - const req = client.request({ - ':path': '/' - }); - req.on('data', () => req.resume()); - req.on('end', common.mustCall(function() { - client.close(); - req.close(); - server.close(); + + client.once('remoteSettings', common.mustCall(() => { + const req = client.request({ + ':path': '/' + }); + req.on('data', () => req.resume()); + req.on('end', common.mustCall(() => { + client.close(); + req.close(); + server.close(); + })); + req.end(); })); - req.end(); - }, 1000); + }); }); }));