diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 462b0c51b88a0a..f20889c98c704c 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -16,6 +16,7 @@ const kHeaders = Symbol('headers'); const kRawHeaders = Symbol('rawHeaders'); const kTrailers = Symbol('trailers'); const kRawTrailers = Symbol('rawTrailers'); +const kSetHeader = Symbol('setHeader'); const { NGHTTP2_NO_ERROR, @@ -32,6 +33,7 @@ const { HTTP_STATUS_OK } = constants; +let socketWarned = false; let statusMessageWarned = false; // Defines and implements an API compatibility layer on top of the core @@ -39,7 +41,7 @@ let statusMessageWarned = false; // close as possible to the current require('http') API function assertValidHeader(name, value) { - if (name === '') + if (name === '' || typeof name !== 'string') throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name); if (isPseudoHeader(name)) throw new errors.Error('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED'); @@ -70,6 +72,18 @@ function statusMessageWarn() { } } +function socketWarn() { + if (socketWarned === false) { + process.emitWarning( + 'Because the of the specific serialization and processing requirements ' + + 'imposed by the HTTP/2 protocol, it is not recommended for user code ' + + 'to read data from or write data to a Socket instance.', + 'UnsupportedWarning' + ); + socketWarned = true; + } +} + function onStreamData(chunk) { if (!this[kRequest].push(chunk)) this.pause(); @@ -141,6 +155,10 @@ function resumeStream(stream) { stream.resume(); } +function unsetStream(self) { + self[kStream] = undefined; +} + class Http2ServerRequest extends Readable { constructor(stream, headers, options, rawHeaders) { super(options); @@ -170,12 +188,8 @@ class Http2ServerRequest extends Readable { this.on('resume', onRequestResume); } - get closed() { - return this[kState].closed; - } - - get code() { - return this[kState].closedCode; + get complete() { + return this._readableState.ended || this[kState].closed; } get stream() { @@ -211,6 +225,8 @@ class Http2ServerRequest extends Readable { } get socket() { + socketWarn(); + const stream = this[kStream]; if (stream === undefined) return; @@ -234,6 +250,13 @@ class Http2ServerRequest extends Readable { return this[kHeaders][HTTP2_HEADER_METHOD]; } + set method(method) { + if (typeof method !== 'string' || method.trim() === '') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'method', 'string'); + + this[kHeaders][HTTP2_HEADER_METHOD] = method; + } + get authority() { return this[kHeaders][HTTP2_HEADER_AUTHORITY]; } @@ -264,7 +287,7 @@ class Http2ServerRequest extends Readable { state.closedCode = Number(code); state.closed = true; this.push(null); - process.nextTick(() => (this[kStream] = undefined)); + process.nextTick(unsetStream, this); } } @@ -272,10 +295,12 @@ class Http2ServerResponse extends Stream { constructor(stream, options) { super(options); this[kState] = { + closed: false, + closedCode: NGHTTP2_NO_ERROR, + ending: false, + headRequest: false, sendDate: true, statusCode: HTTP_STATUS_OK, - closed: false, - closedCode: NGHTTP2_NO_ERROR }; this[kHeaders] = Object.create(null); this[kTrailers] = Object.create(null); @@ -290,17 +315,32 @@ class Http2ServerResponse extends Stream { stream.on('finish', onfinish); } + // User land modules such as finalhandler just check truthiness of this + // but if someone is actually trying to use this for more than that + // then we simply can't support such use cases + get _header() { + return this.headersSent; + } + get finished() { const stream = this[kStream]; - return stream === undefined || stream._writableState.ended; + return stream === undefined || + stream._writableState.ended || + this[kState].closed; } - get closed() { - return this[kState].closed; + + get socket() { + socketWarn(); + + const stream = this[kStream]; + if (stream === undefined) + return; + return stream.session.socket; } - get code() { - return this[kState].closedCode; + get connection() { + return this.socket; } get stream() { @@ -309,7 +349,7 @@ class Http2ServerResponse extends Stream { get headersSent() { const stream = this[kStream]; - return stream !== undefined ? stream.headersSent : this[kState].headersSent; + return stream === undefined ? this[kState].headersSent : stream.headersSent; } get sendDate() { @@ -339,7 +379,7 @@ class Http2ServerResponse extends Stream { name = name.trim().toLowerCase(); assertValidHeader(name, value); - this[kTrailers][name] = String(value); + this[kTrailers][name] = value; } addTrailers(headers) { @@ -379,6 +419,13 @@ class Http2ServerResponse extends Stream { if (typeof name !== 'string') throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + const stream = this[kStream]; + const state = this[kState]; + if (!stream && !state.headersSent) + return; + if (state.headersSent || stream.headersSent) + throw new errors.Error('ERR_HTTP2_HEADERS_SENT'); + name = name.trim().toLowerCase(); delete this[kHeaders][name]; } @@ -387,9 +434,20 @@ class Http2ServerResponse extends Stream { if (typeof name !== 'string') throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + const stream = this[kStream]; + const state = this[kState]; + if (!stream && !state.headersSent) + return; + if (state.headersSent || stream.headersSent) + throw new errors.Error('ERR_HTTP2_HEADERS_SENT'); + + this[kSetHeader](name, value); + } + + [kSetHeader](name, value) { name = name.trim().toLowerCase(); assertValidHeader(name, value); - this[kHeaders][name] = String(value); + this[kHeaders][name] = value; } get statusMessage() { @@ -403,12 +461,22 @@ class Http2ServerResponse extends Stream { } flushHeaders() { - const stream = this[kStream]; - if (stream !== undefined && stream.headersSent === false) - this[kBeginSend](); + const state = this[kState]; + if (!state.closed && !this[kStream].headersSent) { + this.writeHead(state.statusCode); + } } writeHead(statusCode, statusMessage, headers) { + const state = this[kState]; + + if (state.closed) { + throw new errors.Error('ERR_HTTP2_STREAM_CLOSED'); + } + if (this[kStream].headersSent) { + throw new errors.Error('ERR_HTTP2_HEADERS_SENT'); + } + if (typeof statusMessage === 'string') { statusMessageWarn(); } @@ -417,24 +485,16 @@ class Http2ServerResponse extends Stream { headers = statusMessage; } - 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 (typeof headers === 'object') { const keys = Object.keys(headers); let key = ''; for (var i = 0; i < keys.length; i++) { key = keys[i]; - this.setHeader(key, headers[key]); + this[kSetHeader](key, headers[key]); } } - this.statusCode = statusCode; + state.statusCode = statusCode; this[kBeginSend](); } @@ -446,7 +506,7 @@ class Http2ServerResponse extends Stream { encoding = 'utf8'; } - if (stream === undefined) { + if (this[kState].closed) { const err = new errors.Error('ERR_HTTP2_STREAM_CLOSED'); if (typeof cb === 'function') process.nextTick(cb, err); @@ -454,12 +514,20 @@ class Http2ServerResponse extends Stream { throw err; return; } - this[kBeginSend](); + if (!stream.headersSent) { + this.writeHead(this[kState].statusCode); + } return stream.write(chunk, encoding, cb); } end(chunk, encoding, cb) { const stream = this[kStream]; + const state = this[kState]; + + if ((state.closed || state.ending) && + (stream === undefined || state.headRequest === stream.headRequest)) { + return false; + } if (typeof chunk === 'function') { cb = chunk; @@ -468,19 +536,31 @@ class Http2ServerResponse extends Stream { cb = encoding; encoding = 'utf8'; } - if (this.finished === true) { - return false; - } + if (chunk !== null && chunk !== undefined) { this.write(chunk, encoding); } + const isFinished = this.finished; + state.headRequest = stream.headRequest; + state.ending = true; + if (typeof cb === 'function') { - stream.once('finish', cb); + if (isFinished) + this.once('finish', cb); + else + stream.once('finish', cb); + } + + if (!stream.headersSent) { + this.writeHead(this[kState].statusCode); } - this[kBeginSend]({ endStream: true }); - stream.end(); + if (isFinished) { + this[kFinish](); + } else { + stream.end(); + } } destroy(err) { @@ -500,7 +580,7 @@ class Http2ServerResponse extends Stream { if (typeof callback !== 'function') throw new errors.TypeError('ERR_INVALID_CALLBACK'); const stream = this[kStream]; - if (stream === undefined) { + if (this[kState].closed) { process.nextTick(callback, new errors.Error('ERR_HTTP2_STREAM_CLOSED')); return; } @@ -510,43 +590,38 @@ class Http2ServerResponse extends Stream { }); } - [kBeginSend](options) { - const stream = this[kStream]; - if (stream !== undefined && - stream.destroyed === false && - stream.headersSent === false) { - const headers = this[kHeaders]; - headers[HTTP2_HEADER_STATUS] = this[kState].statusCode; - options = options || Object.create(null); - options.getTrailers = (trailers) => { - Object.assign(trailers, this[kTrailers]); - }; - stream.respond(headers, options); - } + [kBeginSend]() { + const state = this[kState]; + const headers = this[kHeaders]; + headers[HTTP2_HEADER_STATUS] = state.statusCode; + const options = { + endStream: state.ending, + getTrailers: (trailers) => Object.assign(trailers, this[kTrailers]) + }; + this[kStream].respond(headers, options); } [kFinish](code) { + const stream = this[kStream]; const state = this[kState]; - if (state.closed) + if (state.closed || stream.headRequest !== state.headRequest) { return; + } if (code !== undefined) state.closedCode = Number(code); state.closed = true; - state.headersSent = this[kStream].headersSent; - this.end(); - process.nextTick(() => (this[kStream] = undefined)); + state.headersSent = stream.headersSent; + process.nextTick(unsetStream, this); this.emit('finish'); } // TODO doesn't support callbacks writeContinue() { const stream = this[kStream]; - if (stream === undefined || - stream.headersSent === true || - stream.destroyed === true) { + if (this[kState].closed || stream.headersSent) { return false; } - this[kStream].additionalHeaders({ + stream.additionalHeaders({ [HTTP2_HEADER_STATUS]: HTTP_STATUS_CONTINUE }); return true; diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index bc41cd4bf913c0..9b872ed10ad794 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -162,8 +162,7 @@ function onSessionHeaders(id, cat, flags, headers) { // For head requests, there must not be a body... // end the writable side immediately. stream.end(); - const state = stream[kState]; - state.headRequest = true; + stream[kState].headRequest = true; } } else { stream = new ClientHttp2Stream(owner, id, { readable: !endOfStream }); @@ -1272,6 +1271,7 @@ class Http2Stream extends Duplex { rst: false, rstCode: NGHTTP2_NO_ERROR, headersSent: false, + headRequest: false, aborted: false, closeHandler: onSessionClose.bind(this) }; @@ -1328,6 +1328,11 @@ class Http2Stream extends Duplex { return this[kState].aborted; } + // true if dealing with a HEAD request + get headRequest() { + return this[kState].headRequest; + } + // The error code reported when this Http2Stream was closed. get rstCode() { return this[kState].rst ? this[kState].rstCode : undefined; diff --git a/test/parallel/test-http2-compat-serverrequest-end.js b/test/parallel/test-http2-compat-serverrequest-end.js index a84f73883d660d..5401f1f950881c 100644 --- a/test/parallel/test-http2-compat-serverrequest-end.js +++ b/test/parallel/test-http2-compat-serverrequest-end.js @@ -4,6 +4,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +const assert = require('assert'); const h2 = require('http2'); // Http2ServerRequest should always end readable stream @@ -13,8 +14,10 @@ const server = h2.createServer(); server.listen(0, common.mustCall(function() { const port = server.address().port; server.once('request', common.mustCall(function(request, response) { + assert.strictEqual(request.complete, false); request.on('data', () => {}); request.on('end', common.mustCall(() => { + assert.strictEqual(request.complete, true); response.on('finish', common.mustCall(function() { server.close(); })); diff --git a/test/parallel/test-http2-compat-serverrequest-headers.js b/test/parallel/test-http2-compat-serverrequest-headers.js index 286a0e19c87c11..5d62e56640a70d 100644 --- a/test/parallel/test-http2-compat-serverrequest-headers.js +++ b/test/parallel/test-http2-compat-serverrequest-headers.js @@ -42,6 +42,27 @@ server.listen(0, common.mustCall(function() { request.url = '/one'; assert.strictEqual(request.url, '/one'); + // third-party plugins for packages like express use query params to + // change the request method + request.method = 'POST'; + assert.strictEqual(request.method, 'POST'); + common.expectsError( + () => request.method = ' ', + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "method" argument must be of type string' + } + ); + common.expectsError( + () => request.method = true, + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "method" argument must be of type string' + } + ); + response.on('finish', common.mustCall(function() { server.close(); })); diff --git a/test/parallel/test-http2-compat-serverrequest-pipe.js b/test/parallel/test-http2-compat-serverrequest-pipe.js index b952372841ed91..ae7501f4b22402 100644 --- a/test/parallel/test-http2-compat-serverrequest-pipe.js +++ b/test/parallel/test-http2-compat-serverrequest-pipe.js @@ -20,6 +20,7 @@ const server = http2.createServer(); server.on('request', common.mustCall((req, res) => { const dest = req.pipe(fs.createWriteStream(fn)); dest.on('finish', common.mustCall(() => { + assert.strictEqual(req.complete, true); assert.deepStrictEqual(fs.readFileSync(loc), fs.readFileSync(fn)); fs.unlinkSync(fn); res.end(); diff --git a/test/parallel/test-http2-compat-serverrequest-socket-warn.js b/test/parallel/test-http2-compat-serverrequest-socket-warn.js new file mode 100644 index 00000000000000..1189839cf028ce --- /dev/null +++ b/test/parallel/test-http2-compat-serverrequest-socket-warn.js @@ -0,0 +1,47 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const h2 = require('http2'); + +// Http2ServerRequest.socket should warn one time when being accessed + +const unsupportedWarned = common.mustCall(1); +process.on('warning', ({ name, message }) => { + const expectedMessage = + 'Because the of the specific serialization and processing requirements ' + + 'imposed by the HTTP/2 protocol, it is not recommended for user code ' + + 'to read data from or write data to a Socket instance.'; + if (name === 'UnsupportedWarning' && message === expectedMessage) + unsupportedWarned(); +}); + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + request.socket; + request.socket; // should not warn + response.socket; // should not warn + response.end(); + })); + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('end', common.mustCall(function() { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-compat-serverrequest.js b/test/parallel/test-http2-compat-serverrequest.js index 8c72e3876527c2..a6ca8cbfd7967c 100644 --- a/test/parallel/test-http2-compat-serverrequest.js +++ b/test/parallel/test-http2-compat-serverrequest.js @@ -20,9 +20,6 @@ server.listen(0, common.mustCall(function() { httpVersionMinor: 0 }; - assert.strictEqual(request.closed, false); - assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR); - assert.strictEqual(request.httpVersion, expected.version); assert.strictEqual(request.httpVersionMajor, expected.httpVersionMajor); assert.strictEqual(request.httpVersionMinor, expected.httpVersionMinor); @@ -32,8 +29,6 @@ server.listen(0, common.mustCall(function() { assert.strictEqual(request.socket, request.connection); response.on('finish', common.mustCall(function() { - assert.strictEqual(request.closed, true); - assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR); process.nextTick(() => { assert.strictEqual(request.socket, undefined); server.close(); diff --git a/test/parallel/test-http2-compat-serverresponse-destroy.js b/test/parallel/test-http2-compat-serverresponse-destroy.js index 20329e0d8fdfb4..41f6062d5bce51 100644 --- a/test/parallel/test-http2-compat-serverresponse-destroy.js +++ b/test/parallel/test-http2-compat-serverresponse-destroy.js @@ -23,7 +23,6 @@ const server = http2.createServer(common.mustCall((req, res) => { res.on('finish', common.mustCall(() => { assert.doesNotThrow(() => res.destroy(nextError)); - assert.strictEqual(res.closed, true); process.nextTick(() => { assert.doesNotThrow(() => res.destroy(nextError)); }); diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index 957983b0aedb6d..46dd545da655a3 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -1,7 +1,13 @@ // Flags: --expose-http2 'use strict'; -const { mustCall, mustNotCall, hasCrypto, skip } = require('../common'); +const { + mustCall, + mustNotCall, + hasCrypto, + platformTimeout, + skip +} = require('../common'); if (!hasCrypto) skip('missing crypto'); const { strictEqual } = require('assert'); @@ -19,15 +25,16 @@ const { // It may be invoked repeatedly without throwing errors // but callback will only be called once const server = createServer(mustCall((request, response) => { - strictEqual(response.closed, false); response.end('end', 'utf8', mustCall(() => { - strictEqual(response.closed, true); response.end(mustNotCall()); process.nextTick(() => { response.end(mustNotCall()); server.close(); }); })); + response.on('finish', mustCall(() => { + response.end(mustNotCall()); + })); response.end(mustNotCall()); })); server.listen(0, mustCall(() => { @@ -112,12 +119,12 @@ const { } { - // Http2ServerResponse.end is not necessary on HEAD requests since the stream - // is already closed. Headers, however, can still be sent to the client. + // Http2ServerResponse.end is necessary on HEAD requests in compat + // for http1 compatibility const server = createServer(mustCall((request, response) => { strictEqual(response.finished, true); response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); - response.end(mustNotCall()); + response.end('data', mustCall()); })); server.listen(0, mustCall(() => { const { port } = server.address(); @@ -145,3 +152,145 @@ const { })); })); } + +{ + // Should be able to call .end with cb from stream 'streamClosed' + const server = createServer(mustCall((request, response) => { + response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); + response.stream.on('streamClosed', mustCall(() => { + response.end(mustCall()); + })); + })); + server.listen(0, mustCall(() => { + const { port } = server.address(); + const url = `http://localhost:${port}`; + const client = connect(url, mustCall(() => { + const headers = { + ':path': '/', + ':method': 'HEAD', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('response', mustCall((headers, flags) => { + strictEqual(headers[HTTP2_HEADER_STATUS], HTTP_STATUS_OK); + strictEqual(flags, 5); // the end of stream flag is set + strictEqual(headers.foo, 'bar'); + })); + request.on('data', mustNotCall()); + request.on('end', mustCall(() => { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); + })); +} + +{ + // Should be able to respond to HEAD request after timeout + const server = createServer(mustCall((request, response) => { + setTimeout(mustCall(() => { + response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); + response.end('data', mustCall()); + }), platformTimeout(10)); + })); + server.listen(0, mustCall(() => { + const { port } = server.address(); + const url = `http://localhost:${port}`; + const client = connect(url, mustCall(() => { + const headers = { + ':path': '/', + ':method': 'HEAD', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('response', mustCall((headers, flags) => { + strictEqual(headers[HTTP2_HEADER_STATUS], HTTP_STATUS_OK); + strictEqual(flags, 5); // the end of stream flag is set + strictEqual(headers.foo, 'bar'); + })); + request.on('data', mustNotCall()); + request.on('end', mustCall(() => { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); + })); +} + +{ + // finish should only trigger after 'end' is called + const server = createServer(mustCall((request, response) => { + let finished = false; + response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); + response.on('finish', mustCall(() => { + finished = false; + })); + response.end('data', mustCall(() => { + strictEqual(finished, false); + response.end('data', mustNotCall()); + })); + })); + server.listen(0, mustCall(() => { + const { port } = server.address(); + const url = `http://localhost:${port}`; + const client = connect(url, mustCall(() => { + const headers = { + ':path': '/', + ':method': 'HEAD', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('response', mustCall((headers, flags) => { + strictEqual(headers[HTTP2_HEADER_STATUS], HTTP_STATUS_OK); + strictEqual(flags, 5); // the end of stream flag is set + strictEqual(headers.foo, 'bar'); + })); + request.on('data', mustNotCall()); + request.on('end', mustCall(() => { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); + })); +} + +{ + // Should be able to respond to HEAD with just .end + const server = createServer(mustCall((request, response) => { + response.end('data', mustCall()); + response.end(mustNotCall()); + })); + server.listen(0, mustCall(() => { + const { port } = server.address(); + const url = `http://localhost:${port}`; + const client = connect(url, mustCall(() => { + const headers = { + ':path': '/', + ':method': 'HEAD', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('response', mustCall((headers, flags) => { + strictEqual(headers[HTTP2_HEADER_STATUS], HTTP_STATUS_OK); + strictEqual(flags, 5); // the end of stream flag is set + })); + request.on('data', mustNotCall()); + request.on('end', mustCall(() => { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); + })); +} diff --git a/test/parallel/test-http2-compat-serverresponse-finished.js b/test/parallel/test-http2-compat-serverresponse-finished.js index e54255f67cd2ee..64752b0769fba3 100644 --- a/test/parallel/test-http2-compat-serverresponse-finished.js +++ b/test/parallel/test-http2-compat-serverresponse-finished.js @@ -6,12 +6,17 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); const h2 = require('http2'); +const net = require('net'); // 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) { + assert.ok(response.socket instanceof net.Socket); + assert.ok(response.connection instanceof net.Socket); + assert.strictEqual(response.socket, response.connection); + response.on('finish', common.mustCall(function() { assert.ok(request.stream !== undefined); assert.ok(response.stream !== undefined); @@ -19,6 +24,8 @@ server.listen(0, common.mustCall(function() { process.nextTick(common.mustCall(() => { assert.strictEqual(request.stream, undefined); assert.strictEqual(response.stream, undefined); + assert.strictEqual(response.socket, undefined); + assert.strictEqual(response.connection, undefined); })); })); assert.strictEqual(response.finished, false); diff --git a/test/parallel/test-http2-compat-serverresponse-flushheaders.js b/test/parallel/test-http2-compat-serverresponse-flushheaders.js index f02732df87b050..c91fac16fb561c 100644 --- a/test/parallel/test-http2-compat-serverresponse-flushheaders.js +++ b/test/parallel/test-http2-compat-serverresponse-flushheaders.js @@ -16,18 +16,23 @@ server.listen(0, common.mustCall(function() { const port = server.address().port; server.once('request', common.mustCall(function(request, response) { assert.strictEqual(response.headersSent, false); + assert.strictEqual(response._header, false); // alias for headersSent response.flushHeaders(); assert.strictEqual(response.headersSent, true); + assert.strictEqual(response._header, true); response.flushHeaders(); // Idempotent common.expectsError(() => { response.writeHead(400, { 'foo-bar': 'abc123' }); }, { - code: 'ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND' + code: 'ERR_HTTP2_HEADERS_SENT' }); response.on('finish', common.mustCall(function() { server.close(); + process.nextTick(() => { + response.flushHeaders(); // Idempotent + }); })); serverResponse = response; })); diff --git a/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js b/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js new file mode 100644 index 00000000000000..e312cf1371b0ed --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js @@ -0,0 +1,49 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const h2 = require('http2'); + +// makes sure that Http2ServerResponse setHeader & removeHeader, do not throw +// any errors if the stream was destroyed before headers were sent + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + response.destroy(); + + response.on('finish', common.mustCall(() => { + assert.strictEqual(response.headersSent, false); + assert.doesNotThrow(() => response.setHeader('test', 'value')); + assert.doesNotThrow(() => response.removeHeader('test', 'value')); + + process.nextTick(() => { + assert.strictEqual(response.stream, undefined); + assert.doesNotThrow(() => response.setHeader('test', 'value')); + assert.doesNotThrow(() => response.removeHeader('test', 'value')); + + server.close(); + }); + })); + })); + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('end', common.mustCall(function() { + client.destroy(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-compat-serverresponse-headers.js b/test/parallel/test-http2-compat-serverresponse-headers.js index d753b3e6cc4230..a6f34ead1c0938 100644 --- a/test/parallel/test-http2-compat-serverresponse-headers.js +++ b/test/parallel/test-http2-compat-serverresponse-headers.js @@ -125,12 +125,44 @@ server.listen(0, common.mustCall(function() { response.sendDate = false; assert.strictEqual(response.sendDate, false); - assert.strictEqual(response.code, h2.constants.NGHTTP2_NO_ERROR); - response.on('finish', common.mustCall(function() { - assert.strictEqual(response.code, h2.constants.NGHTTP2_NO_ERROR); assert.strictEqual(response.headersSent, true); + + common.expectsError( + () => response.setHeader(real, expectedValue), + { + code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, + message: 'Response has already been initiated.' + } + ); + common.expectsError( + () => response.removeHeader(real, expectedValue), + { + code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, + message: 'Response has already been initiated.' + } + ); + process.nextTick(() => { + common.expectsError( + () => response.setHeader(real, expectedValue), + { + code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, + message: 'Response has already been initiated.' + } + ); + common.expectsError( + () => response.removeHeader(real, expectedValue), + { + code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, + message: 'Response has already been initiated.' + } + ); + // can access headersSent after stream is undefined assert.strictEqual(response.stream, undefined); assert.strictEqual(response.headersSent, true); diff --git a/test/parallel/test-http2-compat-serverresponse-socket-warn.js b/test/parallel/test-http2-compat-serverresponse-socket-warn.js new file mode 100644 index 00000000000000..3eb57868669c81 --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-socket-warn.js @@ -0,0 +1,47 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const h2 = require('http2'); + +// Http2ServerResponse.socket should warn one time when being accessed + +const unsupportedWarned = common.mustCall(1); +process.on('warning', ({ name, message }) => { + const expectedMessage = + 'Because the of the specific serialization and processing requirements ' + + 'imposed by the HTTP/2 protocol, it is not recommended for user code ' + + 'to read data from or write data to a Socket instance.'; + if (name === 'UnsupportedWarning' && message === expectedMessage) + unsupportedWarned(); +}); + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + response.socket; + response.socket; // should not warn + request.socket; // should not warn + response.end(); + })); + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('end', common.mustCall(function() { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-compat-serverresponse-writehead.js b/test/parallel/test-http2-compat-serverresponse-writehead.js index 12de2983468781..2e826132adbe54 100644 --- a/test/parallel/test-http2-compat-serverresponse-writehead.js +++ b/test/parallel/test-http2-compat-serverresponse-writehead.js @@ -17,7 +17,7 @@ server.listen(0, common.mustCall(function() { response.writeHead(418, { 'foo-bar': 'abc123' }); // Override common.expectsError(() => { response.writeHead(300); }, { - code: 'ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND' + code: 'ERR_HTTP2_HEADERS_SENT' }); response.on('finish', common.mustCall(function() {