Skip to content
This repository has been archived by the owner on Jul 6, 2018. It is now read-only.

Commit

Permalink
http2: improve header single value checking
Browse files Browse the repository at this point in the history 
PR-URL: #129
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
@jasnell
jasnell committed May 31, 2017
1 parent 79417fe commit e91a1a3
Show file tree
Hide file tree
Showing 8 changed files with 345 additions and 76 deletions.
6 changes: 4 additions & 2 deletions lib/internal/errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,17 @@ E('ERR_CONSOLE_WRITABLE_STREAM',
(name) => `Console expects a writable stream instance for ${name}`);
E('ERR_HTTP2_HEADER_REQUIRED',
(name) => `The ${name} header is required`);
E('ERR_HTTP2_HEADER_SINGLE_VALUE',
(name) => `Header field "${name}" must have only a single value`);
E('ERR_HTTP2_HEADERS_SENT', 'Response has already been initiated.');
E('ERR_HTTP2_HEADERS_AFTER_RESPOND',
'Cannot specify additional headers after response initiated');
E('ERR_HTTP2_INVALID_CONNECTION_HEADERS',
'HTTP/1 Connection specific headers are forbidden');
E('ERR_HTTP2_INVALID_INFO_STATUS',
'Invalid informational status code');
E('ERR_HTTP2_PSEUDO_HEADERS_SINGLE_VALUE',
'HTTP/2 pseudo-headers must have a single value');
E('ERR_HTTP2_INVALID_PSEUDOHEADER',
(name) => `"${name}" is not a valid HTTP/2 pseudoheader`);
E('ERR_HTTP2_SOCKET_BOUND',
'The socket is already bound to an Http2Session');
E('ERR_HTTP2_STATUS_INVALID',
Expand Down
3 changes: 0 additions & 3 deletions lib/internal/http2/compat.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const Stream = require('stream');
const Readable = Stream.Readable;
const binding = process.binding('http2');
const { isIllegalConnectionSpecificHeader } = require('internal/http2/util');
const constants = binding.constants;

const kFinish = Symbol('finish');
Expand All @@ -24,8 +23,6 @@ let statusMessageWarned = false;
function assertValidHeader(name, value) {
if (isPseudoHeader(name))
throw new Error('Cannot set HTTP/2 pseudo-headers');
if (isIllegalConnectionSpecificHeader(name, value))
throw new Error('Connection-specific HTTP/1 headers are not permitted');
if (value === undefined || value === null)
throw new TypeError('Value must not be undefined or null');
}
Expand Down
139 changes: 111 additions & 28 deletions lib/internal/http2/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,89 @@
const binding = process.binding('http2');
const errors = require('internal/errors');

const {
HTTP2_HEADER_STATUS,
HTTP2_HEADER_METHOD,
HTTP2_HEADER_AUTHORITY,
HTTP2_HEADER_SCHEME,
HTTP2_HEADER_PATH,
HTTP2_HEADER_AGE,
HTTP2_HEADER_AUTHORIZATION,
HTTP2_HEADER_CONTENT_ENCODING,
HTTP2_HEADER_CONTENT_LANGUAGE,
HTTP2_HEADER_CONTENT_LENGTH,
HTTP2_HEADER_CONTENT_LOCATION,
HTTP2_HEADER_CONTENT_MD5,
HTTP2_HEADER_CONTENT_RANGE,
HTTP2_HEADER_CONTENT_TYPE,
HTTP2_HEADER_DATE,
HTTP2_HEADER_ETAG,
HTTP2_HEADER_EXPIRES,
HTTP2_HEADER_FROM,
HTTP2_HEADER_IF_MATCH,
HTTP2_HEADER_IF_NONE_MATCH,
HTTP2_HEADER_IF_MODIFIED_SINCE,
HTTP2_HEADER_IF_RANGE,
HTTP2_HEADER_IF_UNMODIFIED_SINCE,
HTTP2_HEADER_LAST_MODIFIED,
HTTP2_HEADER_MAX_FORWARDS,
HTTP2_HEADER_PROXY_AUTHORIZATION,
HTTP2_HEADER_RANGE,
HTTP2_HEADER_REFERER,
HTTP2_HEADER_RETRY_AFTER,
HTTP2_HEADER_USER_AGENT,

HTTP2_HEADER_CONNECTION,
HTTP2_HEADER_UPGRADE,
HTTP2_HEADER_HTTP2_SETTINGS,
HTTP2_HEADER_TE,
HTTP2_HEADER_TRANSFER_ENCODING,
HTTP2_HEADER_HOST,
HTTP2_HEADER_KEEP_ALIVE,
HTTP2_HEADER_PROXY_CONNECTION
} = binding.constants;

const kValidPseudoHeaders = new Set([
HTTP2_HEADER_STATUS,
HTTP2_HEADER_METHOD,
HTTP2_HEADER_AUTHORITY,
HTTP2_HEADER_SCHEME,
HTTP2_HEADER_PATH
]);

const kSingleValueHeaders = new Set([
HTTP2_HEADER_STATUS,
HTTP2_HEADER_METHOD,
HTTP2_HEADER_AUTHORITY,
HTTP2_HEADER_SCHEME,
HTTP2_HEADER_PATH,
HTTP2_HEADER_AGE,
HTTP2_HEADER_AUTHORIZATION,
HTTP2_HEADER_CONTENT_ENCODING,
HTTP2_HEADER_CONTENT_LANGUAGE,
HTTP2_HEADER_CONTENT_LENGTH,
HTTP2_HEADER_CONTENT_LOCATION,
HTTP2_HEADER_CONTENT_MD5,
HTTP2_HEADER_CONTENT_RANGE,
HTTP2_HEADER_CONTENT_TYPE,
HTTP2_HEADER_DATE,
HTTP2_HEADER_ETAG,
HTTP2_HEADER_EXPIRES,
HTTP2_HEADER_FROM,
HTTP2_HEADER_IF_MATCH,
HTTP2_HEADER_IF_MODIFIED_SINCE,
HTTP2_HEADER_IF_NONE_MATCH,
HTTP2_HEADER_IF_RANGE,
HTTP2_HEADER_IF_UNMODIFIED_SINCE,
HTTP2_HEADER_LAST_MODIFIED,
HTTP2_HEADER_MAX_FORWARDS,
HTTP2_HEADER_PROXY_AUTHORIZATION,
HTTP2_HEADER_RANGE,
HTTP2_HEADER_REFERER,
HTTP2_HEADER_RETRY_AFTER,
HTTP2_HEADER_USER_AGENT
]);

// The following ArrayBuffer instances are used to share memory more efficiently
// with the native binding side for a number of methods. These are not intended
// to be used directly by users in any way. The ArrayBuffers are created on
Expand Down Expand Up @@ -126,28 +209,22 @@ function getStreamState(session, stream) {

function isIllegalConnectionSpecificHeader(name, value) {
switch (name) {
case 'connection':
case 'http2-settings':
case 'keep-alive':
case 'proxy-connection':
case 'transfer-encoding':
case 'upgrade':
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:
case HTTP2_HEADER_TRANSFER_ENCODING:
return true;
case 'te':
return value !== 'trailers';
case HTTP2_HEADER_TE:
const val = Array.isArray(value) ? value.join(', ') : value;
return val !== 'trailers';
default:
return false;
}
}

function assertIllegalConnectionSpecificHeader(name, value, ctor) {
if (isIllegalConnectionSpecificHeader(name, value)) {
var err = new errors.Error('ERR_HTTP2_INVALID_CONNECTION_HEADERS');
Error.captureStackTrace(err, ctor);
throw err;
}
}

function mapToHeaders(map) {
var ret = [];
var keys = Object.keys(map);
Expand All @@ -158,26 +235,33 @@ function mapToHeaders(map) {
if (typeof key === 'symbol' || value === undefined || !key)
continue;
key = String(key).toLowerCase();
var isArray = Array.isArray(value);
if (key[0] === ':') {
if (isArray) {
if (value.length > 1)
throw new errors.Error('ERR_HTTP2_PSEUDO_HEADERS_SINGLE_VALUE');
value = value[0];
const isArray = Array.isArray(value);
if (isArray) {
switch (value.length) {
case 0:
continue;
case 1:
value = String(value[0]);
break;
default:
if (kSingleValueHeaders.has(key))
throw new errors.Error('ERR_HTTP2_HEADER_SINGLE_VALUE', key);
}
val = String(value);
assertIllegalConnectionSpecificHeader(key, val, mapToHeaders);
ret.unshift([key, val]);
}
if (key[0] === ':') {
if (!kValidPseudoHeaders.has(key))
throw new errors.Error('ERR_HTTP2_INVALID_PSEUDOHEADER', key);
ret.unshift([key, String(value)]);
} else {
if (isIllegalConnectionSpecificHeader(key, value))
throw new errors.Error('ERR_HTTP2_INVALID_CONNECTION_HEADERS');
if (isArray) {
for (var k = 0; k < value.length; k++) {
val = String(value[k]);
assertIllegalConnectionSpecificHeader(key, val, mapToHeaders);
ret.push([key, val]);
}
} else {
val = String(value);
assertIllegalConnectionSpecificHeader(key, val, mapToHeaders);
ret.push([key, val]);
}
}
Expand Down Expand Up @@ -214,7 +298,6 @@ function assertIsObject(value, name, types) {

module.exports = {
assertIsObject,
isIllegalConnectionSpecificHeader,
emitErrorIfNecessary,
getDefaultSettings,
getSessionState,
Expand Down
58 changes: 53 additions & 5 deletions src/node_http2.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,10 @@ static bool CheckHeaderAllowsMultiple(nghttp2_vec* name) {
break;
case 4:
switch (name->base[3]) {
case 'e':
if (memcmp(name->base, "dat", 3) == 0)
return false;
break;
case 'g':
if (memcmp(name->base, "eta", 3) == 0)
return false;
Expand All @@ -726,6 +730,10 @@ static bool CheckHeaderAllowsMultiple(nghttp2_vec* name) {
return false;
break;
}
case 5:
if (memcmp(name->base, "range", 5) == 0)
return false;
break;
case 6:
if (memcmp(name->base, "server", 6) == 0)
return false;
Expand All @@ -743,16 +751,36 @@ static bool CheckHeaderAllowsMultiple(nghttp2_vec* name) {
}
break;
case 8:
if (memcmp(name->base, "location", 8) == 0)
return false;
switch (name->base[7]) {
case 'e':
if (memcmp(name->base, "if-rang", 7) == 0)
return false;
break;
case 'h':
if (memcmp(name->base, "if-matc", 7) == 0)
return false;
break;
case 'n':
if (memcmp(name->base, "locatio", 7) == 0)
return false;
break;
}
break;
case 10:
if (memcmp(name->base, "user-agent", 10) == 0)
return false;
break;
case 11:
if (memcmp(name->base, "retry-after", 11) == 0)
return false;
switch (name->base[10]) {
case '5':
if (memcmp(name->base, "content-md", 10) == 0)
return false;
break;
case 'r':
if (memcmp(name->base, "retry-afte", 10) == 0)
return false;
break;
}
break;
case 12:
switch (name->base[11]) {
Expand All @@ -772,6 +800,10 @@ static bool CheckHeaderAllowsMultiple(nghttp2_vec* name) {
if (memcmp(name->base, "last-modifie", 12) == 0)
return false;
break;
case 'h':
if (memcmp(name->base, "if-none-matc", 12) == 0)
return false;
break;
case 'n':
if (memcmp(name->base, "authorizatio", 12) == 0)
return false;
Expand All @@ -782,6 +814,22 @@ static bool CheckHeaderAllowsMultiple(nghttp2_vec* name) {
if (memcmp(name->base, "content-length", 14) == 0)
return false;
break;
case 16:
switch (name->base[15]) {
case 'e':
if (memcmp(name->base, "content-languag", 15) == 0)
return false;
break;
case 'g':
if (memcmp(name->base, "content-encodin", 15) == 0)
return false;
break;
case 'n':
if (memcmp(name->base, "content-locatio", 15) == 0)
return false;
break;
}
break;
case 17:
if (memcmp(name->base, "if-modified-since", 17) == 0)
return false;
Expand All @@ -793,7 +841,7 @@ static bool CheckHeaderAllowsMultiple(nghttp2_vec* name) {
return false;
break;
case 'n':
if (memcmp(name->base, "proxy-authenticatio", 18) == 0)
if (memcmp(name->base, "proxy-authorizatio", 18) == 0)
return false;
break;
}
Expand Down
9 changes: 8 additions & 1 deletion src/node_http2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ using v8::MaybeLocal;
V(ALLOW, "allow") \
V(AUTHORIZATION, "authorization") \
V(CACHE_CONTROL, "cache-control") \
V(CONNECTION, "connection") \
V(CONTENT_DISPOSITION, "content-disposition") \
V(CONTENT_ENCODING, "content-encoding") \
V(CONTENT_LANGUAGE, "content-language") \
V(CONTENT_LENGTH, "content-length") \
V(CONTENT_LOCATION, "content-location") \
V(CONTENT_MD5, "content-md5") \
V(CONTENT_RANGE, "content-range") \
V(CONTENT_TYPE, "content-type") \
V(COOKIE, "cookie") \
Expand Down Expand Up @@ -65,10 +67,15 @@ using v8::MaybeLocal;
V(SET_COOKIE, "set-cookie") \
V(STRICT_TRANSPORT_SECURITY, "strict-transport-security") \
V(TRANSFER_ENCODING, "transfer-encoding") \
V(TE, "te") \
V(UPGRADE, "upgrade") \
V(USER_AGENT, "user-agent") \
V(VARY, "vary") \
V(VIA, "via") \
V(WWW_AUTHENTICATE, "www-authenticate")
V(WWW_AUTHENTICATE, "www-authenticate") \
V(HTTP2_SETTINGS, "http2-settings") \
V(KEEP_ALIVE, "keep-alive") \
V(PROXY_CONNECTION, "proxy-connection")

enum http_known_headers {
HTTP_KNOWN_HEADER_MIN,
Expand Down
9 changes: 8 additions & 1 deletion test/parallel/test-http2-binding.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ const expectedHeaderNames = {
HTTP2_HEADER_CONTENT_RANGE: 'content-range',
HTTP2_HEADER_CONTENT_TYPE: 'content-type',
HTTP2_HEADER_COOKIE: 'cookie',
HTTP2_HEADER_CONNECTION: 'connection',
HTTP2_HEADER_ETAG: 'etag',
HTTP2_HEADER_EXPECT: 'expect',
HTTP2_HEADER_EXPIRES: 'expires',
Expand All @@ -143,6 +144,7 @@ const expectedHeaderNames = {
HTTP2_HEADER_PREFER: 'prefer',
HTTP2_HEADER_PROXY_AUTHENTICATE: 'proxy-authenticate',
HTTP2_HEADER_PROXY_AUTHORIZATION: 'proxy-authorization',
HTTP2_HEADER_PROXY_CONNECTION: 'proxy-connection',
HTTP2_HEADER_RANGE: 'range',
HTTP2_HEADER_REFERER: 'referer',
HTTP2_HEADER_REFRESH: 'refresh',
Expand All @@ -154,7 +156,12 @@ const expectedHeaderNames = {
HTTP2_HEADER_USER_AGENT: 'user-agent',
HTTP2_HEADER_VARY: 'vary',
HTTP2_HEADER_VIA: 'via',
HTTP2_HEADER_WWW_AUTHENTICATE: 'www-authenticate'
HTTP2_HEADER_WWW_AUTHENTICATE: 'www-authenticate',
HTTP2_HEADER_KEEP_ALIVE: 'keep-alive',
HTTP2_HEADER_CONTENT_MD5: 'content-md5',
HTTP2_HEADER_TE: 'te',
HTTP2_HEADER_UPGRADE: 'upgrade',
HTTP2_HEADER_HTTP2_SETTINGS: 'http2-settings'
};

const expectedNGConstants = {
Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-http2-compat-serverresponse-headers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ server.listen(0, common.mustCall(function() {
assert.throws(function() {
response.setHeader(':status', 'foobar');
}, Error);
assert.throws(function() {
response.setHeader('connection', 'foobar');
}, Error);
assert.throws(function() {
response.setHeader(real, null);
}, TypeError);
Expand Down
Loading

0 comments on commit e91a1a3

Please sign in to comment.