GeezerVison is showing github.com — http2: improve http2 coverage · nodejs/node@aa0917c
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/http2/core.js +41-65Lines changed: 41 additions & 65 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -38,6 +38,7 @@ const { | 38 | 38 | isPayloadMeaningless, | 39 | 39 | mapToHeaders, | 40 | 40 | NghttpError, | | 41 | + sessionName, | 41 | 42 | toHeaderObject, | 42 | 43 | updateOptionsBuffer, | 43 | 44 | updateSettingsBuffer | @@ -88,7 +89,6 @@ const { | 88 | 89 | NGHTTP2_CANCEL, | 89 | 90 | NGHTTP2_DEFAULT_WEIGHT, | 90 | 91 | NGHTTP2_FLAG_END_STREAM, | 91 | | - NGHTTP2_HCAT_HEADERS, | 92 | 92 | NGHTTP2_HCAT_PUSH_RESPONSE, | 93 | 93 | NGHTTP2_HCAT_RESPONSE, | 94 | 94 | NGHTTP2_INTERNAL_ERROR, | @@ -129,17 +129,6 @@ const { | 129 | 129 | HTTP_STATUS_SWITCHING_PROTOCOLS | 130 | 130 | } = constants; | 131 | 131 |
| 132 | | -function sessionName(type) { | 133 | | - switch (type) { | 134 | | - case NGHTTP2_SESSION_CLIENT: | 135 | | - return 'client'; | 136 | | - case NGHTTP2_SESSION_SERVER: | 137 | | - return 'server'; | 138 | | - default: | 139 | | - return '<invalid>'; | 140 | | - } | 141 | | -} | 142 | | - | 143 | 132 | // Top level to avoid creating a closure | 144 | 133 | function emit() { | 145 | 134 | this.emit.apply(this, arguments); | @@ -164,59 +153,43 @@ function onSessionHeaders(id, cat, flags, headers) { | 164 | 153 | const obj = toHeaderObject(headers); | 165 | 154 |
| 166 | 155 | if (stream === undefined) { | 167 | | - switch (owner[kType]) { | 168 | | - case NGHTTP2_SESSION_SERVER: | 169 | | - stream = new ServerHttp2Stream(owner, id, | 170 | | - { readable: !endOfStream }, | 171 | | - obj); | 172 | | - if (obj[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD) { | 173 | | - // For head requests, there must not be a body... | 174 | | - // end the writable side immediately. | 175 | | - stream.end(); | 176 | | - const state = stream[kState]; | 177 | | - state.headRequest = true; | 178 | | - } | 179 | | - break; | 180 | | - case NGHTTP2_SESSION_CLIENT: | 181 | | - stream = new ClientHttp2Stream(owner, id, { readable: !endOfStream }); | 182 | | - break; | 183 | | - default: | 184 | | - assert.fail(null, null, | 185 | | - 'Internal HTTP/2 Error. Invalid session type. Please ' + | 186 | | - 'report this as a bug in Node.js'); | | 156 | + // owner[kType] can be only one of two possible values | | 157 | + if (owner[kType] === NGHTTP2_SESSION_SERVER) { | | 158 | + stream = new ServerHttp2Stream(owner, id, | | 159 | + { readable: !endOfStream }, | | 160 | + obj); | | 161 | + if (obj[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD) { | | 162 | + // For head requests, there must not be a body... | | 163 | + // end the writable side immediately. | | 164 | + stream.end(); | | 165 | + const state = stream[kState]; | | 166 | + state.headRequest = true; | | 167 | + } | | 168 | + } else { | | 169 | + stream = new ClientHttp2Stream(owner, id, { readable: !endOfStream }); | 187 | 170 | } | 188 | 171 | streams.set(id, stream); | 189 | 172 | process.nextTick(emit.bind(owner, 'stream', stream, obj, flags, headers)); | 190 | 173 | } else { | 191 | 174 | let event; | 192 | | - let status; | 193 | | - switch (cat) { | 194 | | - case NGHTTP2_HCAT_RESPONSE: | 195 | | - status = obj[HTTP2_HEADER_STATUS]; | 196 | | - if (!endOfStream && | 197 | | - status !== undefined && | 198 | | - status >= 100 && | 199 | | - status < 200) { | 200 | | - event = 'headers'; | 201 | | - } else { | 202 | | - event = 'response'; | 203 | | - } | 204 | | - break; | 205 | | - case NGHTTP2_HCAT_PUSH_RESPONSE: | 206 | | - event = 'push'; | 207 | | - break; | 208 | | - case NGHTTP2_HCAT_HEADERS: | 209 | | - status = obj[HTTP2_HEADER_STATUS]; | 210 | | - if (!endOfStream && status !== undefined && status >= 200) { | 211 | | - event = 'response'; | 212 | | - } else { | 213 | | - event = endOfStream ? 'trailers' : 'headers'; | 214 | | - } | 215 | | - break; | 216 | | - default: | 217 | | - assert.fail(null, null, | 218 | | - 'Internal HTTP/2 Error. Invalid headers category. Please ' + | 219 | | - 'report this as a bug in Node.js'); | | 175 | + const status = obj[HTTP2_HEADER_STATUS]; | | 176 | + if (cat === NGHTTP2_HCAT_RESPONSE) { | | 177 | + if (!endOfStream && | | 178 | + status !== undefined && | | 179 | + status >= 100 && | | 180 | + status < 200) { | | 181 | + event = 'headers'; | | 182 | + } else { | | 183 | + event = 'response'; | | 184 | + } | | 185 | + } else if (cat === NGHTTP2_HCAT_PUSH_RESPONSE) { | | 186 | + event = 'push'; | | 187 | + } else { // cat === NGHTTP2_HCAT_HEADERS: | | 188 | + if (!endOfStream && status !== undefined && status >= 200) { | | 189 | + event = 'response'; | | 190 | + } else { | | 191 | + event = endOfStream ? 'trailers' : 'headers'; | | 192 | + } | 220 | 193 | } | 221 | 194 | debug(`[${sessionName(owner[kType])}] emitting stream '${event}' event`); | 222 | 195 | process.nextTick(emit.bind(stream, event, obj, flags, headers)); | @@ -243,8 +216,10 @@ function onSessionTrailers(id) { | 243 | 216 | 'Internal HTTP/2 Failure. Stream does not exist. Please ' + | 244 | 217 | 'report this as a bug in Node.js'); | 245 | 218 | const getTrailers = stream[kState].getTrailers; | 246 | | - if (typeof getTrailers !== 'function') | 247 | | - return []; | | 219 | + // We should not have been able to get here at all if getTrailers | | 220 | + // was not a function, and there's no code that could have changed | | 221 | + // it between there and here. | | 222 | + assert.strictEqual(typeof getTrailers, 'function'); | 248 | 223 | const trailers = Object.create(null); | 249 | 224 | getTrailers.call(stream, trailers); | 250 | 225 | const headersList = mapToHeaders(trailers, assertValidPseudoHeaderTrailer); | @@ -2492,9 +2467,10 @@ Object.defineProperty(connect, promisify.custom, { | 2492 | 2467 | }); | 2493 | 2468 |
| 2494 | 2469 | function createSecureServer(options, handler) { | 2495 | | - if (typeof options === 'function') { | 2496 | | - handler = options; | 2497 | | - options = Object.create(null); | | 2470 | + if (options == null || typeof options !== 'object') { | | 2471 | + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', | | 2472 | + 'options', | | 2473 | + 'object'); | 2498 | 2474 | } | 2499 | 2475 | debug('creating http2secureserver'); | 2500 | 2476 | return new Http2SecureServer(options, handler); | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/http2/util.js +17-3Lines changed: 17 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -4,6 +4,9 @@ const binding = process.binding('http2'); | 4 | 4 | const errors = require('internal/errors'); | 5 | 5 |
| 6 | 6 | const { | | 7 | + NGHTTP2_SESSION_CLIENT, | | 8 | + NGHTTP2_SESSION_SERVER, | | 9 | + | 7 | 10 | HTTP2_HEADER_STATUS, | 8 | 11 | HTTP2_HEADER_METHOD, | 9 | 12 | HTTP2_HEADER_AUTHORITY, | @@ -453,13 +456,12 @@ class NghttpError extends Error { | 453 | 456 | } | 454 | 457 | } | 455 | 458 |
| 456 | | -function assertIsObject(value, name, types) { | | 459 | +function assertIsObject(value, name, types = 'object') { | 457 | 460 | if (value !== undefined && | 458 | 461 | (value === null || | 459 | 462 | typeof value !== 'object' || | 460 | 463 | Array.isArray(value))) { | 461 | | - const err = new errors.TypeError('ERR_INVALID_ARG_TYPE', | 462 | | - name, types || 'object'); | | 464 | + const err = new errors.TypeError('ERR_INVALID_ARG_TYPE', name, types); | 463 | 465 | Error.captureStackTrace(err, assertIsObject); | 464 | 466 | throw err; | 465 | 467 | } | @@ -529,6 +531,17 @@ function isPayloadMeaningless(method) { | 529 | 531 | return kNoPayloadMethods.has(method); | 530 | 532 | } | 531 | 533 |
| | 534 | +function sessionName(type) { | | 535 | + switch (type) { | | 536 | + case NGHTTP2_SESSION_CLIENT: | | 537 | + return 'client'; | | 538 | + case NGHTTP2_SESSION_SERVER: | | 539 | + return 'server'; | | 540 | + default: | | 541 | + return '<invalid>'; | | 542 | + } | | 543 | +} | | 544 | + | 532 | 545 | module.exports = { | 533 | 546 | assertIsObject, | 534 | 547 | assertValidPseudoHeaderResponse, | @@ -541,6 +554,7 @@ module.exports = { | 541 | 554 | isPayloadMeaningless, | 542 | 555 | mapToHeaders, | 543 | 556 | NghttpError, | | 557 | + sessionName, | 544 | 558 | toHeaderObject, | 545 | 559 | updateOptionsBuffer, | 546 | 560 | updateSettingsBuffer | |
Collapse file Copy file name to clipboardExpand all lines: src/node_http2.cc +2Lines changed: 2 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -890,6 +890,8 @@ void Http2Session::OnHeaders(Nghttp2Stream* stream, | 890 | 890 | Local<Function> fn = env()->push_values_to_array_function(); | 891 | 891 | Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX * 2]; | 892 | 892 |
| | 893 | + CHECK_LE(cat, NGHTTP2_HCAT_HEADERS); | | 894 | + | 893 | 895 | // The headers are passed in above as a linked list of nghttp2_header_list | 894 | 896 | // structs. The following converts that into a JS array with the structure: | 895 | 897 | // [name1, value1, name2, value2, name3, value3, name3, value4] and so on. | |
Collapse file Copy file name to clipboard +45Lines changed: 45 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +// Flags: --expose-http2 | | 2 | +'use strict'; | | 3 | + | | 4 | +const common = require('../common'); | | 5 | +if (!common.hasCrypto) | | 6 | + common.skip('missing crypto'); | | 7 | + | | 8 | +const http2 = require('http2'); | | 9 | + | | 10 | +// Error if options are not passed to createSecureServer | | 11 | + | | 12 | +common.expectsError( | | 13 | + () => http2.createSecureServer(), | | 14 | + { | | 15 | + code: 'ERR_INVALID_ARG_TYPE', | | 16 | + type: TypeError | | 17 | + }); | | 18 | + | | 19 | +common.expectsError( | | 20 | + () => http2.createSecureServer(() => {}), | | 21 | + { | | 22 | + code: 'ERR_INVALID_ARG_TYPE', | | 23 | + type: TypeError | | 24 | + }); | | 25 | + | | 26 | +common.expectsError( | | 27 | + () => http2.createSecureServer(1), | | 28 | + { | | 29 | + code: 'ERR_INVALID_ARG_TYPE', | | 30 | + type: TypeError | | 31 | + }); | | 32 | + | | 33 | +common.expectsError( | | 34 | + () => http2.createSecureServer('test'), | | 35 | + { | | 36 | + code: 'ERR_INVALID_ARG_TYPE', | | 37 | + type: TypeError | | 38 | + }); | | 39 | + | | 40 | +common.expectsError( | | 41 | + () => http2.createSecureServer(null), | | 42 | + { | | 43 | + code: 'ERR_INVALID_ARG_TYPE', | | 44 | + type: TypeError | | 45 | + }); |
Collapse file Copy file name to clipboard +49Lines changed: 49 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +// Flags: --expose-internals | | 2 | +'use strict'; | | 3 | + | | 4 | +const common = require('../common'); | | 5 | +if (!common.hasCrypto) | | 6 | + common.skip('missing crypto'); | | 7 | +const assert = require('assert'); | | 8 | + | | 9 | +const { | | 10 | + assertIsObject, | | 11 | + assertWithinRange, | | 12 | + sessionName | | 13 | +} = require('internal/http2/util'); | | 14 | + | | 15 | +// Code coverage for sessionName utility function | | 16 | +assert.strictEqual(sessionName(0), 'server'); | | 17 | +assert.strictEqual(sessionName(1), 'client'); | | 18 | +[2, '', 'test', {}, [], true].forEach((i) => { | | 19 | + assert.strictEqual(sessionName(2), '<invalid>'); | | 20 | +}); | | 21 | + | | 22 | +// Code coverage for assertWithinRange function | | 23 | +common.expectsError( | | 24 | + () => assertWithinRange('test', -1), | | 25 | + { | | 26 | + code: 'ERR_HTTP2_INVALID_SETTING_VALUE', | | 27 | + type: RangeError, | | 28 | + message: 'Invalid value for setting "test": -1' | | 29 | + }); | | 30 | + | | 31 | +assertWithinRange('test', 1); | | 32 | + | | 33 | +common.expectsError( | | 34 | + () => assertIsObject('foo', 'test'), | | 35 | + { | | 36 | + code: 'ERR_INVALID_ARG_TYPE', | | 37 | + type: TypeError, | | 38 | + message: 'The "test" argument must be of type object' | | 39 | + }); | | 40 | + | | 41 | +common.expectsError( | | 42 | + () => assertIsObject('foo', 'test', ['Date']), | | 43 | + { | | 44 | + code: 'ERR_INVALID_ARG_TYPE', | | 45 | + type: TypeError, | | 46 | + message: 'The "test" argument must be of type Date' | | 47 | + }); | | 48 | + | | 49 | +assertIsObject({}, 'test'); |
Collapse file Copy file name to clipboard +42Lines changed: 42 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +// Flags: --expose-http2 | | 2 | +'use strict'; | | 3 | + | | 4 | +const common = require('../common'); | | 5 | +if (!common.hasCrypto) | | 6 | + common.skip('missing crypto'); | | 7 | + | | 8 | +const http2 = require('http2'); | | 9 | + | | 10 | +// Verify that setTimeout callback verifications work correctly | | 11 | + | | 12 | +{ | | 13 | + const server = http2.createServer(); | | 14 | + common.expectsError( | | 15 | + () => server.setTimeout(10, 'test'), | | 16 | + { | | 17 | + code: 'ERR_INVALID_CALLBACK', | | 18 | + type: TypeError | | 19 | + }); | | 20 | + common.expectsError( | | 21 | + () => server.setTimeout(10, 1), | | 22 | + { | | 23 | + code: 'ERR_INVALID_CALLBACK', | | 24 | + type: TypeError | | 25 | + }); | | 26 | +} | | 27 | + | | 28 | +{ | | 29 | + const server = http2.createSecureServer({}); | | 30 | + common.expectsError( | | 31 | + () => server.setTimeout(10, 'test'), | | 32 | + { | | 33 | + code: 'ERR_INVALID_CALLBACK', | | 34 | + type: TypeError | | 35 | + }); | | 36 | + common.expectsError( | | 37 | + () => server.setTimeout(10, 1), | | 38 | + { | | 39 | + code: 'ERR_INVALID_CALLBACK', | | 40 | + type: TypeError | | 41 | + }); | | 42 | +} |
|