Skip to content

Commit e289540

Browse files
apapirovskiBridgeAR
apapirovski
authored and
BridgeAR
committed
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. PR-URL: nodejs#15109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 198fcb9 commit e289540

3 files changed

Lines changed: 114 additions & 15 deletions

File tree

‎lib/internal/http2/core.js‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -485,39 +485,39 @@ function validatePriorityOptions(options) {
485485
if (options.weight === undefined) {
486486
options.weight = NGHTTP2_DEFAULT_WEIGHT;
487487
} else if (typeof options.weight !== 'number') {
488-
const err = new errors.RangeError('ERR_INVALID_OPT_VALUE',
489-
'weight',
490-
options.weight);
488+
const err = new errors.TypeError('ERR_INVALID_OPT_VALUE',
489+
'weight',
490+
options.weight);
491491
Error.captureStackTrace(err, validatePriorityOptions);
492492
throw err;
493493
}
494494

495495
if (options.parent === undefined) {
496496
options.parent = 0;
497497
} else if (typeof options.parent !== 'number' || options.parent < 0) {
498-
const err = new errors.RangeError('ERR_INVALID_OPT_VALUE',
499-
'parent',
500-
options.parent);
498+
const err = new errors.TypeError('ERR_INVALID_OPT_VALUE',
499+
'parent',
500+
options.parent);
501501
Error.captureStackTrace(err, validatePriorityOptions);
502502
throw err;
503503
}
504504

505505
if (options.exclusive === undefined) {
506506
options.exclusive = false;
507507
} else if (typeof options.exclusive !== 'boolean') {
508-
const err = new errors.RangeError('ERR_INVALID_OPT_VALUE',
509-
'exclusive',
510-
options.exclusive);
508+
const err = new errors.TypeError('ERR_INVALID_OPT_VALUE',
509+
'exclusive',
510+
options.exclusive);
511511
Error.captureStackTrace(err, validatePriorityOptions);
512512
throw err;
513513
}
514514

515515
if (options.silent === undefined) {
516516
options.silent = false;
517517
} else if (typeof options.silent !== 'boolean') {
518-
const err = new errors.RangeError('ERR_INVALID_OPT_VALUE',
519-
'silent',
520-
options.silent);
518+
const err = new errors.TypeError('ERR_INVALID_OPT_VALUE',
519+
'silent',
520+
options.silent);
521521
Error.captureStackTrace(err, validatePriorityOptions);
522522
throw err;
523523
}
@@ -1119,9 +1119,9 @@ class ClientHttp2Session extends Http2Session {
11191119
// preference.
11201120
options.endStream = isPayloadMeaningless(headers[HTTP2_HEADER_METHOD]);
11211121
} else if (typeof options.endStream !== 'boolean') {
1122-
throw new errors.RangeError('ERR_INVALID_OPT_VALUE',
1123-
'endStream',
1124-
options.endStream);
1122+
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
1123+
'endStream',
1124+
options.endStream);
11251125
}
11261126

11271127
if (options.getTrailers !== undefined &&

‎test/parallel/test-http2-client-request-options-errors.js‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Flags: --expose-http2
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
const assert = require('assert');
8+
const http2 = require('http2');
9+
10+
// Check if correct errors are emitted when wrong type of data is passed
11+
// to certain options of ClientHttp2Session request method
12+
13+
const optionsToTest = {
14+
endStream: 'boolean',
15+
getTrailers: 'function',
16+
weight: 'number',
17+
parent: 'number',
18+
exclusive: 'boolean',
19+
silent: 'boolean'
20+
};
21+
22+
const types = {
23+
boolean: true,
24+
function: () => {},
25+
number: 1,
26+
object: {},
27+
array: [],
28+
null: null,
29+
symbol: Symbol('test')
30+
};
31+
32+
const server = http2.createServer(common.mustNotCall());
33+
34+
server.listen(0, common.mustCall(() => {
35+
const port = server.address().port;
36+
const client = http2.connect(`http://localhost:${port}`);
37+
38+
Object.keys(optionsToTest).forEach((option) => {
39+
Object.keys(types).forEach((type) => {
40+
if (type === optionsToTest[option]) {
41+
return;
42+
}
43+
44+
assert.throws(
45+
() => client.request({
46+
':method': 'CONNECT',
47+
':authority': `localhost:${port}`
48+
}, {
49+
[option]: types[type]
50+
}),
51+
common.expectsError({
52+
type: TypeError,
53+
code: 'ERR_INVALID_OPT_VALUE',
54+
message: `The value "${String(types[type])}" is invalid ` +
55+
`for option "${option}"`
56+
})
57+
);
58+
});
59+
});
60+
61+
server.close();
62+
client.destroy();
63+
}));

‎test/parallel/test-http2-connect-method.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ const { URL } = require('url');
1212
const {
1313
HTTP2_HEADER_METHOD,
1414
HTTP2_HEADER_AUTHORITY,
15+
HTTP2_HEADER_SCHEME,
16+
HTTP2_HEADER_PATH,
1517
NGHTTP2_CONNECT_ERROR
1618
} = http2.constants;
1719

@@ -53,6 +55,40 @@ server.listen(0, common.mustCall(() => {
5355
proxy.listen(0, () => {
5456
const client = http2.connect(`http://localhost:${proxy.address().port}`);
5557

58+
// confirm that :authority is required and :scheme & :path are forbidden
59+
assert.throws(
60+
() => client.request({
61+
[HTTP2_HEADER_METHOD]: 'CONNECT'
62+
}),
63+
common.expectsError({
64+
code: 'ERR_HTTP2_CONNECT_AUTHORITY',
65+
message: ':authority header is required for CONNECT requests'
66+
})
67+
);
68+
assert.throws(
69+
() => client.request({
70+
[HTTP2_HEADER_METHOD]: 'CONNECT',
71+
[HTTP2_HEADER_AUTHORITY]: `localhost:${port}`,
72+
[HTTP2_HEADER_SCHEME]: 'http'
73+
}),
74+
common.expectsError({
75+
code: 'ERR_HTTP2_CONNECT_SCHEME',
76+
message: 'The :scheme header is forbidden for CONNECT requests'
77+
})
78+
);
79+
assert.throws(
80+
() => client.request({
81+
[HTTP2_HEADER_METHOD]: 'CONNECT',
82+
[HTTP2_HEADER_AUTHORITY]: `localhost:${port}`,
83+
[HTTP2_HEADER_PATH]: '/'
84+
}),
85+
common.expectsError({
86+
code: 'ERR_HTTP2_CONNECT_PATH',
87+
message: 'The :path header is forbidden for CONNECT requests'
88+
})
89+
);
90+
91+
// valid CONNECT request
5692
const req = client.request({
5793
[HTTP2_HEADER_METHOD]: 'CONNECT',
5894
[HTTP2_HEADER_AUTHORITY]: `localhost:${port}`,

0 commit comments

Comments
 (0)