From 26e1f8e01c1d25200e08ef0c39f8c8d8c3c0550c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 31 Jul 2017 11:53:15 -0700 Subject: [PATCH] http2: address initial pr feedback Backport-PR-URL: https://github.com/nodejs/node/pull/14813 Backport-Reviewed-By: Anna Henningsen Backport-Reviewed-By: Timothy Gu PR-URL: https://github.com/nodejs/node/pull/14239 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Matteo Collina --- doc/api/http2.md | 11 +-- lib/internal/http2/core.js | 32 ++++----- lib/internal/http2/util.js | 1 - src/node.h | 4 +- src/node_crypto_bio.cc | 1 + src/node_http2.cc | 51 +++++-------- src/node_http2_core.h | 15 ++-- src/stream_base.cc | 1 - ...p2-client-stream-destroy-before-connect.js | 6 +- .../test-http2-compat-serverresponse-end.js | 2 +- ...est-http2-compat-serverresponse-headers.js | 18 ++++- ...http2-compat-serverresponse-write-no-cb.js | 8 ++- ...test-http2-create-client-secure-session.js | 4 ++ test/parallel/test-http2-https-fallback.js | 40 +++++------ .../test-http2-multi-content-length.js | 72 ++++++++++--------- test/parallel/test-http2-serve-file.js | 6 +- test/parallel/test-http2-server-startup.js | 4 ++ test/parallel/test-http2-window-size.js | 2 +- 18 files changed, 144 insertions(+), 134 deletions(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index bf2f6a3c526e7b..ae08fe49a7bb50 100755 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -410,9 +410,10 @@ added: REPLACEME (No Error). * `lastStreamID` {number} The Stream ID of the last successfully processed `Http2Stream` on this `Http2Session`. - * `opaqueData` {Buffer} A `Buffer` instance containing arbitrary additional - data to send to the peer upon disconnection. This is used, typically, to - provide additional data for debugging failures, if necessary. + * `opaqueData` {Buffer|Uint8Array} A `Buffer` or `Uint8Array` instance + containing arbitrary additional data to send to the peer upon disconnection. + This is used, typically, to provide additional data for debugging failures, + if necessary. * `callback` {Function} A callback that is invoked after the session shutdown has been completed. * Returns: {undefined} @@ -965,7 +966,7 @@ added: REPLACEME initiated. * Returns: {undefined} -Initiates a push stream. The callback is invoked with the new `Htt2Stream` +Initiates a push stream. The callback is invoked with the new `Http2Stream` instance created for the push stream. ```js @@ -1619,7 +1620,7 @@ passed in. These will always be reported by a synchronous `throw`. State Errors occur when an action is attempted at an incorrect time (for instance, attempting to send data on a stream after it has closed). These will -be repoorted using either a synchronous `throw` or via an `'error'` event on +be reported using either a synchronous `throw` or via an `'error'` event on the `Http2Stream`, `Http2Session` or HTTP/2 Server objects, depending on where and when the error occurs. diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index dc3033f2edc84d..97a36050ac01e0 100755 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -2,8 +2,9 @@ /* eslint-disable no-use-before-define */ +require('internal/util').assertCrypto(); + const binding = process.binding('http2'); -const debug = require('util').debuglog('http2'); const assert = require('assert'); const Buffer = require('buffer').Buffer; const EventEmitter = require('events'); @@ -18,6 +19,8 @@ const { onServerStream } = require('internal/http2/compat'); const { utcDate } = require('internal/http'); const { _connectionListener: httpConnectionListener } = require('http'); const { isUint8Array } = process.binding('util'); +const debug = util.debuglog('http2'); + const { assertIsObject, @@ -459,9 +462,9 @@ function requestOnConnect(headers, options) { } function validatePriorityOptions(options) { - if (options.weight === undefined) + if (options.weight === undefined) { options.weight = NGHTTP2_DEFAULT_WEIGHT; - else if (typeof options.weight !== 'number') { + } else if (typeof options.weight !== 'number') { const err = new errors.RangeError('ERR_INVALID_OPT_VALUE', 'weight', options.weight); @@ -469,9 +472,9 @@ function validatePriorityOptions(options) { throw err; } - if (options.parent === undefined) + if (options.parent === undefined) { options.parent = 0; - else if (typeof options.parent !== 'number' || options.parent < 0) { + } else if (typeof options.parent !== 'number' || options.parent < 0) { const err = new errors.RangeError('ERR_INVALID_OPT_VALUE', 'parent', options.parent); @@ -479,9 +482,9 @@ function validatePriorityOptions(options) { throw err; } - if (options.exclusive === undefined) + if (options.exclusive === undefined) { options.exclusive = false; - else if (typeof options.exclusive !== 'boolean') { + } else if (typeof options.exclusive !== 'boolean') { const err = new errors.RangeError('ERR_INVALID_OPT_VALUE', 'exclusive', options.exclusive); @@ -489,9 +492,9 @@ function validatePriorityOptions(options) { throw err; } - if (options.silent === undefined) + if (options.silent === undefined) { options.silent = false; - else if (typeof options.silent !== 'boolean') { + } else if (typeof options.silent !== 'boolean') { const err = new errors.RangeError('ERR_INVALID_OPT_VALUE', 'silent', options.silent); @@ -982,7 +985,7 @@ class Http2Session extends EventEmitter { options = Object.assign(Object.create(null), options); if (options.opaqueData !== undefined && - !Buffer.isBuffer(options.opaqueData)) { + !isUint8Array(options.opaqueData)) { throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'opaqueData', options.opaqueData); @@ -1008,13 +1011,6 @@ class Http2Session extends EventEmitter { options.lastStreamID); } - if (options.opaqueData !== undefined && - !Buffer.isBuffer(options.opaqueData)) { - throw new errors.TypeError('ERR_INVALID_OPT_VALUE', - 'opaqueData', - options.opaqueData); - } - if (callback) { this.on('shutdown', callback); } @@ -1233,7 +1229,6 @@ function streamOnceReady() { function abort(stream) { if (!stream[kState].aborted && - stream._writableState && !(stream._writableState.ended || stream._writableState.ending)) { stream.emit('aborted'); stream[kState].aborted = true; @@ -1351,7 +1346,6 @@ class Http2Stream extends Duplex { if (err) throw util._errnoException(err, 'write', req.error); this._bytesDispatched += req.bytes; - } _writev(data, cb) { diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index ea36444fadfa36..33e8ff33e64edb 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -219,7 +219,6 @@ function getDefaultSettings() { if ((flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) === (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { - console.log('setting it'); holder.maxConcurrentStreams = settingsBuffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]; } diff --git a/src/node.h b/src/node.h index a3c29c22423b02..269abdfd73d00a 100644 --- a/src/node.h +++ b/src/node.h @@ -258,7 +258,9 @@ NODE_EXTERN void RunAtExit(Environment* env); v8::Isolate* isolate = target->GetIsolate(); \ v8::Local context = isolate->GetCurrentContext(); \ v8::Local constant_name = \ - v8::String::NewFromUtf8(isolate, #constant); \ + v8::String::NewFromUtf8(isolate, #constant, \ + v8::NewStringType::kInternalized) \ + .ToLocalChecked(); \ v8::Local constant_value = \ v8::Number::New(isolate, static_cast(constant)); \ v8::PropertyAttribute constant_attributes = \ diff --git a/src/node_crypto_bio.cc b/src/node_crypto_bio.cc index 4c84973f75facc..00fd0b420c38c5 100644 --- a/src/node_crypto_bio.cc +++ b/src/node_crypto_bio.cc @@ -357,6 +357,7 @@ size_t NodeBIO::IndexOf(char delim, size_t limit) { return max; } + void NodeBIO::Write(const char* data, size_t size) { size_t offset = 0; size_t left = size; diff --git a/src/node_http2.cc b/src/node_http2.cc index ee12072939617e..bbf4f1c9efd514 100755 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -61,33 +61,28 @@ Http2Options::Http2Options(Environment* env) { uint32_t* buffer = env->http2_options_buffer(); uint32_t flags = buffer[IDX_OPTIONS_FLAGS]; - if ((flags & (1 << IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE)) == - (1 << IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE)) { + if (flags & (1 << IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE)) { SetMaxDeflateDynamicTableSize( buffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE]); } - if ((flags & (1 << IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS)) == - (1 << IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS)) { + if (flags & (1 << IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS)) { SetMaxReservedRemoteStreams( buffer[IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS]); } - if ((flags & (1 << IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH)) == - (1 << IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH)) { + if (flags & (1 << IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH)) { SetMaxSendHeaderBlockLength( buffer[IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH]); } SetPeerMaxConcurrentStreams(100); // Recommended default - if ((flags & (1 << IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS)) == - (1 << IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS)) { + if (flags & (1 << IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS)) { SetPeerMaxConcurrentStreams( buffer[IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS]); } - if ((flags & (1 << IDX_OPTIONS_PADDING_STRATEGY)) == - (1 << IDX_OPTIONS_PADDING_STRATEGY)) { + if (flags & (1 << IDX_OPTIONS_PADDING_STRATEGY)) { SetPaddingStrategy(buffer[IDX_OPTIONS_PADDING_STRATEGY]); } } @@ -197,48 +192,42 @@ void PackSettings(const FunctionCallbackInfo& args) { uint32_t* const buffer = env->http2_settings_buffer(); uint32_t flags = buffer[IDX_SETTINGS_COUNT]; - if ((flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) == - (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { + if (flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { DEBUG_HTTP2("Setting header table size: %d\n", buffer[IDX_SETTINGS_HEADER_TABLE_SIZE]); entries.push_back({NGHTTP2_SETTINGS_HEADER_TABLE_SIZE, buffer[IDX_SETTINGS_HEADER_TABLE_SIZE]}); } - if ((flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) == - (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { + if (flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { DEBUG_HTTP2("Setting max concurrent streams: %d\n", buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]); entries.push_back({NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]}); } - if ((flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) == - (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { + if (flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { DEBUG_HTTP2("Setting max frame size: %d\n", buffer[IDX_SETTINGS_MAX_FRAME_SIZE]); entries.push_back({NGHTTP2_SETTINGS_MAX_FRAME_SIZE, buffer[IDX_SETTINGS_MAX_FRAME_SIZE]}); } - if ((flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) == - (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { + if (flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { DEBUG_HTTP2("Setting initial window size: %d\n", buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE]); entries.push_back({NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE]}); } - if ((flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) == - (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { + if (flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { DEBUG_HTTP2("Setting max header list size: %d\n", buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE]); entries.push_back({NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE, buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE]}); } - if ((flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) == - (1 << IDX_SETTINGS_ENABLE_PUSH)) { + if (flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) { DEBUG_HTTP2("Setting enable push: %d\n", buffer[IDX_SETTINGS_ENABLE_PUSH]); entries.push_back({NGHTTP2_SETTINGS_ENABLE_PUSH, @@ -457,48 +446,42 @@ void Http2Session::SubmitSettings(const FunctionCallbackInfo& args) { std::vector entries; entries.reserve(6); - if ((flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) == - (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { + if (flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { DEBUG_HTTP2("Setting header table size: %d\n", buffer[IDX_SETTINGS_HEADER_TABLE_SIZE]); entries.push_back({NGHTTP2_SETTINGS_HEADER_TABLE_SIZE, buffer[IDX_SETTINGS_HEADER_TABLE_SIZE]}); } - if ((flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) == - (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { + if (flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) { DEBUG_HTTP2("Setting max concurrent streams: %d\n", buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]); entries.push_back({NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS]}); } - if ((flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) == - (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { + if (flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) { DEBUG_HTTP2("Setting max frame size: %d\n", buffer[IDX_SETTINGS_MAX_FRAME_SIZE]); entries.push_back({NGHTTP2_SETTINGS_MAX_FRAME_SIZE, buffer[IDX_SETTINGS_MAX_FRAME_SIZE]}); } - if ((flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) == - (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { + if (flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) { DEBUG_HTTP2("Setting initial window size: %d\n", buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE]); entries.push_back({NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE]}); } - if ((flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) == - (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { + if (flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) { DEBUG_HTTP2("Setting max header list size: %d\n", buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE]); entries.push_back({NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE, buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE]}); } - if ((flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) == - (1 << IDX_SETTINGS_ENABLE_PUSH)) { + if (flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) { DEBUG_HTTP2("Setting enable push: %d\n", buffer[IDX_SETTINGS_ENABLE_PUSH]); entries.push_back({NGHTTP2_SETTINGS_ENABLE_PUSH, diff --git a/src/node_http2_core.h b/src/node_http2_core.h index 80dd98e9ff1e22..8461ab64180015 100755 --- a/src/node_http2_core.h +++ b/src/node_http2_core.h @@ -143,8 +143,7 @@ class Nghttp2Session { // Removes a stream instance from this session inline void RemoveStream(int32_t id); - virtual void Send(uv_buf_t* buf, - size_t length) {} + virtual void Send(uv_buf_t* buf, size_t length) {} virtual void OnHeaders(Nghttp2Stream* stream, nghttp2_header_list* headers, nghttp2_headers_category cat, @@ -294,7 +293,7 @@ class Nghttp2Stream { // Returns true if this stream has been destroyed inline bool IsDestroyed() const { - return (flags_ & NGHTTP2_STREAM_DESTROYED) == NGHTTP2_STREAM_DESTROYED; + return flags_ & NGHTTP2_STREAM_DESTROYED; } // Queue outbound chunks of data to be sent on this stream @@ -340,7 +339,7 @@ class Nghttp2Stream { // Returns true if this stream is writable. inline bool IsWritable() const { - return (flags_ & NGHTTP2_STREAM_FLAG_SHUT) == 0; + return !(flags_ & NGHTTP2_STREAM_FLAG_SHUT); } // Start Reading. If there are queued data chunks, they are pushed into @@ -352,15 +351,15 @@ class Nghttp2Stream { // Returns true if reading is paused inline bool IsPaused() const { - return (flags_ & NGHTTP2_STREAM_READ_PAUSED) == NGHTTP2_STREAM_READ_PAUSED; + return flags_ & NGHTTP2_STREAM_READ_PAUSED; } // Returns true if this stream is in the reading state, which occurs when // the NGHTTP2_STREAM_READ_START flag has been set and the // NGHTTP2_STREAM_READ_PAUSED flag is *not* set. inline bool IsReading() const { - return ((flags_ & NGHTTP2_STREAM_READ_START) == NGHTTP2_STREAM_READ_START) - && ((flags_ & NGHTTP2_STREAM_READ_PAUSED) == 0); + return flags_ & NGHTTP2_STREAM_READ_START && + !(flags_ & NGHTTP2_STREAM_READ_PAUSED); } inline void Close(int32_t code) { @@ -374,7 +373,7 @@ class Nghttp2Stream { // Returns true if this stream has been closed either by receiving or // sending an RST_STREAM frame. inline bool IsClosed() const { - return (flags_ & NGHTTP2_STREAM_CLOSED) == NGHTTP2_STREAM_CLOSED; + return flags_ & NGHTTP2_STREAM_CLOSED; } // Returns the RST_STREAM code used to close this stream diff --git a/src/stream_base.cc b/src/stream_base.cc index 3e94054546d69b..51bad94a4fabc0 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -408,7 +408,6 @@ void StreamBase::AfterWrite(WriteWrap* req_wrap, int status) { // Unref handle property Local req_wrap_obj = req_wrap->object(); req_wrap_obj->Delete(env->context(), env->handle_string()).FromJust(); - wrap->OnAfterWrite(req_wrap); Local argv[] = { diff --git a/test/parallel/test-http2-client-stream-destroy-before-connect.js b/test/parallel/test-http2-client-stream-destroy-before-connect.js index 5ab0cac5082aed..d8a9db4d371aba 100644 --- a/test/parallel/test-http2-client-stream-destroy-before-connect.js +++ b/test/parallel/test-http2-client-stream-destroy-before-connect.js @@ -16,10 +16,8 @@ server.on('stream', (stream) => { // must call). The error may or may not be reported depending on operating // system specific timings. stream.on('error', (err) => { - if (err) { - assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_ERROR'); - assert.strictEqual(err.message, 'Stream closed with error code 2'); - } + assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_ERROR'); + assert.strictEqual(err.message, 'Stream closed with error code 2'); }); stream.respond({}); stream.end(); diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index 88536e36256c28..6b4b375764ec5d 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -1,8 +1,8 @@ // Flags: --expose-http2 'use strict'; -const { strictEqual } = require('assert'); const { mustCall, mustNotCall } = require('../common'); +const { strictEqual } = require('assert'); const { createServer, connect, diff --git a/test/parallel/test-http2-compat-serverresponse-headers.js b/test/parallel/test-http2-compat-serverresponse-headers.js index 8e11babddfddd4..de80469265fe92 100644 --- a/test/parallel/test-http2-compat-serverresponse-headers.js +++ b/test/parallel/test-http2-compat-serverresponse-headers.js @@ -42,13 +42,25 @@ server.listen(0, common.mustCall(function() { assert.throws(function() { response.setHeader(':status', 'foobar'); - }, Error); + }, common.expectsError({ + code: 'ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED', + type: Error, + message: 'Cannot set HTTP/2 pseudo-headers' + })); assert.throws(function() { response.setHeader(real, null); - }, TypeError); + }, common.expectsError({ + code: 'ERR_HTTP2_INVALID_HEADER_VALUE', + type: TypeError, + message: 'Value must not be undefined or null' + })); assert.throws(function() { response.setHeader(real, undefined); - }, TypeError); + }, common.expectsError({ + code: 'ERR_HTTP2_INVALID_HEADER_VALUE', + type: TypeError, + message: 'Value must not be undefined or null' + })); response.setHeader(real, expectedValue); const expectedHeaderNames = [real]; diff --git a/test/parallel/test-http2-compat-serverresponse-write-no-cb.js b/test/parallel/test-http2-compat-serverresponse-write-no-cb.js index 2428cebd8bcf09..e6164ae9319d17 100644 --- a/test/parallel/test-http2-compat-serverresponse-write-no-cb.js +++ b/test/parallel/test-http2-compat-serverresponse-write-no-cb.js @@ -1,8 +1,8 @@ // Flags: --expose-http2 'use strict'; -const { throws } = require('assert'); const { mustCall, mustNotCall, expectsError } = require('../common'); +const { throws } = require('assert'); const { createServer, connect } = require('http2'); // Http2ServerResponse.write does not imply there is a callback @@ -35,7 +35,11 @@ const expectedError = expectsError({ response.on('error', mustNotCall()); throws( () => { response.write('muahaha'); }, - /The stream is already closed/ + expectsError({ + code: 'ERR_HTTP2_STREAM_CLOSED', + type: Error, + message: 'The stream is already closed' + }) ); server.close(); })); diff --git a/test/parallel/test-http2-create-client-secure-session.js b/test/parallel/test-http2-create-client-secure-session.js index a775ebd71e0b68..824b899dfcdb40 100644 --- a/test/parallel/test-http2-create-client-secure-session.js +++ b/test/parallel/test-http2-create-client-secure-session.js @@ -2,6 +2,10 @@ 'use strict'; const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + const assert = require('assert'); const path = require('path'); const fs = require('fs'); diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js index b0424397f22696..d49c7987143d33 100644 --- a/test/parallel/test-http2-https-fallback.js +++ b/test/parallel/test-http2-https-fallback.js @@ -1,11 +1,11 @@ // Flags: --expose-http2 'use strict'; -const { - fixturesDir, - mustCall, - mustNotCall -} = require('../common'); +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + const { strictEqual } = require('assert'); const { join } = require('path'); const { readFileSync } = require('fs'); @@ -18,7 +18,7 @@ const { connect: tls } = require('tls'); const countdown = (count, done) => () => --count === 0 && done(); function loadKey(keyname) { - return readFileSync(join(fixturesDir, 'keys', keyname)); + return readFileSync(join(common.fixturesDir, 'keys', keyname)); } const key = loadKey('agent8-key.pem'); @@ -46,14 +46,14 @@ function onSession(session) { }; const request = session.request(headers); - request.on('response', mustCall((headers) => { + request.on('response', common.mustCall((headers) => { strictEqual(headers[':status'], 200); strictEqual(headers['content-type'], 'application/json'); })); request.setEncoding('utf8'); let raw = ''; request.on('data', (chunk) => { raw += chunk; }); - request.on('end', mustCall(() => { + request.on('end', common.mustCall(() => { const { alpnProtocol, httpVersion } = JSON.parse(raw); strictEqual(alpnProtocol, 'h2'); strictEqual(httpVersion, '2.0'); @@ -68,12 +68,12 @@ function onSession(session) { { const server = createSecureServer( { cert, key, allowHTTP1: true }, - mustCall(onRequest, 2) + common.mustCall(onRequest, 2) ); server.listen(0); - server.on('listening', mustCall(() => { + server.on('listening', common.mustCall(() => { const { port } = server.address(); const origin = `https://localhost:${port}`; @@ -83,13 +83,13 @@ function onSession(session) { connect( origin, clientOptions, - mustCall(onSession.bind({ cleanup, server })) + common.mustCall(onSession.bind({ cleanup, server })) ); // HTTP/1.1 client get( Object.assign(parse(origin), clientOptions), - mustCall((response) => { + common.mustCall((response) => { strictEqual(response.statusCode, 200); strictEqual(response.statusMessage, 'OK'); strictEqual(response.headers['content-type'], 'application/json'); @@ -97,7 +97,7 @@ function onSession(session) { response.setEncoding('utf8'); let raw = ''; response.on('data', (chunk) => { raw += chunk; }); - response.on('end', mustCall(() => { + response.on('end', common.mustCall(() => { const { alpnProtocol, httpVersion } = JSON.parse(raw); strictEqual(alpnProtocol, false); strictEqual(httpVersion, '1.1'); @@ -113,16 +113,16 @@ function onSession(session) { { const server = createSecureServer( { cert, key }, - mustCall(onRequest) + common.mustCall(onRequest) ); - server.on('unknownProtocol', mustCall((socket) => { + server.on('unknownProtocol', common.mustCall((socket) => { socket.destroy(); }, 2)); server.listen(0); - server.on('listening', mustCall(() => { + server.on('listening', common.mustCall(() => { const { port } = server.address(); const origin = `https://localhost:${port}`; @@ -132,15 +132,15 @@ function onSession(session) { connect( origin, clientOptions, - mustCall(onSession.bind({ cleanup, server })) + common.mustCall(onSession.bind({ cleanup, server })) ); // HTTP/1.1 client - get(Object.assign(parse(origin), clientOptions), mustNotCall()) - .on('error', mustCall(cleanup)); + get(Object.assign(parse(origin), clientOptions), common.mustNotCall()) + .on('error', common.mustCall(cleanup)); // Incompatible ALPN TLS client tls(Object.assign({ port, ALPNProtocols: ['fake'] }, clientOptions)) - .on('error', mustCall(cleanup)); + .on('error', common.mustCall(cleanup)); })); } diff --git a/test/parallel/test-http2-multi-content-length.js b/test/parallel/test-http2-multi-content-length.js index 9ab1d48c21bed9..96407215a33f6a 100644 --- a/test/parallel/test-http2-multi-content-length.js +++ b/test/parallel/test-http2-multi-content-length.js @@ -22,37 +22,43 @@ server.listen(0, common.mustCall(() => { } } - // Request 1 will fail because there are two content-length header values - const req = client.request({ - ':method': 'POST', - 'content-length': 1, - 'Content-Length': 2 - }); - req.on('error', common.expectsError({ - code: 'ERR_HTTP2_HEADER_SINGLE_VALUE', - type: Error, - message: 'Header field "content-length" must have only a single value' - })); - req.on('error', common.mustCall(maybeClose)); - req.end('a'); - - // Request 2 will succeed - const req2 = client.request({ - ':method': 'POST', - 'content-length': 1 - }); - req2.resume(); - req2.on('end', common.mustCall(maybeClose)); - req2.end('a'); - - // Request 3 will fail because nghttp2 does not allow the content-length - // header to be set for non-payload bearing requests... - const req3 = client.request({ 'content-length': 1 }); - req3.resume(); - req3.on('end', common.mustCall(maybeClose)); - req3.on('error', common.expectsError({ - code: 'ERR_HTTP2_STREAM_ERROR', - type: Error, - message: 'Stream closed with error code 1' - })); + { + // Request 1 will fail because there are two content-length header values + const req = client.request({ + ':method': 'POST', + 'content-length': 1, + 'Content-Length': 2 + }); + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_HEADER_SINGLE_VALUE', + type: Error, + message: 'Header field "content-length" must have only a single value' + })); + req.on('error', common.mustCall(maybeClose)); + req.end('a'); + } + + { + // Request 2 will succeed + const req = client.request({ + ':method': 'POST', + 'content-length': 1 + }); + req.resume(); + req.on('end', common.mustCall(maybeClose)); + req.end('a'); + } + + { + // Request 3 will fail because nghttp2 does not allow the content-length + // header to be set for non-payload bearing requests... + const req = client.request({ 'content-length': 1 }); + req.resume(); + req.on('end', common.mustCall(maybeClose)); + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + type: Error, + message: 'Stream closed with error code 1' + })); + } })); diff --git a/test/parallel/test-http2-serve-file.js b/test/parallel/test-http2-serve-file.js index 6c88a4286311fd..b7149963a1b2c3 100644 --- a/test/parallel/test-http2-serve-file.js +++ b/test/parallel/test-http2-serve-file.js @@ -2,6 +2,10 @@ 'use strict'; const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + const assert = require('assert'); const http2 = require('http2'); const fs = require('fs'); @@ -42,7 +46,7 @@ server.on('stream', (stream, headers) => { }); }); -server.listen(8000, () => { +server.listen(0, () => { const secureContext = tls.createSecureContext({ ca }); const client = http2.connect(`https://localhost:${server.address().port}`, diff --git a/test/parallel/test-http2-server-startup.js b/test/parallel/test-http2-server-startup.js index c2e94f3ac4502a..1d236d2e65f207 100644 --- a/test/parallel/test-http2-server-startup.js +++ b/test/parallel/test-http2-server-startup.js @@ -6,6 +6,10 @@ // other than start listening. const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + const assert = require('assert'); const http2 = require('http2'); const path = require('path'); diff --git a/test/parallel/test-http2-window-size.js b/test/parallel/test-http2-window-size.js index d914e99f6aa195..f96a273fec5b5f 100644 --- a/test/parallel/test-http2-window-size.js +++ b/test/parallel/test-http2-window-size.js @@ -6,8 +6,8 @@ // TODO: This test makes large buffer allocations (128KiB) and should be tested // on smaller / IoT platforms in case this poses problems for these targets. -const assert = require('assert'); const common = require('../common'); +const assert = require('assert'); const h2 = require('http2'); // Given a list of buffers and an initial window size, have a server write