From 685d968774e18d26096a58c01954429ee79901c7 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sun, 21 Oct 2018 21:48:05 +0300 Subject: [PATCH 01/21] lib: http2 compatibility error with connection header Ignoring the connection header and disable the `ERR_HTTP2_INVALID_CONNECTION_HEADERS` error. Added a warning log on the compatibility. Fixes: https://github.com/nodejs/node/issues/23748 --- lib/internal/http2/util.js | 58 +++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index f62d936025229f..44ad06b0636959 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -246,43 +246,43 @@ function getDefaultSettings() { const flags = settingsBuffer[IDX_SETTINGS_FLAGS]; if ((flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) === - (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { + (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { holder.headerTableSize = settingsBuffer[IDX_SETTINGS_HEADER_TABLE_SIZE]; } if ((flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) === - (1 << IDX_SETTINGS_ENABLE_PUSH)) { + (1 << IDX_SETTINGS_ENABLE_PUSH)) { holder.enablePush = settingsBuffer[IDX_SETTINGS_ENABLE_PUSH] === 1; } if ((flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) === - (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { + (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { holder.initialWindowSize = settingsBuffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE]; } if ((flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) === - (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { + (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { holder.maxFrameSize = settingsBuffer[IDX_SETTINGS_MAX_FRAME_SIZE]; } if ((flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) === - (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { + (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { holder.maxConcurrentStreams = settingsBuffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]; } if ((flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) === - (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { + (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { holder.maxHeaderListSize = settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE]; } if ((flags & (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) === - (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) { + (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) { holder.enableConnectProtocol = settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL]; } @@ -387,7 +387,6 @@ function getStreamState(stream) { function isIllegalConnectionSpecificHeader(name, value) { switch (name) { - case HTTP2_HEADER_CONNECTION: case HTTP2_HEADER_UPGRADE: case HTTP2_HEADER_HOST: case HTTP2_HEADER_HTTP2_SETTINGS: @@ -425,7 +424,7 @@ function assertValidPseudoHeaderTrailer(key) { } function mapToHeaders(map, - assertValuePseudoHeader = assertValidPseudoHeader) { + assertValuePseudoHeader = assertValidPseudoHeader) { let ret = ''; let count = 0; const keys = Object.keys(map); @@ -470,15 +469,22 @@ function mapToHeaders(map, throw err; ret = `${key}\0${value}\0${ret}`; count++; - continue; - } - if (isIllegalConnectionSpecificHeader(key, value)) { - throw new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key); - } - if (isArray) { - for (var k = 0; k < value.length; k++) { - const val = String(value[k]); - ret += `${key}\0${val}\0`; + } else { + if (isIllegalConnectionSpecificHeader(key, value)) { + return new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key); + } + if (isArray) { + for (var k = 0; k < value.length; k++) { + const val = String(value[k]); + ret += `${key}\0${val}\0`; + } + count += value.length; + } + else if (key === HTTP2_HEADER_CONNECTION) { + headerMessageWarn(key); + } else { + ret += `${key}\0${value}\0`; + count++; } count += value.length; continue; @@ -501,9 +507,9 @@ class NghttpError extends Error { function assertIsObject(value, name, types = 'Object') { if (value !== undefined && - (value === null || - typeof value !== 'object' || - Array.isArray(value))) { + (value === null || + typeof value !== 'object' || + Array.isArray(value))) { const err = new ERR_INVALID_ARG_TYPE(name, types, value); Error.captureStackTrace(err, assertIsObject); throw err; @@ -512,7 +518,7 @@ function assertIsObject(value, name, types = 'Object') { function assertWithinRange(name, value, min = 0, max = Infinity) { if (value !== undefined && - (typeof value !== 'number' || value < min || value > max)) { + (typeof value !== 'number' || value < min || value > max)) { const err = new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError(name, value); err.min = min; err.max = max; @@ -581,6 +587,14 @@ function sessionName(type) { } } +function headerMessageWarn(key) { + process.emitWarning( + `The provided header "${key}" is not valid, ` + + 'and is supported only for compatibility.', + 'UnsupportedWarning' + ); +} + module.exports = { assertIsObject, assertValidPseudoHeaderResponse, From 706d7b09e5f7a2194c0dc5267f7a38c4f703cea4 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 26 Oct 2018 21:30:04 +0300 Subject: [PATCH 02/21] Update lib/internal/http2/util.js linter fix --- lib/internal/http2/util.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 44ad06b0636959..99bfd3ee01d6fc 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -479,8 +479,7 @@ function mapToHeaders(map, ret += `${key}\0${val}\0`; } count += value.length; - } - else if (key === HTTP2_HEADER_CONNECTION) { + } else if (key === HTTP2_HEADER_CONNECTION) { headerMessageWarn(key); } else { ret += `${key}\0${value}\0`; From 56a1ef681d60e6622e370a9739442251cafbd156 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 26 Oct 2018 21:34:19 +0300 Subject: [PATCH 03/21] spaces remove --- lib/internal/http2/util.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 99bfd3ee01d6fc..40d3f148d2928f 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -246,43 +246,43 @@ function getDefaultSettings() { const flags = settingsBuffer[IDX_SETTINGS_FLAGS]; if ((flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) === - (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { + (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { holder.headerTableSize = settingsBuffer[IDX_SETTINGS_HEADER_TABLE_SIZE]; } if ((flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) === - (1 << IDX_SETTINGS_ENABLE_PUSH)) { + (1 << IDX_SETTINGS_ENABLE_PUSH)) { holder.enablePush = settingsBuffer[IDX_SETTINGS_ENABLE_PUSH] === 1; } if ((flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) === - (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { + (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { holder.initialWindowSize = settingsBuffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE]; } if ((flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) === - (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { + (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { holder.maxFrameSize = settingsBuffer[IDX_SETTINGS_MAX_FRAME_SIZE]; } if ((flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) === - (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { + (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { holder.maxConcurrentStreams = settingsBuffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]; } if ((flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) === - (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { + (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { holder.maxHeaderListSize = settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE]; } if ((flags & (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) === - (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) { + (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) { holder.enableConnectProtocol = settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL]; } From 8492823730be65a6f602f782087bdd72e2a0175e Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 26 Oct 2018 21:35:24 +0300 Subject: [PATCH 04/21] spaces remove --- lib/internal/http2/util.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 40d3f148d2928f..3e0bd6e05adfa0 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -246,43 +246,43 @@ function getDefaultSettings() { const flags = settingsBuffer[IDX_SETTINGS_FLAGS]; if ((flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) === - (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { + (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { holder.headerTableSize = settingsBuffer[IDX_SETTINGS_HEADER_TABLE_SIZE]; } if ((flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) === - (1 << IDX_SETTINGS_ENABLE_PUSH)) { + (1 << IDX_SETTINGS_ENABLE_PUSH)) { holder.enablePush = settingsBuffer[IDX_SETTINGS_ENABLE_PUSH] === 1; } if ((flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) === - (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { + (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { holder.initialWindowSize = settingsBuffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE]; } if ((flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) === - (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { + (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { holder.maxFrameSize = settingsBuffer[IDX_SETTINGS_MAX_FRAME_SIZE]; } if ((flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) === - (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { + (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { holder.maxConcurrentStreams = settingsBuffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]; } if ((flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) === - (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { + (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { holder.maxHeaderListSize = settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE]; } if ((flags & (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) === - (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) { + (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) { holder.enableConnectProtocol = settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL]; } From 5287044f22e9585b7ceafa45a76a290669679969 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 26 Oct 2018 21:38:07 +0300 Subject: [PATCH 05/21] spaces --- lib/internal/http2/util.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 3e0bd6e05adfa0..402277b58ad292 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -506,9 +506,9 @@ class NghttpError extends Error { function assertIsObject(value, name, types = 'Object') { if (value !== undefined && - (value === null || - typeof value !== 'object' || - Array.isArray(value))) { + (value === null || + typeof value !== 'object' || + Array.isArray(value))) { const err = new ERR_INVALID_ARG_TYPE(name, types, value); Error.captureStackTrace(err, assertIsObject); throw err; @@ -517,7 +517,7 @@ function assertIsObject(value, name, types = 'Object') { function assertWithinRange(name, value, min = 0, max = Infinity) { if (value !== undefined && - (typeof value !== 'number' || value < min || value > max)) { + (typeof value !== 'number' || value < min || value > max)) { const err = new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError(name, value); err.min = min; err.max = max; From 8c83d2c2ffb39ec9787ff1ffd5e18fff333efbb4 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 26 Oct 2018 22:29:37 +0300 Subject: [PATCH 06/21] fix linter --- lib/internal/http2/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 402277b58ad292..88db24b2c6d7c0 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -424,7 +424,7 @@ function assertValidPseudoHeaderTrailer(key) { } function mapToHeaders(map, - assertValuePseudoHeader = assertValidPseudoHeader) { + assertValuePseudoHeader = assertValidPseudoHeader) { let ret = ''; let count = 0; const keys = Object.keys(map); From 5c38e7d4c6eb8aa130b06d344265c73f109baa23 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sat, 27 Oct 2018 11:41:47 +0300 Subject: [PATCH 07/21] move logic into compat.js --- lib/internal/http2/compat.js | 16 ++++++++++++++++ lib/internal/http2/util.js | 11 +---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 1783a48ada814c..2e204291a63fa0 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -47,6 +47,7 @@ const { } = constants; let statusMessageWarned = false; +const statusHeaderMessageWarned = {}; // Defines and implements an API compatibility layer on top of the core // HTTP/2 implementation, intended to provide an interface that is as @@ -60,6 +61,9 @@ function assertValidHeader(name, value) { err = new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED(); } else if (value === undefined || value === null) { err = new ERR_HTTP2_INVALID_HEADER_VALUE(value, name); + } else if (name === constants.HTTP2_HEADER_CONNECTION) { + headerMessageWarn(name); + value = ''; } if (err !== undefined) { Error.captureStackTrace(err, assertValidHeader); @@ -90,6 +94,18 @@ function statusMessageWarn() { } } +function headerMessageWarn(key) { + if (!statusHeaderMessageWarned[key] || + statusHeaderMessageWarned[key] === false) { + process.emitWarning( + `The provided header "${key}" is not valid, ` + + 'and is supported only for compatibility.', + 'UnsupportedWarning' + ); + statusHeaderMessageWarned[key] = true; + } +} + function onStreamData(chunk) { const request = this[kRequest]; if (request !== undefined && !request.push(chunk)) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 88db24b2c6d7c0..d31dc024a3d183 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -387,6 +387,7 @@ function getStreamState(stream) { function isIllegalConnectionSpecificHeader(name, value) { switch (name) { + case HTTP2_HEADER_CONNECTION: case HTTP2_HEADER_UPGRADE: case HTTP2_HEADER_HOST: case HTTP2_HEADER_HTTP2_SETTINGS: @@ -479,8 +480,6 @@ function mapToHeaders(map, ret += `${key}\0${val}\0`; } count += value.length; - } else if (key === HTTP2_HEADER_CONNECTION) { - headerMessageWarn(key); } else { ret += `${key}\0${value}\0`; count++; @@ -586,14 +585,6 @@ function sessionName(type) { } } -function headerMessageWarn(key) { - process.emitWarning( - `The provided header "${key}" is not valid, ` + - 'and is supported only for compatibility.', - 'UnsupportedWarning' - ); -} - module.exports = { assertIsObject, assertValidPseudoHeaderResponse, From 3ef516a8999758a3470c1e9e91b3aa7b3cdc8274 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sat, 27 Oct 2018 19:55:09 +0300 Subject: [PATCH 08/21] change status variable ignore connection header --- lib/internal/http2/compat.js | 20 +++++++++++-------- ...http2-compat-short-stream-client-server.js | 1 + 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 2e204291a63fa0..8b4195d1f8475c 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -47,7 +47,7 @@ const { } = constants; let statusMessageWarned = false; -const statusHeaderMessageWarned = {}; +let statusConnectionHeaderWarned = false; // Defines and implements an API compatibility layer on top of the core // HTTP/2 implementation, intended to provide an interface that is as @@ -62,8 +62,7 @@ function assertValidHeader(name, value) { } else if (value === undefined || value === null) { err = new ERR_HTTP2_INVALID_HEADER_VALUE(value, name); } else if (name === constants.HTTP2_HEADER_CONNECTION) { - headerMessageWarn(name); - value = ''; + connectionHeaderMessageWarn(); } if (err !== undefined) { Error.captureStackTrace(err, assertValidHeader); @@ -94,15 +93,14 @@ function statusMessageWarn() { } } -function headerMessageWarn(key) { - if (!statusHeaderMessageWarned[key] || - statusHeaderMessageWarned[key] === false) { +function connectionHeaderMessageWarn() { + if (statusConnectionHeaderWarned === false) { process.emitWarning( - `The provided header "${key}" is not valid, ` + + `The provided "connection" header is not valid, ` + 'and is supported only for compatibility.', 'UnsupportedWarning' ); - statusHeaderMessageWarned[key] = true; + statusConnectionHeaderWarned = true; } } @@ -557,6 +555,12 @@ class Http2ServerResponse extends Stream { [kSetHeader](name, value) { name = name.trim().toLowerCase(); assertValidHeader(name, value); + + // In case of a `connection` header ignore it + if (name === constants.HTTP2_HEADER_CONNECTION) { + return; + } + this[kHeaders][name] = value; } diff --git a/test/parallel/test-http2-compat-short-stream-client-server.js b/test/parallel/test-http2-compat-short-stream-client-server.js index f7ef9412106f59..3741bfabae9669 100644 --- a/test/parallel/test-http2-compat-short-stream-client-server.js +++ b/test/parallel/test-http2-compat-short-stream-client-server.js @@ -9,6 +9,7 @@ const { Readable } = require('stream'); const server = http2.createServer(common.mustCall((req, res) => { res.setHeader('content-type', 'text/html'); + res.setHeader('connection', 'sagiii'); const input = new Readable({ read() { this.push('test'); From 4a3a9b7de8de43582c2c32c8e68b2e47f5c1158e Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sat, 27 Oct 2018 19:58:37 +0300 Subject: [PATCH 09/21] delete internal tests --- test/parallel/test-http2-compat-short-stream-client-server.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-http2-compat-short-stream-client-server.js b/test/parallel/test-http2-compat-short-stream-client-server.js index 3741bfabae9669..f7ef9412106f59 100644 --- a/test/parallel/test-http2-compat-short-stream-client-server.js +++ b/test/parallel/test-http2-compat-short-stream-client-server.js @@ -9,7 +9,6 @@ const { Readable } = require('stream'); const server = http2.createServer(common.mustCall((req, res) => { res.setHeader('content-type', 'text/html'); - res.setHeader('connection', 'sagiii'); const input = new Readable({ read() { this.push('test'); From f158370d311881f19e8d1c15898cfc64718e454e Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sat, 27 Oct 2018 20:20:43 +0300 Subject: [PATCH 10/21] singlequote --- lib/internal/http2/compat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 8b4195d1f8475c..61fcf70c4d7355 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -96,7 +96,7 @@ function statusMessageWarn() { function connectionHeaderMessageWarn() { if (statusConnectionHeaderWarned === false) { process.emitWarning( - `The provided "connection" header is not valid, ` + + `The provided connection header is not valid, ` + 'and is supported only for compatibility.', 'UnsupportedWarning' ); From 3468c747491f30479c044c0e6e8743943519ee3f Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sat, 27 Oct 2018 20:36:33 +0300 Subject: [PATCH 11/21] labeling --- lib/internal/http2/compat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 61fcf70c4d7355..d7a6e6b971da18 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -97,7 +97,7 @@ function connectionHeaderMessageWarn() { if (statusConnectionHeaderWarned === false) { process.emitWarning( `The provided connection header is not valid, ` + - 'and is supported only for compatibility.', + 'it is supported only for compatibility.', 'UnsupportedWarning' ); statusConnectionHeaderWarned = true; From 9173cb178fb332431647d2211285b6f0c7eaceb2 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sat, 27 Oct 2018 20:55:11 +0300 Subject: [PATCH 12/21] change warning message --- lib/internal/http2/compat.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index d7a6e6b971da18..4b472ee98f4809 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -96,8 +96,9 @@ function statusMessageWarn() { function connectionHeaderMessageWarn() { if (statusConnectionHeaderWarned === false) { process.emitWarning( - `The provided connection header is not valid, ` + - 'it is supported only for compatibility.', + 'The provided connection header is not valid, ' + + 'the value will be dropped from the header and ' + + 'will never be in use.', 'UnsupportedWarning' ); statusConnectionHeaderWarned = true; From 9bedc323485c90b213f2027b21925ab3de868e1d Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Wed, 31 Oct 2018 20:43:48 +0200 Subject: [PATCH 13/21] change structure check if trailer --- lib/internal/http2/compat.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 4b472ee98f4809..f8dacb4c9dc301 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -61,7 +61,7 @@ function assertValidHeader(name, value) { err = new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED(); } else if (value === undefined || value === null) { err = new ERR_HTTP2_INVALID_HEADER_VALUE(value, name); - } else if (name === constants.HTTP2_HEADER_CONNECTION) { + } else if (!isConnectionHeaderAllowed(name, value)) { connectionHeaderMessageWarn(); } if (err !== undefined) { @@ -93,6 +93,10 @@ function statusMessageWarn() { } } +function isConnectionHeaderAllowed(name, value) { + return (name === constants.HTTP2_HEADER_CONNECTION && value === 'trailers'); +} + function connectionHeaderMessageWarn() { if (statusConnectionHeaderWarned === false) { process.emitWarning( @@ -557,8 +561,7 @@ class Http2ServerResponse extends Stream { name = name.trim().toLowerCase(); assertValidHeader(name, value); - // In case of a `connection` header ignore it - if (name === constants.HTTP2_HEADER_CONNECTION) { + if (!isConnectionHeaderAllowed(name, value)) { return; } From 0ab7481a2dd5a64c3cbb7c6ca03e3f40162e38d6 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 2 Nov 2018 20:54:39 +0200 Subject: [PATCH 14/21] change term --- lib/internal/http2/compat.js | 5 ++++- test/parallel/test-http2-compat-serverresponse-end.js | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index f8dacb4c9dc301..4056a66e539bfb 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -94,7 +94,10 @@ function statusMessageWarn() { } function isConnectionHeaderAllowed(name, value) { - return (name === constants.HTTP2_HEADER_CONNECTION && value === 'trailers'); + if (name !== constants.HTTP2_HEADER_CONNECTION) + return true; + else + return value !== 'trailers'; } function connectionHeaderMessageWarn() { diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index 0e846a5948e3cc..ca5a1a514933cc 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -1,5 +1,5 @@ 'use strict'; - +console.log("1111") const { mustCall, mustNotCall, From bdf62cefdaf81350aadb70d4fe934b1e8e4d4503 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 2 Nov 2018 20:59:04 +0200 Subject: [PATCH 15/21] remove comment --- test/parallel/test-http2-compat-serverresponse-end.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index ca5a1a514933cc..0e846a5948e3cc 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -1,5 +1,5 @@ 'use strict'; -console.log("1111") + const { mustCall, mustNotCall, From bc1ba7ca51112c5ebdf73798fa476bfe07e02cc7 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 2 Nov 2018 21:02:50 +0200 Subject: [PATCH 16/21] trailers fix --- lib/internal/http2/compat.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 4056a66e539bfb..144c9fd92f9e84 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -94,9 +94,9 @@ function statusMessageWarn() { } function isConnectionHeaderAllowed(name, value) { - if (name !== constants.HTTP2_HEADER_CONNECTION) + if (name !== constants.HTTP2_HEADER_CONNECTION) return true; - else + else return value !== 'trailers'; } From de389ebc5a76a8914e3c95c3841de2ded7506c77 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Fri, 2 Nov 2018 21:56:55 +0200 Subject: [PATCH 17/21] only trailers are supported --- lib/internal/http2/compat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 144c9fd92f9e84..2f1f56cd747bae 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -97,7 +97,7 @@ function isConnectionHeaderAllowed(name, value) { if (name !== constants.HTTP2_HEADER_CONNECTION) return true; else - return value !== 'trailers'; + return value === 'trailers'; } function connectionHeaderMessageWarn() { From d7fd930cdb1045be9112677ac513ed7b5834106f Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Mon, 3 Dec 2018 20:39:55 +0200 Subject: [PATCH 18/21] c --- lib/internal/http2/util.js | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index d31dc024a3d183..ff8102d9d7aa89 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -470,19 +470,15 @@ function mapToHeaders(map, throw err; ret = `${key}\0${value}\0${ret}`; count++; - } else { - if (isIllegalConnectionSpecificHeader(key, value)) { - return new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key); - } - if (isArray) { - for (var k = 0; k < value.length; k++) { - const val = String(value[k]); - ret += `${key}\0${val}\0`; - } - count += value.length; - } else { - ret += `${key}\0${value}\0`; - count++; + continue; + } + if (isIllegalConnectionSpecificHeader(key, value)) { + throw new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key); + } + if (isArray) { + for (var k = 0; k < value.length; k++) { + const val = String(value[k]); + ret += `${key}\0${val}\0`; } count += value.length; continue; @@ -602,4 +598,4 @@ module.exports = { toHeaderObject, updateOptionsBuffer, updateSettingsBuffer -}; +}; \ No newline at end of file From 7066aa7583c5adc5936eb662c4c9b5854a9b1f24 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Mon, 3 Dec 2018 21:06:26 +0200 Subject: [PATCH 19/21] new line --- lib/internal/http2/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index ff8102d9d7aa89..f62d936025229f 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -598,4 +598,4 @@ module.exports = { toHeaderObject, updateOptionsBuffer, updateSettingsBuffer -}; \ No newline at end of file +}; From 058a0295a163f6480807e4b66b0e0efa8d4fa815 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sat, 15 Dec 2018 12:23:06 +0200 Subject: [PATCH 20/21] added test for the connection header --- test/parallel/test-http2-server-set-header.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-http2-server-set-header.js b/test/parallel/test-http2-server-set-header.js index 83f373ec21b314..1dfaa04584f8d9 100644 --- a/test/parallel/test-http2-server-set-header.js +++ b/test/parallel/test-http2-server-set-header.js @@ -11,6 +11,7 @@ const body = const server = http2.createServer((req, res) => { res.setHeader('foobar', 'baz'); res.setHeader('X-POWERED-BY', 'node-test'); + res.setHeader('connection', 'connection-test'); res.end(body); }); From 0a3cedc29dc76a2ed5b9f04893234423869e5bc6 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sat, 15 Dec 2018 18:28:07 +0200 Subject: [PATCH 21/21] verify warning is emitted --- test/parallel/test-http2-server-set-header.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/parallel/test-http2-server-set-header.js b/test/parallel/test-http2-server-set-header.js index 1dfaa04584f8d9..13ca1142fa3892 100644 --- a/test/parallel/test-http2-server-set-header.js +++ b/test/parallel/test-http2-server-set-header.js @@ -35,4 +35,10 @@ server.listen(0, common.mustCall(() => { req.end(); })); +const compatMsg = 'The provided connection header is not valid, ' + + 'the value will be dropped from the header and ' + + 'will never be in use.'; + +common.expectWarning('UnsupportedWarning', compatMsg); + server.on('error', common.mustNotCall());