From d0a0d920b5276b314a21796b89c6b4be873191f2 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 30 Aug 2017 21:49:41 -0400 Subject: [PATCH] http2: adjust error types, test coverage Change error types emitted from request and validatePriorityOptions to be TypeError rather than the incorrect RangeError. Add tests to confirm that all errors are thrown as expected (weight, parent, exclusive, silent, endStream and getTrailers). Add test for method CONNECT throwing error on missing :authority or superfluous :scheme and :path. --- lib/internal/http2/core.js | 30 ++++----- ...est-http2-client-request-options-errors.js | 63 +++++++++++++++++++ test/parallel/test-http2-connect-method.js | 36 +++++++++++ 3 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 test/parallel/test-http2-client-request-options-errors.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 492a2baa5fb77e..1b010fbe6b3a89 100755 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -485,9 +485,9 @@ function validatePriorityOptions(options) { if (options.weight === undefined) { options.weight = NGHTTP2_DEFAULT_WEIGHT; } else if (typeof options.weight !== 'number') { - const err = new errors.RangeError('ERR_INVALID_OPT_VALUE', - 'weight', - options.weight); + const err = new errors.TypeError('ERR_INVALID_OPT_VALUE', + 'weight', + options.weight); Error.captureStackTrace(err, validatePriorityOptions); throw err; } @@ -495,9 +495,9 @@ function validatePriorityOptions(options) { if (options.parent === undefined) { options.parent = 0; } else if (typeof options.parent !== 'number' || options.parent < 0) { - const err = new errors.RangeError('ERR_INVALID_OPT_VALUE', - 'parent', - options.parent); + const err = new errors.TypeError('ERR_INVALID_OPT_VALUE', + 'parent', + options.parent); Error.captureStackTrace(err, validatePriorityOptions); throw err; } @@ -505,9 +505,9 @@ function validatePriorityOptions(options) { if (options.exclusive === undefined) { options.exclusive = false; } else if (typeof options.exclusive !== 'boolean') { - const err = new errors.RangeError('ERR_INVALID_OPT_VALUE', - 'exclusive', - options.exclusive); + const err = new errors.TypeError('ERR_INVALID_OPT_VALUE', + 'exclusive', + options.exclusive); Error.captureStackTrace(err, validatePriorityOptions); throw err; } @@ -515,9 +515,9 @@ function validatePriorityOptions(options) { if (options.silent === undefined) { options.silent = false; } else if (typeof options.silent !== 'boolean') { - const err = new errors.RangeError('ERR_INVALID_OPT_VALUE', - 'silent', - options.silent); + const err = new errors.TypeError('ERR_INVALID_OPT_VALUE', + 'silent', + options.silent); Error.captureStackTrace(err, validatePriorityOptions); throw err; } @@ -1119,9 +1119,9 @@ class ClientHttp2Session extends Http2Session { // preference. options.endStream = isPayloadMeaningless(headers[HTTP2_HEADER_METHOD]); } else if (typeof options.endStream !== 'boolean') { - throw new errors.RangeError('ERR_INVALID_OPT_VALUE', - 'endStream', - options.endStream); + throw new errors.TypeError('ERR_INVALID_OPT_VALUE', + 'endStream', + options.endStream); } if (options.getTrailers !== undefined && diff --git a/test/parallel/test-http2-client-request-options-errors.js b/test/parallel/test-http2-client-request-options-errors.js new file mode 100644 index 00000000000000..92dad5c8430428 --- /dev/null +++ b/test/parallel/test-http2-client-request-options-errors.js @@ -0,0 +1,63 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const http2 = require('http2'); + +// Check if correct errors are emitted when wrong type of data is passed +// to certain options of ClientHttp2Session request method + +const optionsToTest = { + endStream: 'boolean', + getTrailers: 'function', + weight: 'number', + parent: 'number', + exclusive: 'boolean', + silent: 'boolean' +}; + +const types = { + boolean: true, + function: () => {}, + number: 1, + object: {}, + array: [], + null: null, + symbol: Symbol('test') +}; + +const server = http2.createServer(common.mustNotCall()); + +server.listen(0, common.mustCall(() => { + const port = server.address().port; + const client = http2.connect(`http://localhost:${port}`); + + Object.keys(optionsToTest).forEach((option) => { + Object.keys(types).forEach((type) => { + if (type === optionsToTest[option]) { + return; + } + + assert.throws( + () => client.request({ + ':method': 'CONNECT', + ':authority': `localhost:${port}` + }, { + [option]: types[type] + }), + common.expectsError({ + type: TypeError, + code: 'ERR_INVALID_OPT_VALUE', + message: `The value "${String(types[type])}" is invalid ` + + `for option "${option}"` + }) + ); + }); + }); + + server.close(); + client.destroy(); +})); diff --git a/test/parallel/test-http2-connect-method.js b/test/parallel/test-http2-connect-method.js index 4cd8daf484e56b..3626c7f2e51a26 100644 --- a/test/parallel/test-http2-connect-method.js +++ b/test/parallel/test-http2-connect-method.js @@ -12,6 +12,8 @@ const { URL } = require('url'); const { HTTP2_HEADER_METHOD, HTTP2_HEADER_AUTHORITY, + HTTP2_HEADER_SCHEME, + HTTP2_HEADER_PATH, NGHTTP2_CONNECT_ERROR } = http2.constants; @@ -53,6 +55,40 @@ server.listen(0, common.mustCall(() => { proxy.listen(0, () => { const client = http2.connect(`http://localhost:${proxy.address().port}`); + // confirm that :authority is required and :scheme & :path are forbidden + assert.throws( + () => client.request({ + [HTTP2_HEADER_METHOD]: 'CONNECT' + }), + common.expectsError({ + code: 'ERR_HTTP2_CONNECT_AUTHORITY', + message: ':authority header is required for CONNECT requests' + }) + ); + assert.throws( + () => client.request({ + [HTTP2_HEADER_METHOD]: 'CONNECT', + [HTTP2_HEADER_AUTHORITY]: `localhost:${port}`, + [HTTP2_HEADER_SCHEME]: 'http' + }), + common.expectsError({ + code: 'ERR_HTTP2_CONNECT_SCHEME', + message: 'The :scheme header is forbidden for CONNECT requests' + }) + ); + assert.throws( + () => client.request({ + [HTTP2_HEADER_METHOD]: 'CONNECT', + [HTTP2_HEADER_AUTHORITY]: `localhost:${port}`, + [HTTP2_HEADER_PATH]: '/' + }), + common.expectsError({ + code: 'ERR_HTTP2_CONNECT_PATH', + message: 'The :path header is forbidden for CONNECT requests' + }) + ); + + // valid CONNECT request const req = client.request({ [HTTP2_HEADER_METHOD]: 'CONNECT', [HTTP2_HEADER_AUTHORITY]: `localhost:${port}`,