Skip to content

Commit

Permalink
http2: allow Host in HTTP/2 requests
Browse files Browse the repository at this point in the history
The HTTP/2 spec allows Host to be used instead of :authority in
requests, and this is in fact *preferred* when converting from HTTP/1.

We erroneously treated Host as a connection header, thus disallowing
it in requests. The patch corrects this, aligning Node.js behaviour
with the HTTP/2 spec and with nghttp2:

 - Treat Host as a single-value header instead of a connection header.
 - Don't autofill :authority if Host is present.
 - The compatibility API (request.authority) falls back to using Host
   if :authority is not present.

This is semver-major because requests are no longer guaranteed to
have :authority set. An explanatory note was added to the docs.

Fixes: nodejs#29858
  • Loading branch information
mildsunrise committed Aug 7, 2020
1 parent df17fcd commit 014c725
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 14 deletions.
31 changes: 28 additions & 3 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
<!-- YAML
added: v8.4.0
changes:
- version: REPLACEME
pr-url: REPLACEME
description: Requests with the `host` header (with or without
`:authority`) can now be sent / received.
- version: v10.10.0
pr-url: https://github.com/nodejs/node/pull/22466
description: HTTP/2 is now Stable. Previously, it had been Experimental.
Expand Down Expand Up @@ -2530,7 +2534,7 @@ For incoming headers:
`access-control-max-age`, `access-control-request-method`, `content-encoding`,
`content-language`, `content-length`, `content-location`, `content-md5`,
`content-range`, `content-type`, `date`, `dnt`, `etag`, `expires`, `from`,
`if-match`, `if-modified-since`, `if-none-match`, `if-range`,
`host`, `if-match`, `if-modified-since`, `if-none-match`, `if-range`,
`if-unmodified-since`, `last-modified`, `location`, `max-forwards`,
`proxy-authorization`, `range`, `referer`,`retry-after`, `tk`,
`upgrade-insecure-requests`, `user-agent` or `x-content-type-options` are
Expand Down Expand Up @@ -2908,8 +2912,9 @@ added: v8.4.0

* {string}

The request authority pseudo header field. It can also be accessed via
`req.headers[':authority']`.
The request authority pseudo header field. Because HTTP/2 allows requests
to set either `:authority` or `host`, this value is derived from
`req.headers[':authority']` if present, `req.headers['host']` otherwise.

#### `request.complete`
<!-- YAML
Expand Down Expand Up @@ -3708,6 +3713,25 @@ following additional properties:
* `type` {string} Either `'server'` or `'client'` to identify the type of
`Http2Session`.

## Note on `:authority` and `host`

HTTP/2 requires requests to have either the `:authority` pseudo-header
or the `host` header. Using `:authority` should be preferred when
constructing an HTTP/2 request directly, and `host` should be
preferred when converting from HTTP/1 (in proxies, for instance).

Historically, Node.js required all requests to have the `:authority`
pseudo-header and rejected with the `host` header present.
This has been corrected; the `host` header is now allowed, and the
only requirement is for either `host` or `:authority` to be
present (or both). The same validations from `:authority` extend
to `host`.

The compatibility API falls back to `host` if `:authority` is not
present, see [`request.authority`][]. However, if you don't use the
compatibility API (or use `req.headers` directly), you need to
implement any fall-back behaviour yourself.

[ALPN Protocol ID]: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
[ALPN negotiation]: #http2_alpn_negotiation
[Compatibility API]: #http2_compatibility_api
Expand Down Expand Up @@ -3748,6 +3772,7 @@ following additional properties:
[`net.Socket.prototype.unref()`]: net.html#net_socket_unref
[`net.Socket`]: net.html#net_class_net_socket
[`net.connect()`]: net.html#net_net_connect
[`request.authority`]: #http2_request_authority
[`request.socket`]: #http2_request_socket
[`request.socket.getPeerCertificate()`]: tls.html#tls_tlssocket_getpeercertificate_detailed
[`response.end()`]: #http2_response_end_data_encoding_callback
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ const {
kSocket,
kRequest,
kProxySocket,
assertValidPseudoHeader
assertValidPseudoHeader,
getAuthority
} = require('internal/http2/util');
const { _checkIsHttpToken: checkIsHttpToken } = require('_http_common');

Expand Down Expand Up @@ -395,7 +396,7 @@ class Http2ServerRequest extends Readable {
}

get authority() {
return this[kHeaders][HTTP2_HEADER_AUTHORITY];
return getAuthority(this[kHeaders]);
}

get scheme() {
Expand Down
9 changes: 5 additions & 4 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ const {
assertValidPseudoHeaderResponse,
assertValidPseudoHeaderTrailer,
assertWithinRange,
getAuthority,
getDefaultSettings,
getSessionState,
getSettings,
Expand Down Expand Up @@ -1633,7 +1634,7 @@ class ClientHttp2Session extends Http2Session {
const connect = headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_CONNECT;

if (!connect || headers[HTTP2_HEADER_PROTOCOL] !== undefined) {
if (headers[HTTP2_HEADER_AUTHORITY] === undefined)
if (getAuthority(headers) === undefined)
headers[HTTP2_HEADER_AUTHORITY] = this[kAuthority];
if (headers[HTTP2_HEADER_SCHEME] === undefined)
headers[HTTP2_HEADER_SCHEME] = this[kProtocol].slice(0, -1);
Expand Down Expand Up @@ -1664,7 +1665,7 @@ class ClientHttp2Session extends Http2Session {
const stream = new ClientHttp2Stream(this, undefined, undefined, {});
stream[kSentHeaders] = headers;
stream[kOrigin] = `${headers[HTTP2_HEADER_SCHEME]}://` +
`${headers[HTTP2_HEADER_AUTHORITY]}`;
`${getAuthority(headers)}`;

// Close the writable side of the stream if options.endStream is set.
if (options.endStream)
Expand Down Expand Up @@ -2456,7 +2457,7 @@ class ServerHttp2Stream extends Http2Stream {
handle.owner = this;
this[kInit](id, handle);
this[kProtocol] = headers[HTTP2_HEADER_SCHEME];
this[kAuthority] = headers[HTTP2_HEADER_AUTHORITY];
this[kAuthority] = getAuthority(headers);
}

// True if the remote peer accepts push streams
Expand Down Expand Up @@ -2499,7 +2500,7 @@ class ServerHttp2Stream extends Http2Stream {

if (headers[HTTP2_HEADER_METHOD] === undefined)
headers[HTTP2_HEADER_METHOD] = HTTP2_METHOD_GET;
if (headers[HTTP2_HEADER_AUTHORITY] === undefined)
if (getAuthority(headers) === undefined)
headers[HTTP2_HEADER_AUTHORITY] = this[kAuthority];
if (headers[HTTP2_HEADER_SCHEME] === undefined)
headers[HTTP2_HEADER_SCHEME] = this[kProtocol];
Expand Down
16 changes: 14 additions & 2 deletions lib/internal/http2/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const {
HTTP2_HEADER_ETAG,
HTTP2_HEADER_EXPIRES,
HTTP2_HEADER_FROM,
HTTP2_HEADER_HOST,
HTTP2_HEADER_IF_MATCH,
HTTP2_HEADER_IF_NONE_MATCH,
HTTP2_HEADER_IF_MODIFIED_SINCE,
Expand All @@ -84,7 +85,6 @@ const {
HTTP2_HEADER_HTTP2_SETTINGS,
HTTP2_HEADER_TE,
HTTP2_HEADER_TRANSFER_ENCODING,
HTTP2_HEADER_HOST,
HTTP2_HEADER_KEEP_ALIVE,
HTTP2_HEADER_PROXY_CONNECTION,

Expand Down Expand Up @@ -131,6 +131,7 @@ const kSingleValueHeaders = new Set([
HTTP2_HEADER_ETAG,
HTTP2_HEADER_EXPIRES,
HTTP2_HEADER_FROM,
HTTP2_HEADER_HOST,
HTTP2_HEADER_IF_MATCH,
HTTP2_HEADER_IF_MODIFIED_SINCE,
HTTP2_HEADER_IF_NONE_MATCH,
Expand Down Expand Up @@ -418,7 +419,6 @@ function isIllegalConnectionSpecificHeader(name, value) {
switch (name) {
case HTTP2_HEADER_CONNECTION:
case HTTP2_HEADER_UPGRADE:
case HTTP2_HEADER_HOST:
case HTTP2_HEADER_HTTP2_SETTINGS:
case HTTP2_HEADER_KEEP_ALIVE:
case HTTP2_HEADER_PROXY_CONNECTION:
Expand Down Expand Up @@ -614,12 +614,24 @@ function sessionName(type) {
}
}

function getAuthority(headers) {
// For non-CONNECT requests, HTTP/2 allows either :authority
// or Host to be used equivalently. The first is preferred
// when making HTTP/2 requests, and the latter is preferred
// when converting from an HTTP/1 message.
if (headers[HTTP2_HEADER_AUTHORITY] !== undefined)
return headers[HTTP2_HEADER_AUTHORITY];
if (headers[HTTP2_HEADER_HOST] !== undefined)
return headers[HTTP2_HEADER_HOST];
}

module.exports = {
assertIsObject,
assertValidPseudoHeader,
assertValidPseudoHeaderResponse,
assertValidPseudoHeaderTrailer,
assertWithinRange,
getAuthority,
getDefaultSettings,
getSessionState,
getSettings,
Expand Down
63 changes: 63 additions & 0 deletions test/parallel/test-http2-compat-serverrequest-host.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const h2 = require('http2');

// Requests using host instead of :authority should be allowed
// and Http2ServerRequest.authority should fall back to hos

// :authority should NOT be auto-filled if host is present

const server = h2.createServer();
server.listen(0, common.mustCall(function() {
const port = server.address().port;
server.once('request', common.mustCall(function(request, response) {
const expected = {
':path': '/foobar',
':method': 'GET',
':scheme': 'http',
'host': `localhost:${port}`
};

assert.strictEqual(request.authority, expected.host);

const headers = request.headers;
for (const [name, value] of Object.entries(expected)) {
assert.strictEqual(headers[name], value);
}

const rawHeaders = request.rawHeaders;
for (const [name, value] of Object.entries(expected)) {
const position = rawHeaders.indexOf(name);
assert.notStrictEqual(position, -1);
assert.strictEqual(rawHeaders[position + 1], value);
}

assert(!Object.hasOwnProperty.call(headers, ':authority'));
assert(!Object.hasOwnProperty.call(rawHeaders, ':authority'));

response.on('finish', common.mustCall(function() {
server.close();
}));
response.end();
}));

const url = `http://localhost:${port}`;
const client = h2.connect(url, common.mustCall(function() {
const headers = {
':path': '/foobar',
':method': 'GET',
':scheme': 'http',
'host': `localhost:${port}`
};
const request = client.request(headers);
request.on('end', common.mustCall(function() {
client.close();
}));
request.end();
request.resume();
}));
}));
21 changes: 18 additions & 3 deletions test/parallel/test-http2-util-headers-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const { mapToHeaders, toHeaderObject } = require('internal/http2/util');
const {
getAuthority,
mapToHeaders,
toHeaderObject
} = require('internal/http2/util');
const { sensitiveHeaders } = require('http2');
const { internalBinding } = require('internal/test/binding');
const {
Expand All @@ -34,6 +38,7 @@ const {
HTTP2_HEADER_ETAG,
HTTP2_HEADER_EXPIRES,
HTTP2_HEADER_FROM,
HTTP2_HEADER_HOST,
HTTP2_HEADER_IF_MATCH,
HTTP2_HEADER_IF_MODIFIED_SINCE,
HTTP2_HEADER_IF_NONE_MATCH,
Expand Down Expand Up @@ -86,7 +91,6 @@ const {
HTTP2_HEADER_HTTP2_SETTINGS,
HTTP2_HEADER_TE,
HTTP2_HEADER_TRANSFER_ENCODING,
HTTP2_HEADER_HOST,
HTTP2_HEADER_KEEP_ALIVE,
HTTP2_HEADER_PROXY_CONNECTION
} = internalBinding('http2').constants;
Expand Down Expand Up @@ -225,6 +229,7 @@ const {
HTTP2_HEADER_ETAG,
HTTP2_HEADER_EXPIRES,
HTTP2_HEADER_FROM,
HTTP2_HEADER_HOST,
HTTP2_HEADER_IF_MATCH,
HTTP2_HEADER_IF_MODIFIED_SINCE,
HTTP2_HEADER_IF_NONE_MATCH,
Expand Down Expand Up @@ -289,7 +294,6 @@ const {
HTTP2_HEADER_HTTP2_SETTINGS,
HTTP2_HEADER_TE,
HTTP2_HEADER_TRANSFER_ENCODING,
HTTP2_HEADER_HOST,
HTTP2_HEADER_PROXY_CONNECTION,
HTTP2_HEADER_KEEP_ALIVE,
'Connection',
Expand Down Expand Up @@ -327,6 +331,17 @@ assert.throws(
mapToHeaders({ te: 'trailers' });
mapToHeaders({ te: ['trailers'] });

// HTTP/2 encourages use of Host instead of :authority when converting
// from HTTP/1 to HTTP/2, so we no longer disallow it.
// Refs: https://github.com/nodejs/node/issues/29858
mapToHeaders({ [HTTP2_HEADER_HOST]: 'abc' });

// If both are present, the latter has priority
assert.strictEqual(getAuthority({
[HTTP2_HEADER_AUTHORITY]: 'abc',
[HTTP2_HEADER_HOST]: 'def'
}), 'abc');


{
const rawHeaders = [
Expand Down

0 comments on commit 014c725

Please sign in to comment.