Skip to content

Commit

Permalink
http2: send error text in case of ALPN mismatch
Browse files Browse the repository at this point in the history
Send a human-readable HTTP/1 response in case of an unexpected
ALPN protocol. This helps with debugging this condition,
since previously the only result of it would be a closed socket.

PR-URL: nodejs#18986
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
addaleax committed Mar 5, 2018
1 parent 12856b0 commit 4471369
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
13 changes: 11 additions & 2 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -2488,8 +2488,17 @@ function connectionListener(socket) {
return httpConnectionListener.call(this, socket);
}
// Let event handler deal with the socket
if (!this.emit('unknownProtocol', socket))
socket.destroy();
debug(`Unknown protocol from ${socket.remoteAddress}:${socket.remotePort}`);
if (!this.emit('unknownProtocol', socket)) {
// We don't know what to do, so let's just tell the other side what's
// going on in a format that they *might* understand.
socket.end('HTTP/1.0 403 Forbidden\r\n' +
'Content-Type: text/plain\r\n\r\n' +
'Unknown ALPN Protocol, expected `h2` to be available.\n' +
'If this is a HTTP request: The server was not ' +
'configured with the `allowHTTP1` option or a ' +
'listener for the `unknownProtocol` event.\n');
}
return;
}

Expand Down
13 changes: 10 additions & 3 deletions test/parallel/test-http2-https-fallback.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const fixtures = require('../common/fixtures');
if (!common.hasCrypto)
common.skip('missing crypto');

const { strictEqual } = require('assert');
const { strictEqual, ok } = require('assert');
const { createSecureContext } = require('tls');
const { createSecureServer, connect } = require('http2');
const { get } = require('https');
Expand Down Expand Up @@ -131,10 +131,17 @@ function onSession(session) {

// HTTP/1.1 client
get(Object.assign(parse(origin), clientOptions), common.mustNotCall())
.on('error', common.mustCall(cleanup));
.on('error', common.mustCall(cleanup))
.end();

// Incompatible ALPN TLS client
let text = '';
tls(Object.assign({ port, ALPNProtocols: ['fake'] }, clientOptions))
.on('error', common.mustCall(cleanup));
.setEncoding('utf8')
.on('data', (chunk) => text += chunk)
.on('end', common.mustCall(() => {
ok(/Unknown ALPN Protocol, expected `h2` to be available/.test(text));
cleanup();
}));
}));
}

0 comments on commit 4471369

Please sign in to comment.