Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: allow Host in HTTP/2 requests #34664

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 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: https://github.com/nodejs/node/pull/34664
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,10 @@ 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. Otherwise, it is derived from
`req.headers['host']`.

#### `request.complete`
<!-- YAML
Expand Down Expand Up @@ -3708,6 +3714,18 @@ 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. Prefer `:authority` when constructing an HTTP/2
request directly, and `host` when converting from HTTP/1 (in proxies,
for instance).

The compatibility API falls back to `host` if `:authority` is not
present. See [`request.authority`][] for more information. 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 +3766,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 host

// :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