From e90bf146633fa1671e21c7b2966488fd0fcac208 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Sat, 25 Apr 2020 03:37:43 +0530 Subject: [PATCH 1/5] http2: add `invalidheaders` test Refs: https://github.com/nodejs/node/issues/29829 --- lib/internal/http2/compat.js | 2 +- lib/internal/http2/util.js | 6 +- .../parallel/test-http2-invalidheaderfield.js | 59 +++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-http2-invalidheaderfield.js diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 841f9b69a033cd..a8b8b0bd922d5e 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -73,7 +73,7 @@ let statusConnectionHeaderWarned = false; // close as possible to the current require('http') API const assertValidHeader = hideStackFrames((name, value) => { - if (name === '' || typeof name !== 'string') { + if (name === '' || typeof name !== 'string' || name.indexOf(' ') > 0) { throw new ERR_INVALID_HTTP_TOKEN('Header name', name); } if (isPseudoHeader(name)) { diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 2ef0824cf760bc..e49bd4b3cdc201 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -18,7 +18,8 @@ const { ERR_HTTP2_INVALID_CONNECTION_HEADERS, ERR_HTTP2_INVALID_PSEUDOHEADER, ERR_HTTP2_INVALID_SETTING_VALUE, - ERR_INVALID_ARG_TYPE + ERR_INVALID_ARG_TYPE, + ERR_INVALID_HTTP_TOKEN }, addCodeToName, hideStackFrames @@ -490,6 +491,9 @@ function mapToHeaders(map, count++; continue; } + if (key.indexOf(' ') > 0) { + throw new ERR_INVALID_HTTP_TOKEN('Header name', key); + } if (isIllegalConnectionSpecificHeader(key, value)) { throw new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key); } diff --git a/test/parallel/test-http2-invalidheaderfield.js b/test/parallel/test-http2-invalidheaderfield.js new file mode 100644 index 00000000000000..5d5f6203872dca --- /dev/null +++ b/test/parallel/test-http2-invalidheaderfield.js @@ -0,0 +1,59 @@ +// spaced headers +// psuedo heaers +// capitalized headers + +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) { common.skip('missing crypto'); } +const http2 = require('http2'); +const { throws } = require('assert'); + +const server = http2.createServer(common.mustCall((req, res) => { + + throws(() => { + res.setHeader(':path', '/'); + }, TypeError); + + throws(() => { + res.setHeader('t est', 123); + }, TypeError); + + // should not fail + res.setHeader('TEST', 123); + + // should not fail + res.setHeader('test_', 123); + + res.end(); +}, 2)); + +server.listen(0, common.mustCall(() => { + const session = http2.connect(`http://localhost:${server.address().port}`); + + throws(() => { + session.request({ + 't est': 123 + }); + }, TypeError); + + throws(() => { + session.request({ + ':test': 123 + }); + }, TypeError); + + // should not fail + session.request({ + 'TEST': 123 + }); + + // should not fail + const req = session.request({ + 'test_': 123 + }); + + req.on('end', () => { + session.close(); + server.close(); + }); +})); From e492874ef4cc80ee5e3a8887167e3c49534071ec Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Sun, 7 Jun 2020 13:27:30 +0530 Subject: [PATCH 2/5] fix test structure and checks --- lib/internal/http2/compat.js | 2 +- lib/internal/http2/util.js | 2 +- test/parallel/test-http2-invalidheaderfield.js | 10 ++++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index a8b8b0bd922d5e..6a2d5097116cc2 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -73,7 +73,7 @@ let statusConnectionHeaderWarned = false; // close as possible to the current require('http') API const assertValidHeader = hideStackFrames((name, value) => { - if (name === '' || typeof name !== 'string' || name.indexOf(' ') > 0) { + if (name === '' || typeof name !== 'string' || name.indexOf(' ') >= 0) { throw new ERR_INVALID_HTTP_TOKEN('Header name', name); } if (isPseudoHeader(name)) { diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index e49bd4b3cdc201..8327a1d248f803 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -491,7 +491,7 @@ function mapToHeaders(map, count++; continue; } - if (key.indexOf(' ') > 0) { + if (key.indexOf(' ') >= 0) { throw new ERR_INVALID_HTTP_TOKEN('Header name', key); } if (isIllegalConnectionSpecificHeader(key, value)) { diff --git a/test/parallel/test-http2-invalidheaderfield.js b/test/parallel/test-http2-invalidheaderfield.js index 5d5f6203872dca..efb13e77e3a2df 100644 --- a/test/parallel/test-http2-invalidheaderfield.js +++ b/test/parallel/test-http2-invalidheaderfield.js @@ -1,10 +1,12 @@ -// spaced headers -// psuedo heaers -// capitalized headers - 'use strict'; const common = require('../common'); if (!common.hasCrypto) { common.skip('missing crypto'); } + +// Check for: +// Spaced headers +// Psuedo headers +// Capitalized headers + const http2 = require('http2'); const { throws } = require('assert'); From f3ba48d6749232b60a320e284c4750e26b9edc84 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Mon, 8 Jun 2020 13:23:46 +0530 Subject: [PATCH 3/5] fix tests --- .../parallel/test-http2-invalidheaderfield.js | 67 +++++++++---------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/test/parallel/test-http2-invalidheaderfield.js b/test/parallel/test-http2-invalidheaderfield.js index efb13e77e3a2df..ec7746591f1ca9 100644 --- a/test/parallel/test-http2-invalidheaderfield.js +++ b/test/parallel/test-http2-invalidheaderfield.js @@ -8,54 +8,47 @@ if (!common.hasCrypto) { common.skip('missing crypto'); } // Capitalized headers const http2 = require('http2'); -const { throws } = require('assert'); +const { throws, strictEqual } = require('assert'); const server = http2.createServer(common.mustCall((req, res) => { - throws(() => { res.setHeader(':path', '/'); }, TypeError); - throws(() => { res.setHeader('t est', 123); }, TypeError); - - // should not fail res.setHeader('TEST', 123); - - // should not fail res.setHeader('test_', 123); - + res.setHeader(' test', 123); res.end(); -}, 2)); +})); server.listen(0, common.mustCall(() => { - const session = http2.connect(`http://localhost:${server.address().port}`); - - throws(() => { - session.request({ - 't est': 123 - }); - }, TypeError); - - throws(() => { - session.request({ - ':test': 123 - }); - }, TypeError); - - // should not fail - session.request({ - 'TEST': 123 - }); - - // should not fail - const req = session.request({ - 'test_': 123 - }); - - req.on('end', () => { - session.close(); - server.close(); - }); + const session1 = http2.connect(`http://localhost:${server.address().port}`); + session1.request({ 'test_': 123, 'TEST': 123 }) + .on('end', common.mustCall(() => { + session1.close(); + server.close(); + })); + const session2 = http2.connect(`http://localhost:${server.address().port}`); + session2.on('error', common.mustCall((e) => { + strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); + })); + try { + session2.request({ 't est': 123 }); + } catch (e) { } // eslint-disable-line no-unused-vars + const session3 = http2.connect(`http://localhost:${server.address().port}`); + session3.on('error', common.mustCall((e) => { + strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); + })); + try { + session3.request({ ' test': 123 }); + } catch (e) { } // eslint-disable-line no-unused-vars + const session4 = http2.connect(`http://localhost:${server.address().port}`); + try { + session4.request({ ':test': 123 }); + } catch (e) { + strictEqual(e.code, 'ERR_HTTP2_INVALID_PSEUDOHEADER'); + session4.destroy(); + } })); From 8dedf1f7b61226958c01f6fe59ad9cf4db0860ac Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Mon, 8 Jun 2020 15:28:14 +0530 Subject: [PATCH 4/5] fix tests --- .../parallel/test-http2-invalidheaderfield.js | 23 ++++++++++++++----- .../test-http2-invalidheaderfields-client.js | 20 ++++++++++------ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/test/parallel/test-http2-invalidheaderfield.js b/test/parallel/test-http2-invalidheaderfield.js index ec7746591f1ca9..ab2090ab2a4977 100644 --- a/test/parallel/test-http2-invalidheaderfield.js +++ b/test/parallel/test-http2-invalidheaderfield.js @@ -13,10 +13,14 @@ const { throws, strictEqual } = require('assert'); const server = http2.createServer(common.mustCall((req, res) => { throws(() => { res.setHeader(':path', '/'); - }, TypeError); + }, { + code: 'ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED' + }); throws(() => { res.setHeader('t est', 123); - }, TypeError); + }, { + code: 'ERR_INVALID_HTTP_TOKEN' + }); res.setHeader('TEST', 123); res.setHeader('test_', 123); res.setHeader(' test', 123); @@ -30,20 +34,27 @@ server.listen(0, common.mustCall(() => { session1.close(); server.close(); })); + const session2 = http2.connect(`http://localhost:${server.address().port}`); session2.on('error', common.mustCall((e) => { strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); })); - try { + throws(() => { session2.request({ 't est': 123 }); - } catch (e) { } // eslint-disable-line no-unused-vars + }, { + code: 'ERR_INVALID_HTTP_TOKEN' + }); + const session3 = http2.connect(`http://localhost:${server.address().port}`); session3.on('error', common.mustCall((e) => { strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); })); - try { + throws(() => { session3.request({ ' test': 123 }); - } catch (e) { } // eslint-disable-line no-unused-vars + }, { + code: 'ERR_INVALID_HTTP_TOKEN' + }); + const session4 = http2.connect(`http://localhost:${server.address().port}`); try { session4.request({ ':test': 123 }); diff --git a/test/parallel/test-http2-invalidheaderfields-client.js b/test/parallel/test-http2-invalidheaderfields-client.js index 90a3ea4622fccc..a5681970faab27 100644 --- a/test/parallel/test-http2-invalidheaderfields-client.js +++ b/test/parallel/test-http2-invalidheaderfields-client.js @@ -9,7 +9,11 @@ const server1 = http2.createServer(); server1.listen(0, common.mustCall(() => { const session = http2.connect(`http://localhost:${server1.address().port}`); // Check for req headers - session.request({ 'no underscore': 123 }); + assert.throws(() => { + session.request({ 'no underscore': 123 }); + }, { + code: 'ERR_INVALID_HTTP_TOKEN' + }); session.on('error', common.mustCall((e) => { assert.strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); server1.close(); @@ -18,15 +22,18 @@ server1.listen(0, common.mustCall(() => { const server2 = http2.createServer(common.mustCall((req, res) => { // check for setHeader - res.setHeader('x y z', 123); + assert.throws(() => { + res.setHeader('x y z', 123); + }, { + code: 'ERR_INVALID_HTTP_TOKEN' + }); res.end(); })); server2.listen(0, common.mustCall(() => { const session = http2.connect(`http://localhost:${server2.address().port}`); const req = session.request(); - req.on('error', common.mustCall((e) => { - assert.strictEqual(e.code, 'ERR_HTTP2_STREAM_ERROR'); + req.on('end', common.mustCall(() => { session.close(); server2.close(); })); @@ -39,7 +46,7 @@ const server3 = http2.createServer(common.mustCall((req, res) => { 'an invalid header': 123 }); }), { - code: 'ERR_HTTP2_INVALID_STREAM' + code: 'ERR_INVALID_HTTP_TOKEN' }); res.end(); })); @@ -47,8 +54,7 @@ const server3 = http2.createServer(common.mustCall((req, res) => { server3.listen(0, common.mustCall(() => { const session = http2.connect(`http://localhost:${server3.address().port}`); const req = session.request(); - req.on('error', common.mustCall((e) => { - assert.strictEqual(e.code, 'ERR_HTTP2_STREAM_ERROR'); + req.on('end', common.mustCall(() => { server3.close(); session.close(); })); From e173ca1a77b707ef5a078bf0188f0dfa21def2de Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Tue, 9 Jun 2020 00:49:13 +0530 Subject: [PATCH 5/5] fix tests fix lint error --- test/parallel/test-http2-invalidheaderfield.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-http2-invalidheaderfield.js b/test/parallel/test-http2-invalidheaderfield.js index ab2090ab2a4977..0ff8503b8cf79b 100644 --- a/test/parallel/test-http2-invalidheaderfield.js +++ b/test/parallel/test-http2-invalidheaderfield.js @@ -56,10 +56,10 @@ server.listen(0, common.mustCall(() => { }); const session4 = http2.connect(`http://localhost:${server.address().port}`); - try { + throws(() => { session4.request({ ':test': 123 }); - } catch (e) { - strictEqual(e.code, 'ERR_HTTP2_INVALID_PSEUDOHEADER'); - session4.destroy(); - } + }, { + code: 'ERR_HTTP2_INVALID_PSEUDOHEADER' + }); + session4.close(); }));