From dccf7775e29ae7c91a51f9665ebe23289158642c Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Tue, 16 May 2017 16:16:07 +0800 Subject: [PATCH 1/8] http2: ALPN fallback to HTTP/1.1 --- lib/internal/http2/core.js | 9 +++- ...test-http2-create-client-secure-session.js | 4 +- test/parallel/test-http2-https-fallback.js | 52 +++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-http2-https-fallback.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index d054da75ec..72b9bf86cb 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -13,6 +13,7 @@ const { Duplex } = require('stream'); const { URL } = require('url'); const { onServerStream } = require('internal/http2/compat'); const { utcDate } = require('internal/http'); +const { _connectionListener: httpConnectionListener } = require('http'); const { assertIsObject, @@ -1255,6 +1256,11 @@ function connectionListener(socket) { socket.on('timeout', socketOnTimeout); } + // TLS ALPN fallback to HTTP/1.1 + if (socket.alpnProtocol === false || socket.alpnProtocol === 'http/1.1') { + return httpConnectionListener.call(this, socket); + } + socket.on('error', socketOnError); socket.on('resume', socketOnResume); socket.on('pause', socketOnPause); @@ -1287,8 +1293,7 @@ function initializeOptions(options) { function initializeTLSOptions(options, servername) { options = initializeOptions(options); - options.ALPNProtocols = ['hc', 'h2']; - options.NPNProtocols = ['hc', 'h2']; + options.ALPNProtocols = ['h2', 'http/1.1']; if (servername !== undefined && options.servername === undefined) { options.servername = servername; } diff --git a/test/parallel/test-http2-create-client-secure-session.js b/test/parallel/test-http2-create-client-secure-session.js index e4330f85ba..7ab752e323 100644 --- a/test/parallel/test-http2-create-client-secure-session.js +++ b/test/parallel/test-http2-create-client-secure-session.js @@ -50,8 +50,7 @@ function verifySecureSession(key, cert, ca, opts) { req.on('end', common.mustCall(() => { const jsonData = JSON.parse(data); assert.strictEqual(jsonData.servername, opts.servername || 'localhost'); - assert( - jsonData.alpnProtocol === 'h2' || jsonData.alpnProtocol === 'hc'); + assert.strictEqual(jsonData.alpnProtocol, 'h2'); server.close(); client.socket.destroy(); })); @@ -66,7 +65,6 @@ verifySecureSession( loadKey('agent8-cert.pem'), loadKey('fake-startcom-root-cert.pem')); - // Custom servername is specified. verifySecureSession( loadKey('agent1-key.pem'), diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js new file mode 100644 index 0000000000..1ca16b2783 --- /dev/null +++ b/test/parallel/test-http2-https-fallback.js @@ -0,0 +1,52 @@ +'use strict'; + +const { fixturesDir, mustCall } = require('../common'); +const { strictEqual } = require('assert'); +const { join } = require('path'); +const { readFileSync } = require('fs'); +const { createSecureContext } = require('tls'); +const { createSecureServer } = require('http2'); +const { get } = require('https'); +const { parse } = require('url'); + +const key = loadKey('agent8-key.pem'); +const cert = loadKey('agent8-cert.pem'); +const ca = loadKey('fake-startcom-root-cert.pem'); + +function loadKey(keyname) { + return readFileSync( + join(fixturesDir, 'keys', keyname), 'binary'); +} + +const server = createSecureServer( + { cert, key }, + mustCall((request, response) => { + response.writeHead(200, 'OK', { 'content-type': 'application/json' }); + response.end(JSON.stringify({ alpnProtocol: request.socket.alpnProtocol })); + }) +); + +server.listen(0); + +server.on('listening', mustCall(() => { + const clientOptions = Object.assign( + { secureContext: createSecureContext({ ca }) }, + parse(`https://localhost:${server.address().port}`) + ); + + get(clientOptions, (response) => { + strictEqual(response.statusCode, 200); + strictEqual(response.statusMessage, 'OK'); + strictEqual(response.headers['content-type'], 'application/json'); + + response.setEncoding('utf8'); + let raw = ''; + response.on('data', (chunk) => { raw += chunk; }); + response.on('end', mustCall(() => { + const data = JSON.parse(raw); + strictEqual(data.alpnProtocol, false); + + server.close(); + })); + }); +})); From 2d6bece50b6e7e367625165586309d00c4c5cf98 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Wed, 17 May 2017 00:13:46 +0800 Subject: [PATCH 2/8] http2: allowHTTP1 option, more tests, compat fix (Squash) --- lib/internal/http2/compat.js | 4 + lib/internal/http2/core.js | 3 +- test/parallel/test-http2-https-fallback.js | 130 ++++++++++++++++----- 3 files changed, 105 insertions(+), 32 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 9b77ba3f1a..78742587b6 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -181,6 +181,10 @@ class Http2ServerRequest extends Readable { return '2.0'; } + get socket() { + return this.stream.session.socket; + } + _read(nread) { var stream = this[kStream]; if (stream) { diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 72b9bf86cb..2f7bb83432 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1257,7 +1257,8 @@ function connectionListener(socket) { } // TLS ALPN fallback to HTTP/1.1 - if (socket.alpnProtocol === false || socket.alpnProtocol === 'http/1.1') { + if ((options.allowHTTP1 === undefined || !!options.allowHTTP1 === true) && + (socket.alpnProtocol === false || socket.alpnProtocol === 'http/1.1')) { return httpConnectionListener.call(this, socket); } diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js index 1ca16b2783..b0438560fb 100644 --- a/test/parallel/test-http2-https-fallback.js +++ b/test/parallel/test-http2-https-fallback.js @@ -1,11 +1,11 @@ 'use strict'; -const { fixturesDir, mustCall } = require('../common'); +const { fixturesDir, mustCall, mustNotCall } = require('../common'); const { strictEqual } = require('assert'); const { join } = require('path'); const { readFileSync } = require('fs'); const { createSecureContext } = require('tls'); -const { createSecureServer } = require('http2'); +const { createSecureServer, connect } = require('http2'); const { get } = require('https'); const { parse } = require('url'); @@ -18,35 +18,103 @@ function loadKey(keyname) { join(fixturesDir, 'keys', keyname), 'binary'); } -const server = createSecureServer( - { cert, key }, - mustCall((request, response) => { - response.writeHead(200, 'OK', { 'content-type': 'application/json' }); - response.end(JSON.stringify({ alpnProtocol: request.socket.alpnProtocol })); - }) -); +// HTTP/2 & HTTP/1.1 server +{ + const server = createSecureServer( + { cert, key }, + mustCall((request, response) => { + response.writeHead(200, { 'content-type': 'application/json' }); + response.end(JSON.stringify({ + alpnProtocol: request.socket.alpnProtocol, + httpVersion: request.httpVersion + })); + }, 2) + ); -server.listen(0); + server.listen(0); -server.on('listening', mustCall(() => { - const clientOptions = Object.assign( - { secureContext: createSecureContext({ ca }) }, - parse(`https://localhost:${server.address().port}`) - ); + server.on('listening', mustCall(() => { + const port = server.address().port; + const origin = `https://localhost:${port}`; + const clientOptions = { secureContext: createSecureContext({ ca }) }; + + let count = 2; + + // HTTP/2 client + connect( + origin, + { secureContext: createSecureContext({ ca }) }, + mustCall((session) => { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'https', + ':authority': `localhost:${port}` + }; + + const request = session.request(headers); + request.on('response', mustCall((headers) => { + strictEqual(headers[':status'], '200'); + strictEqual(headers['content-type'], 'application/json'); + })); + request.setEncoding('utf8'); + let raw = ''; + request.on('data', (chunk) => { raw += chunk; }); + request.on('end', mustCall(() => { + const data = JSON.parse(raw); + strictEqual(data.alpnProtocol, 'h2'); + strictEqual(data.httpVersion, '2.0'); + + session.destroy(); + if (--count === 0) server.close(); + })); + request.end(); + }) + ); + + // HTTP/1.1 client + get( + Object.assign(parse(origin), clientOptions), + mustCall((response) => { + strictEqual(response.statusCode, 200); + strictEqual(response.statusMessage, 'OK'); + strictEqual(response.headers['content-type'], 'application/json'); + + response.setEncoding('utf8'); + let raw = ''; + response.on('data', (chunk) => { raw += chunk; }); + response.on('end', mustCall(() => { + const data = JSON.parse(raw); + strictEqual(data.alpnProtocol, false); + strictEqual(data.httpVersion, '1.1'); - get(clientOptions, (response) => { - strictEqual(response.statusCode, 200); - strictEqual(response.statusMessage, 'OK'); - strictEqual(response.headers['content-type'], 'application/json'); - - response.setEncoding('utf8'); - let raw = ''; - response.on('data', (chunk) => { raw += chunk; }); - response.on('end', mustCall(() => { - const data = JSON.parse(raw); - strictEqual(data.alpnProtocol, false); - - server.close(); - })); - }); -})); + if (--count === 0) server.close(); + })); + }) + ); + })); +} + +// HTTP/2-only server +{ + const server = createSecureServer({ cert, key, allowHTTP1: false }); + + server.listen(0); + + server.on('listening', mustCall(() => { + const port = server.address().port; + const origin = `https://localhost:${port}`; + const clientOptions = { secureContext: createSecureContext({ ca }) }; + + // HTTP/2 client + connect( + origin, + { secureContext: createSecureContext({ ca }) }, + mustCall((session) => { session.destroy(); }) + ); + + // HTTP/1.1 client + get(Object.assign(parse(origin), clientOptions), mustNotCall()) + .on('error', mustCall(server.close.bind(server))); + })); +} From 2f330b086a987a4c1bae4c07c8e27a76c4de5843 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Wed, 17 May 2017 00:23:49 +0800 Subject: [PATCH 3/8] =?UTF-8?q?http2:=20Fixed=20race=20condition=20?= =?UTF-8?q?=F0=9F=99=80=20(Squash)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/parallel/test-http2-https-fallback.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js index b0438560fb..56c8e651eb 100644 --- a/test/parallel/test-http2-https-fallback.js +++ b/test/parallel/test-http2-https-fallback.js @@ -106,15 +106,22 @@ function loadKey(keyname) { const origin = `https://localhost:${port}`; const clientOptions = { secureContext: createSecureContext({ ca }) }; + let count = 2; + // HTTP/2 client connect( origin, { secureContext: createSecureContext({ ca }) }, - mustCall((session) => { session.destroy(); }) + mustCall((session) => { + session.destroy(); + if (--count === 0) server.close(); + }) ); // HTTP/1.1 client get(Object.assign(parse(origin), clientOptions), mustNotCall()) - .on('error', mustCall(server.close.bind(server))); + .on('error', mustCall(() => { + if (--count === 0) server.close(); + })); })); } From dd2f033b31eea2caebab96c587acb5ed2496e993 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Wed, 17 May 2017 01:23:22 +0800 Subject: [PATCH 4/8] http2: Minor refactoring (Squash) --- test/parallel/test-http2-https-fallback.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js index 56c8e651eb..7468afe17a 100644 --- a/test/parallel/test-http2-https-fallback.js +++ b/test/parallel/test-http2-https-fallback.js @@ -109,14 +109,10 @@ function loadKey(keyname) { let count = 2; // HTTP/2 client - connect( - origin, - { secureContext: createSecureContext({ ca }) }, - mustCall((session) => { - session.destroy(); - if (--count === 0) server.close(); - }) - ); + connect(origin, clientOptions, mustCall((session) => { + session.destroy(); + if (--count === 0) server.close(); + })); // HTTP/1.1 client get(Object.assign(parse(origin), clientOptions), mustNotCall()) From 4e869cbdd8c5000dfef1e02605f4dc81a8982149 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Wed, 17 May 2017 11:26:13 +0800 Subject: [PATCH 5/8] http2: Full test for h2 client against h2-only server (Squash) --- test/parallel/test-http2-https-fallback.js | 113 +++++++++++---------- 1 file changed, 60 insertions(+), 53 deletions(-) diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js index 7468afe17a..14af3c33e9 100644 --- a/test/parallel/test-http2-https-fallback.js +++ b/test/parallel/test-http2-https-fallback.js @@ -9,26 +9,58 @@ const { createSecureServer, connect } = require('http2'); const { get } = require('https'); const { parse } = require('url'); +const countdown = (count, done) => () => --count === 0 && done(); + +function loadKey(keyname) { + return readFileSync(join(fixturesDir, 'keys', keyname)); +} + const key = loadKey('agent8-key.pem'); const cert = loadKey('agent8-cert.pem'); const ca = loadKey('fake-startcom-root-cert.pem'); -function loadKey(keyname) { - return readFileSync( - join(fixturesDir, 'keys', keyname), 'binary'); +const clientOptions = { secureContext: createSecureContext({ ca }) }; + +function onRequest(request, response) { + response.writeHead(200, { 'content-type': 'application/json' }); + response.end(JSON.stringify({ + alpnProtocol: request.socket.alpnProtocol, + httpVersion: request.httpVersion + })); +} + +function onSession(session) { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'https', + ':authority': `localhost:${this.server.address().port}` + }; + + const request = session.request(headers); + request.on('response', mustCall((headers) => { + strictEqual(headers[':status'], '200'); + strictEqual(headers['content-type'], 'application/json'); + })); + request.setEncoding('utf8'); + let raw = ''; + request.on('data', (chunk) => { raw += chunk; }); + request.on('end', mustCall(() => { + const { alpnProtocol, httpVersion } = JSON.parse(raw); + strictEqual(alpnProtocol, 'h2'); + strictEqual(httpVersion, '2.0'); + + session.destroy(); + this.cleanup(); + })); + request.end(); } // HTTP/2 & HTTP/1.1 server { const server = createSecureServer( { cert, key }, - mustCall((request, response) => { - response.writeHead(200, { 'content-type': 'application/json' }); - response.end(JSON.stringify({ - alpnProtocol: request.socket.alpnProtocol, - httpVersion: request.httpVersion - })); - }, 2) + mustCall(onRequest, 2) ); server.listen(0); @@ -36,40 +68,14 @@ function loadKey(keyname) { server.on('listening', mustCall(() => { const port = server.address().port; const origin = `https://localhost:${port}`; - const clientOptions = { secureContext: createSecureContext({ ca }) }; - let count = 2; + const cleanup = countdown(2, () => server.close()); // HTTP/2 client connect( origin, - { secureContext: createSecureContext({ ca }) }, - mustCall((session) => { - const headers = { - ':path': '/', - ':method': 'GET', - ':scheme': 'https', - ':authority': `localhost:${port}` - }; - - const request = session.request(headers); - request.on('response', mustCall((headers) => { - strictEqual(headers[':status'], '200'); - strictEqual(headers['content-type'], 'application/json'); - })); - request.setEncoding('utf8'); - let raw = ''; - request.on('data', (chunk) => { raw += chunk; }); - request.on('end', mustCall(() => { - const data = JSON.parse(raw); - strictEqual(data.alpnProtocol, 'h2'); - strictEqual(data.httpVersion, '2.0'); - - session.destroy(); - if (--count === 0) server.close(); - })); - request.end(); - }) + clientOptions, + mustCall(onSession.bind({ cleanup, server })) ); // HTTP/1.1 client @@ -84,11 +90,11 @@ function loadKey(keyname) { let raw = ''; response.on('data', (chunk) => { raw += chunk; }); response.on('end', mustCall(() => { - const data = JSON.parse(raw); - strictEqual(data.alpnProtocol, false); - strictEqual(data.httpVersion, '1.1'); + const { alpnProtocol, httpVersion } = JSON.parse(raw); + strictEqual(alpnProtocol, false); + strictEqual(httpVersion, '1.1'); - if (--count === 0) server.close(); + cleanup(); })); }) ); @@ -97,27 +103,28 @@ function loadKey(keyname) { // HTTP/2-only server { - const server = createSecureServer({ cert, key, allowHTTP1: false }); + const server = createSecureServer( + { cert, key, allowHTTP1: false }, + mustCall(onRequest) + ); server.listen(0); server.on('listening', mustCall(() => { const port = server.address().port; const origin = `https://localhost:${port}`; - const clientOptions = { secureContext: createSecureContext({ ca }) }; - let count = 2; + const cleanup = countdown(2, () => server.close()); // HTTP/2 client - connect(origin, clientOptions, mustCall((session) => { - session.destroy(); - if (--count === 0) server.close(); - })); + connect( + origin, + clientOptions, + mustCall(onSession.bind({ cleanup, server })) + ); // HTTP/1.1 client get(Object.assign(parse(origin), clientOptions), mustNotCall()) - .on('error', mustCall(() => { - if (--count === 0) server.close(); - })); + .on('error', mustCall(cleanup)); })); } From 05951eeeafde676cff9763887e81ec4bf643be38 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Wed, 17 May 2017 16:37:16 +0800 Subject: [PATCH 6/8] http2: Following feedback from @mcollina (Squash) --- lib/internal/http2/compat.js | 4 ---- lib/internal/http2/core.js | 5 +++-- test/parallel/test-http2-https-fallback.js | 8 +++++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 78742587b6..9b77ba3f1a 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -181,10 +181,6 @@ class Http2ServerRequest extends Readable { return '2.0'; } - get socket() { - return this.stream.session.socket; - } - _read(nread) { var stream = this[kStream]; if (stream) { diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 2f7bb83432..b1aa068c58 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1257,8 +1257,9 @@ function connectionListener(socket) { } // TLS ALPN fallback to HTTP/1.1 - if ((options.allowHTTP1 === undefined || !!options.allowHTTP1 === true) && - (socket.alpnProtocol === false || socket.alpnProtocol === 'http/1.1')) { + if (options.allowHTTP1 === true && + (socket.alpnProtocol === false || + socket.alpnProtocol === 'http/1.1')) { return httpConnectionListener.call(this, socket); } diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js index 14af3c33e9..bb73de2364 100644 --- a/test/parallel/test-http2-https-fallback.js +++ b/test/parallel/test-http2-https-fallback.js @@ -22,9 +22,11 @@ const ca = loadKey('fake-startcom-root-cert.pem'); const clientOptions = { secureContext: createSecureContext({ ca }) }; function onRequest(request, response) { + const { socket: { alpnProtocol } } = request.httpVersion === '2.0' ? + request.stream.session : request; response.writeHead(200, { 'content-type': 'application/json' }); response.end(JSON.stringify({ - alpnProtocol: request.socket.alpnProtocol, + alpnProtocol, httpVersion: request.httpVersion })); } @@ -59,7 +61,7 @@ function onSession(session) { // HTTP/2 & HTTP/1.1 server { const server = createSecureServer( - { cert, key }, + { cert, key, allowHTTP1: true }, mustCall(onRequest, 2) ); @@ -104,7 +106,7 @@ function onSession(session) { // HTTP/2-only server { const server = createSecureServer( - { cert, key, allowHTTP1: false }, + { cert, key }, mustCall(onRequest) ); From c2bd45912d0a6eecb2389549c65fd05367604200 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Thu, 18 May 2017 00:59:27 +0800 Subject: [PATCH 7/8] http2: Added allowHTTP1 option to docs (Squash) --- doc/api/http2.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/api/http2.md b/doc/api/http2.md index 04ff7e56bd..988ee012d0 100755 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -611,6 +611,7 @@ server.listen(80); * `noRecvClientMagic` {boolean} (TODO: Add detail) * `paddingStrategy` {number} (TODO: Add detail) * `peerMaxConcurrentStreams` {number} (TODO: Add detail) + * `allowHTTP1` {boolean} Incoming client connections that do not support HTTP/2 will be downgraded to HTTP/1.x when set to `true`. The default value is `false`, which rejects non-HTTP/2 client connections. * `settings` {[Settings Object][]} The initial settings to send to the remote peer upon connection. * ...: Any [`tls.createServer()`][] options can be provided. For From e9d7e5e14270fe5a174404a1eb95623e358b2530 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Thu, 18 May 2017 08:26:38 +0800 Subject: [PATCH 8/8] http2: fix line wrap (Squash) --- doc/api/http2.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index 988ee012d0..28ed4fb836 100755 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -611,7 +611,9 @@ server.listen(80); * `noRecvClientMagic` {boolean} (TODO: Add detail) * `paddingStrategy` {number} (TODO: Add detail) * `peerMaxConcurrentStreams` {number} (TODO: Add detail) - * `allowHTTP1` {boolean} Incoming client connections that do not support HTTP/2 will be downgraded to HTTP/1.x when set to `true`. The default value is `false`, which rejects non-HTTP/2 client connections. + * `allowHTTP1` {boolean} Incoming client connections that do not support + HTTP/2 will be downgraded to HTTP/1.x when set to `true`. The default value + is `false`, which rejects non-HTTP/2 client connections. * `settings` {[Settings Object][]} The initial settings to send to the remote peer upon connection. * ...: Any [`tls.createServer()`][] options can be provided. For