diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index fa565e9bd67386..04f4bb3c5a338c 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -17,6 +17,21 @@ const kRawHeaders = Symbol('rawHeaders'); const kTrailers = Symbol('trailers'); const kRawTrailers = Symbol('rawTrailers'); +const { + NGHTTP2_NO_ERROR, + + HTTP2_HEADER_AUTHORITY, + HTTP2_HEADER_METHOD, + HTTP2_HEADER_PATH, + HTTP2_HEADER_SCHEME, + HTTP2_HEADER_STATUS, + + HTTP_STATUS_CONTINUE, + HTTP_STATUS_EXPECTATION_FAILED, + HTTP_STATUS_METHOD_NOT_ALLOWED, + HTTP_STATUS_OK +} = constants; + let statusMessageWarned = false; // Defines and implements an API compatibility layer on top of the core @@ -24,6 +39,8 @@ let statusMessageWarned = false; // close as possible to the current require('http') API function assertValidHeader(name, value) { + if (name === '') + throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name); if (isPseudoHeader(name)) throw new errors.Error('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED'); if (value === undefined || value === null) @@ -32,15 +49,11 @@ function assertValidHeader(name, value) { function isPseudoHeader(name) { switch (name) { - case ':status': - return true; - case ':method': - return true; - case ':path': - return true; - case ':authority': - return true; - case ':scheme': + case HTTP2_HEADER_STATUS: // :status + case HTTP2_HEADER_METHOD: // :method + case HTTP2_HEADER_PATH: // :path + case HTTP2_HEADER_AUTHORITY: // :authority + case HTTP2_HEADER_SCHEME: // :scheme return true; default: return false; @@ -58,8 +71,7 @@ function statusMessageWarn() { } function onStreamData(chunk) { - const request = this[kRequest]; - if (!request.push(chunk)) + if (!this[kRequest].push(chunk)) this.pause(); } @@ -71,8 +83,7 @@ function onStreamTrailers(trailers, flags, rawTrailers) { function onStreamEnd() { // Cause the request stream to end as well. - const request = this[kRequest]; - request.push(null); + this[kRequest].push(null); } function onStreamError(error) { @@ -86,13 +97,11 @@ function onStreamError(error) { } function onRequestPause() { - const stream = this[kStream]; - stream.pause(); + this[kStream].pause(); } function onRequestResume() { - const stream = this[kStream]; - stream.resume(); + this[kStream].resume(); } function onRequestDrain() { @@ -101,13 +110,11 @@ function onRequestDrain() { } function onStreamResponseDrain() { - const response = this[kResponse]; - response.emit('drain'); + this[kResponse].emit('drain'); } function onStreamClosedRequest() { - const req = this[kRequest]; - req.push(null); + this[kRequest].push(null); } function onStreamClosedResponse() { @@ -127,9 +134,8 @@ class Http2ServerRequest extends Readable { constructor(stream, headers, options, rawHeaders) { super(options); this[kState] = { - statusCode: null, closed: false, - closedCode: constants.NGHTTP2_NO_ERROR + closedCode: NGHTTP2_NO_ERROR }; this[kHeaders] = headers; this[kRawHeaders] = rawHeaders; @@ -155,23 +161,17 @@ class Http2ServerRequest extends Readable { } get closed() { - const state = this[kState]; - return Boolean(state.closed); + return this[kState].closed; } get code() { - const state = this[kState]; - return Number(state.closedCode); + return this[kState].closedCode; } get stream() { return this[kStream]; } - get statusCode() { - return this[kState].statusCode; - } - get headers() { return this[kHeaders]; } @@ -201,7 +201,10 @@ class Http2ServerRequest extends Readable { } get socket() { - return this.stream.session.socket; + const stream = this[kStream]; + if (stream === undefined) + return; + return stream.session.socket; } get connection() { @@ -210,7 +213,7 @@ class Http2ServerRequest extends Readable { _read(nread) { const stream = this[kStream]; - if (stream) { + if (stream !== undefined) { stream.resume(); } else { this.emit('error', new errors.Error('ERR_HTTP2_STREAM_CLOSED')); @@ -218,46 +221,23 @@ class Http2ServerRequest extends Readable { } get method() { - const headers = this[kHeaders]; - if (headers === undefined) - return; - return headers[constants.HTTP2_HEADER_METHOD]; + return this[kHeaders][HTTP2_HEADER_METHOD]; } get authority() { - const headers = this[kHeaders]; - if (headers === undefined) - return; - return headers[constants.HTTP2_HEADER_AUTHORITY]; + return this[kHeaders][HTTP2_HEADER_AUTHORITY]; } get scheme() { - const headers = this[kHeaders]; - if (headers === undefined) - return; - return headers[constants.HTTP2_HEADER_SCHEME]; + return this[kHeaders][HTTP2_HEADER_SCHEME]; } get url() { - return this.path; + return this[kHeaders][HTTP2_HEADER_PATH]; } set url(url) { - this.path = url; - } - - get path() { - const headers = this[kHeaders]; - if (headers === undefined) - return; - return headers[constants.HTTP2_HEADER_PATH]; - } - - set path(path) { - let headers = this[kHeaders]; - if (headers === undefined) - headers = this[kHeaders] = Object.create(null); - headers[constants.HTTP2_HEADER_PATH] = path; + this[kHeaders][HTTP2_HEADER_PATH] = url; } setTimeout(msecs, callback) { @@ -271,10 +251,10 @@ class Http2ServerRequest extends Readable { if (state.closed) return; if (code !== undefined) - state.closedCode = code; + state.closedCode = Number(code); state.closed = true; this.push(null); - this[kStream] = undefined; + process.nextTick(() => (this[kStream] = undefined)); } } @@ -283,12 +263,12 @@ class Http2ServerResponse extends Stream { super(options); this[kState] = { sendDate: true, - statusCode: constants.HTTP_STATUS_OK, - headerCount: 0, - trailerCount: 0, + statusCode: HTTP_STATUS_OK, closed: false, - closedCode: constants.NGHTTP2_NO_ERROR + closedCode: NGHTTP2_NO_ERROR }; + this[kHeaders] = Object.create(null); + this[kTrailers] = Object.create(null); this[kStream] = stream; stream[kResponse] = this; this.writable = true; @@ -305,13 +285,11 @@ class Http2ServerResponse extends Stream { } get closed() { - const state = this[kState]; - return Boolean(state.closed); + return this[kState].closed; } get code() { - const state = this[kState]; - return Number(state.closedCode); + return this[kState].closedCode; } get stream() { @@ -324,7 +302,7 @@ class Http2ServerResponse extends Stream { } get sendDate() { - return Boolean(this[kState].sendDate); + return this[kState].sendDate; } set sendDate(bool) { @@ -336,70 +314,71 @@ class Http2ServerResponse extends Stream { } set statusCode(code) { - const state = this[kState]; code |= 0; if (code >= 100 && code < 200) throw new errors.RangeError('ERR_HTTP2_INFO_STATUS_NOT_ALLOWED'); - if (code < 200 || code > 599) + if (code < 100 || code > 599) throw new errors.RangeError('ERR_HTTP2_STATUS_INVALID', code); - state.statusCode = code; + this[kState].statusCode = code; + } + + setTrailer(name, value) { + if (typeof name !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + + name = name.trim().toLowerCase(); + assertValidHeader(name, value); + this[kTrailers][name] = String(value); } addTrailers(headers) { - let trailers = this[kTrailers]; const keys = Object.keys(headers); - if (keys.length === 0) - return; - if (trailers === undefined) - trailers = this[kTrailers] = Object.create(null); + let key = ''; for (var i = 0; i < keys.length; i++) { - trailers[keys[i]] = String(headers[keys[i]]); + key = keys[i]; + this.setTrailer(key, headers[key]); } } getHeader(name) { - const headers = this[kHeaders]; - if (headers === undefined) - return; - name = String(name).trim().toLowerCase(); - return headers[name]; + if (typeof name !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + + name = name.trim().toLowerCase(); + return this[kHeaders][name]; } getHeaderNames() { - const headers = this[kHeaders]; - if (headers === undefined) - return []; - return Object.keys(headers); + return Object.keys(this[kHeaders]); } getHeaders() { - const headers = this[kHeaders]; - return Object.assign({}, headers); + return Object.assign({}, this[kHeaders]); } hasHeader(name) { - const headers = this[kHeaders]; - if (headers === undefined) - return false; - name = String(name).trim().toLowerCase(); - return Object.prototype.hasOwnProperty.call(headers, name); + if (typeof name !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + + name = name.trim().toLowerCase(); + return Object.prototype.hasOwnProperty.call(this[kHeaders], name); } removeHeader(name) { - const headers = this[kHeaders]; - if (headers === undefined) - return; - name = String(name).trim().toLowerCase(); - delete headers[name]; + if (typeof name !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + + name = name.trim().toLowerCase(); + delete this[kHeaders][name]; } setHeader(name, value) { - name = String(name).trim().toLowerCase(); + if (typeof name !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + + name = name.trim().toLowerCase(); assertValidHeader(name, value); - let headers = this[kHeaders]; - if (headers === undefined) - headers = this[kHeaders] = Object.create(null); - headers[name] = String(value); + this[kHeaders][name] = String(value); } get statusMessage() { @@ -413,7 +392,8 @@ class Http2ServerResponse extends Stream { } flushHeaders() { - if (this[kStream].headersSent === false) + const stream = this[kStream]; + if (stream !== undefined && stream.headersSent === false) this[kBeginSend](); } @@ -427,11 +407,14 @@ class Http2ServerResponse extends Stream { } const stream = this[kStream]; + if (stream === undefined) { + throw new errors.Error('ERR_HTTP2_STREAM_CLOSED'); + } if (stream.headersSent === true) { throw new errors.Error('ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND'); } - if (headers) { + if (typeof headers === 'object') { const keys = Object.keys(headers); let key = ''; for (var i = 0; i < keys.length; i++) { @@ -479,15 +462,16 @@ class Http2ServerResponse extends Stream { this.write(chunk, encoding); } - if (typeof cb === 'function' && stream !== undefined) { + if (stream === undefined) { + return; + } + + if (typeof cb === 'function') { stream.once('finish', cb); } this[kBeginSend]({ endStream: true }); - - if (stream !== undefined) { - stream.end(); - } + stream.end(); } destroy(err) { @@ -519,19 +503,19 @@ class Http2ServerResponse extends Stream { [kBeginSend](options) { const stream = this[kStream]; - options = options || Object.create(null); - if (stream !== undefined && stream.headersSent === false) { + if (stream !== undefined && + stream.destroyed === false && + stream.headersSent === false) { + options = options || Object.create(null); const state = this[kState]; - const headers = this[kHeaders] || Object.create(null); - headers[constants.HTTP2_HEADER_STATUS] = state.statusCode; + const headers = this[kHeaders]; + headers[HTTP2_HEADER_STATUS] = state.statusCode; if (stream.finished === true) options.endStream = true; options.getTrailers = (trailers) => { Object.assign(trailers, this[kTrailers]); }; - if (stream.destroyed === false) { - stream.respond(headers, options); - } + stream.respond(headers, options); } } @@ -540,11 +524,11 @@ class Http2ServerResponse extends Stream { if (state.closed) return; if (code !== undefined) - state.closedCode = code; + state.closedCode = Number(code); state.closed = true; state.headersSent = this[kStream].headersSent; this.end(); - this[kStream] = undefined; + process.nextTick(() => (this[kStream] = undefined)); this.emit('finish'); } @@ -553,7 +537,7 @@ class Http2ServerResponse extends Stream { const stream = this[kStream]; if (stream === undefined) return false; this[kStream].additionalHeaders({ - [constants.HTTP2_HEADER_STATUS]: constants.HTTP_STATUS_CONTINUE + [HTTP2_HEADER_STATUS]: HTTP_STATUS_CONTINUE }); return true; } @@ -566,10 +550,10 @@ function onServerStream(stream, headers, flags, rawHeaders) { const response = new Http2ServerResponse(stream); // Check for the CONNECT method - const method = headers[constants.HTTP2_HEADER_METHOD]; + const method = headers[HTTP2_HEADER_METHOD]; if (method === 'CONNECT') { if (!server.emit('connect', request, response)) { - response.statusCode = constants.HTTP_STATUS_METHOD_NOT_ALLOWED; + response.statusCode = HTTP_STATUS_METHOD_NOT_ALLOWED; response.end(); } return; @@ -587,7 +571,7 @@ function onServerStream(stream, headers, flags, rawHeaders) { } else if (server.listenerCount('checkExpectation')) { server.emit('checkExpectation', request, response); } else { - response.statusCode = constants.HTTP_STATUS_EXPECTATION_FAILED; + response.statusCode = HTTP_STATUS_EXPECTATION_FAILED; response.end(); } return; diff --git a/test/parallel/test-http2-compat-serverrequest-headers.js b/test/parallel/test-http2-compat-serverrequest-headers.js index d5a66391058708..286a0e19c87c11 100644 --- a/test/parallel/test-http2-compat-serverrequest-headers.js +++ b/test/parallel/test-http2-compat-serverrequest-headers.js @@ -21,9 +21,9 @@ server.listen(0, common.mustCall(function() { 'foo-bar': 'abc123' }; + assert.strictEqual(request.path, undefined); assert.strictEqual(request.method, expected[':method']); assert.strictEqual(request.scheme, expected[':scheme']); - assert.strictEqual(request.path, expected[':path']); assert.strictEqual(request.url, expected[':path']); assert.strictEqual(request.authority, expected[':authority']); @@ -41,11 +41,6 @@ server.listen(0, common.mustCall(function() { request.url = '/one'; assert.strictEqual(request.url, '/one'); - assert.strictEqual(request.path, '/one'); - - request.path = '/two'; - assert.strictEqual(request.url, '/two'); - assert.strictEqual(request.path, '/two'); response.on('finish', common.mustCall(function() { server.close(); diff --git a/test/parallel/test-http2-compat-serverrequest.js b/test/parallel/test-http2-compat-serverrequest.js index ebc91b7a9edabd..a6882c6a9bf1c4 100644 --- a/test/parallel/test-http2-compat-serverrequest.js +++ b/test/parallel/test-http2-compat-serverrequest.js @@ -15,7 +15,6 @@ server.listen(0, common.mustCall(function() { const port = server.address().port; server.once('request', common.mustCall(function(request, response) { const expected = { - statusCode: null, version: '2.0', httpVersionMajor: 2, httpVersionMinor: 0 @@ -24,7 +23,6 @@ server.listen(0, common.mustCall(function() { assert.strictEqual(request.closed, false); assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR); - assert.strictEqual(request.statusCode, expected.statusCode); assert.strictEqual(request.httpVersion, expected.version); assert.strictEqual(request.httpVersionMajor, expected.httpVersionMajor); assert.strictEqual(request.httpVersionMinor, expected.httpVersionMinor); diff --git a/test/parallel/test-http2-compat-serverresponse-destroy.js b/test/parallel/test-http2-compat-serverresponse-destroy.js index 40c73b0887c32f..f2b3ae7cfefa49 100644 --- a/test/parallel/test-http2-compat-serverresponse-destroy.js +++ b/test/parallel/test-http2-compat-serverresponse-destroy.js @@ -26,7 +26,7 @@ const server = http2.createServer(common.mustCall((req, res) => { assert.strictEqual(res.closed, true); })); - if (req.path !== '/') { + if (req.url !== '/') { nextError = errors.shift(); } res.destroy(nextError); diff --git a/test/parallel/test-http2-compat-serverresponse-finished.js b/test/parallel/test-http2-compat-serverresponse-finished.js index c80bdc33f8615a..e54255f67cd2ee 100644 --- a/test/parallel/test-http2-compat-serverresponse-finished.js +++ b/test/parallel/test-http2-compat-serverresponse-finished.js @@ -8,13 +8,18 @@ const assert = require('assert'); const h2 = require('http2'); // Http2ServerResponse.finished - const server = h2.createServer(); server.listen(0, common.mustCall(function() { const port = server.address().port; server.once('request', common.mustCall(function(request, response) { response.on('finish', common.mustCall(function() { + assert.ok(request.stream !== undefined); + assert.ok(response.stream !== undefined); server.close(); + process.nextTick(common.mustCall(() => { + assert.strictEqual(request.stream, undefined); + assert.strictEqual(response.stream, undefined); + })); })); assert.strictEqual(response.finished, false); response.end(); diff --git a/test/parallel/test-http2-compat-serverresponse-headers.js b/test/parallel/test-http2-compat-serverresponse-headers.js index b2285f9d2aabfc..2cc82d4dd3c82f 100644 --- a/test/parallel/test-http2-compat-serverresponse-headers.js +++ b/test/parallel/test-http2-compat-serverresponse-headers.js @@ -42,6 +42,31 @@ server.listen(0, common.mustCall(function() { response.removeHeader(denormalised); assert.strictEqual(response.hasHeader(denormalised), false); + common.expectsError( + () => response.hasHeader(), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "name" argument must be of type string' + } + ); + common.expectsError( + () => response.getHeader(), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "name" argument must be of type string' + } + ); + common.expectsError( + () => response.removeHeader(), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "name" argument must be of type string' + } + ); + [ ':status', ':method', @@ -70,6 +95,14 @@ server.listen(0, common.mustCall(function() { type: TypeError, message: 'Value must not be undefined or null' })); + common.expectsError( + () => response.setHeader(), // header name undefined + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "name" argument must be of type string' + } + ); response.setHeader(real, expectedValue); const expectedHeaderNames = [real]; diff --git a/test/parallel/test-http2-compat-serverresponse-writehead.js b/test/parallel/test-http2-compat-serverresponse-writehead.js index 04d7499083524f..12de2983468781 100644 --- a/test/parallel/test-http2-compat-serverresponse-writehead.js +++ b/test/parallel/test-http2-compat-serverresponse-writehead.js @@ -22,6 +22,11 @@ server.listen(0, common.mustCall(function() { response.on('finish', common.mustCall(function() { server.close(); + process.nextTick(common.mustCall(() => { + common.expectsError(() => { response.writeHead(300); }, { + code: 'ERR_HTTP2_STREAM_CLOSED' + }); + })); })); response.end(); })); diff --git a/test/parallel/test-http2-createwritereq.js b/test/parallel/test-http2-createwritereq.js index 7e4bd5cf112ee9..f4151d94e6aa10 100644 --- a/test/parallel/test-http2-createwritereq.js +++ b/test/parallel/test-http2-createwritereq.js @@ -30,7 +30,7 @@ const testsToRun = Object.keys(encodings).length; let testsFinished = 0; const server = http2.createServer(common.mustCall((req, res) => { - const testEncoding = encodings[req.path.slice(1)]; + const testEncoding = encodings[req.url.slice(1)]; req.on('data', common.mustCall((chunk) => assert.ok( Buffer.from(testString, testEncoding).equals(chunk)