From ed77c36ef6ac9679677c8e41de8a7b9b8ebea679 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Thu, 23 Mar 2017 21:24:28 +0800 Subject: [PATCH 1/4] http2: compatibility with http API & push --- doc/http2-implementation-notes.md | 18 +- lib/internal/http2/compat.js | 300 +++++++++++------- lib/internal/http2/core.js | 74 +++-- src/node_http2_core.cc | 21 +- .../parallel/test-http2-compat-pushrequest.js | 89 ++++++ ...test-http2-compat-serverrequest-headers.js | 61 ++++ .../test-http2-compat-serverrequest.js | 46 +++ ...ttp2-compat-serverresponse-flushheaders.js | 42 +++ ...est-http2-compat-serverresponse-headers.js | 85 +++++ ...-http2-compat-serverresponse-statuscode.js | 68 ++++ ...t-http2-compat-serverresponse-writehead.js | 45 +++ .../test-http2-create-client-connect.js | 2 +- test/parallel/test-http2-server-startup.js | 4 +- 13 files changed, 684 insertions(+), 171 deletions(-) create mode 100644 test/parallel/test-http2-compat-pushrequest.js create mode 100644 test/parallel/test-http2-compat-serverrequest-headers.js create mode 100644 test/parallel/test-http2-compat-serverrequest.js create mode 100644 test/parallel/test-http2-compat-serverresponse-flushheaders.js create mode 100644 test/parallel/test-http2-compat-serverresponse-headers.js create mode 100644 test/parallel/test-http2-compat-serverresponse-statuscode.js create mode 100644 test/parallel/test-http2-compat-serverresponse-writehead.js diff --git a/doc/http2-implementation-notes.md b/doc/http2-implementation-notes.md index 0cb29dd971..dae747d81f 100755 --- a/doc/http2-implementation-notes.md +++ b/doc/http2-implementation-notes.md @@ -50,18 +50,16 @@ const server = http2.createSecureServer(options, (req, res) => { res.writeHead(200, {'content-type': 'text/html'}); - const favicon = res.createPushResponse(); - favicon.path = '/favicon.ico'; - favicon.push((req, res) => { - res.setHeader('content-type', 'image/jpeg'); - fs.createReadStream('/some/image.jpg').pipe(res); + const favicon = res.createPushRequest({':path': '/favicon.ico'}); + favicon.push((err, res) => { + res.setHeader('content-type', 'image/png'); + fs.createReadStream('/some/logo.png').pipe(res); }); - const pushResponse = res.createPushResponse(); - pushResponse.path = '/image.jpg'; - pushResponse.push((req, res) => { + const pushResponse = res.createPushRequest({':path': '/image.jpg'}); + pushResponse.push((err, res) => { res.setHeader('content-type', 'image/jpeg'); - fs.createReadStream('/some/image/jpg').pipe(res); + fs.createReadStream('/some/image.jpg').pipe(res); }); res.end('' + @@ -299,7 +297,7 @@ The `'rst-stream'` event is emitted when a RST-STREAM frame is received. ### Method: `response.writeHeader(statusCode, headers)` ### Method: `response.write()` ### Method: `response.end()` -### Method: `response.createPushResponse()` +### Method: `response.createPushRequest(headers)` ## HTTP2.createServerSession(options) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index d42c97a518..756f1fb7e2 100755 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -1,6 +1,5 @@ 'use strict'; -const linkedList = require('internal/linkedlist'); const Stream = require('stream'); const Readable = Stream.Readable; const binding = process.binding('http2'); @@ -22,31 +21,13 @@ const kTrailers = Symbol('trailers'); // HTTP/2 implementation, intended to provide an interface that is as // close as possible to the current require('http') API -function setHeader(list, name, value) { - name = String(name).toLowerCase().trim(); +function assertValidHeader(name, value) { if (isPseudoHeader(name)) throw new Error('Cannot set HTTP/2 pseudo-headers'); if (isIllegalConnectionSpecificHeader(name, value)) throw new Error('Connection-specific HTTP/1 headers are not permitted'); if (value === undefined || value === null) throw new TypeError('Value must not be undefined or null'); - linkedList.append(list, [name, String(value)]); -} - -function llistToHeaders(list, count) { - var ret = {}; - while (!linkedList.isEmpty(list)) { - var item = list._idlePrev; - linkedList.remove(item); - var key = item[0]; - - if (ret[key]) { - ret[key].push(item[1]); - } else { - ret[key] = [item[1]]; - } - } - return ret; } function isPseudoHeader(name) { @@ -127,12 +108,14 @@ function onAborted(hadError, code) { } class Http2ServerRequest extends Readable { - constructor(stream, options) { + constructor(stream, headers, options) { super(options); this[kState] = { + statusCode: null, closed: false, closedCode: constants.NGHTTP2_NO_ERROR }; + this[kHeaders] = headers; this[kStream] = stream; stream[kRequest] = this; @@ -143,6 +126,9 @@ class Http2ServerRequest extends Readable { stream.on('error', onStreamError); stream.on('close', onStreamClosedRequest); stream.on('aborted', onAborted.bind(this)); + const onfinish = this[kFinish].bind(this); + stream.on('streamClosed', onfinish); + stream.on('finish', onfinish); this.on('pause', onRequestPause); this.on('resume', onRequestResume); this.on('drain', onRequestDrain); @@ -166,48 +152,33 @@ class Http2ServerRequest extends Readable { return this[kStream]; } - get headers() { - var stream = this[kStream]; - return stream ? stream[kHeaders] : undefined; + get statusCode() { + return this[kState].statusCode; } - getHeader(name) { - var stream = this[kStream]; - if (!stream) - return; - var headers = stream[kHeaders]; - name = String(name).trim().toLowerCase(); - return headers.get(name); + get headers() { + return this[kHeaders]; } - hasHeader(name) { - var stream = this[kStream]; - if (!stream) - return false; - var headers = stream[kHeaders]; - name = String(name).trim().toLowerCase(); - return headers.has(name); + get rawHeaders() { + var headers = this[kHeaders]; + if (headers === undefined) + return []; + var tuples = Object.entries(headers); + var flattened = Array.prototype.concat.apply([], tuples); + return flattened.map(String); } - getTrailer(name) { - var trailers = this[kTrailers]; - if (!trailers) - return; - name = String(name).trim().toLowerCase(); - return trailers.get(name); + get trailers() { + return this[kTrailers]; } - hasTrailer(name) { - var trailers = this[kTrailers]; - if (!trailers) - return false; - name = String(name).trim().toLowerCase(); - return trailers.has(name); + get httpVersionMajor() { + return 2; } - get trailers() { - var stream = this[kStream]; - return stream ? stream[kTrailers] : undefined; + get httpVersionMinor() { + return 0; } get httpVersion() { @@ -224,43 +195,48 @@ class Http2ServerRequest extends Readable { } get method() { - var stream = this[kStream]; - if (!stream) return; - var headers = stream[kHeaders]; - return headers.get(constants.HTTP2_HEADER_METHOD); + var headers = this[kHeaders]; + if (headers === undefined) + return; + return headers[constants.HTTP2_HEADER_METHOD]; } get authority() { - var stream = this[kStream]; - if (!stream) return; - var headers = stream[kHeaders]; - return headers.get(constants.HTTP2_HEADER_AUTHORITY); + var headers = this[kHeaders]; + if (headers === undefined) + return; + return headers[constants.HTTP2_HEADER_AUTHORITY]; } get scheme() { - var stream = this[kStream]; - if (!stream) return; - var headers = stream[kHeaders]; - return headers.get(constants.HTTP2_HEADER_SCHEME); + var headers = this[kHeaders]; + if (headers === undefined) + return; + return headers[constants.HTTP2_HEADER_SCHEME]; + } + + get url() { + return this.path; } get path() { - var stream = this[kStream]; - if (!stream) return; - var headers = stream[kHeaders]; - return headers.get(constants.HTTP2_HEADER_PATH); + var headers = this[kHeaders]; + if (headers === undefined) + return; + return headers[constants.HTTP2_HEADER_PATH]; } setTimeout(msecs, callback) { var stream = this[kStream]; - if (!stream) return; + if (stream === undefined) return; stream.setTimeout(msecs, callback); - return this; } [kFinish](code) { var state = this[kState]; - state.closeCode = code; + if (state.closed) + return; + state.closedCode = code; state.closed = true; this.push(null); this[kStream] = undefined; @@ -273,6 +249,7 @@ class Http2ServerResponse extends Stream { this[kState] = { sendDate: true, statusCode: constants.HTTP_STATUS_OK, + headersSent: false, headerCount: 0, trailerCount: 0, closed: false, @@ -280,23 +257,13 @@ class Http2ServerResponse extends Stream { }; this[kStream] = stream; stream[kResponse] = this; - var headerList = { - _idleNext: null, - _idlePrev: null, - }; - linkedList.init(headerList); - this[kHeaders] = headerList; - var trailersList = { - _idleNext: null, - _idlePrev: null, - }; - linkedList.init(trailersList); - this[kHeaders] = trailersList; this.writable = true; stream.on('drain', onStreamResponseDrain); stream.on('error', onStreamResponseError); stream.on('close', onStreamClosedResponse); stream.on('aborted', onAborted.bind(this)); + stream.on('streamClosed', this[kFinish].bind(this)); + stream.on('finish', this[kFinish].bind(this)); } get finished() { @@ -336,7 +303,7 @@ class Http2ServerResponse extends Stream { set statusCode(code) { var state = this[kState]; - if (state.headersSent) + if (state.headersSent === true) throw new Error('Cannot set status after the HTTP message has been sent'); code |= 0; if (code >= 100 && code < 200) @@ -346,45 +313,94 @@ class Http2ServerResponse extends Stream { state.statusCode = code; } + addTrailers(headers) { + for (var name of Object.keys(headers)) { + name = String(name).trim().toLowerCase(); + var value = headers[name]; + assertValidHeader(name, value); + var trailers = this[kTrailers]; + if (trailers === undefined) + trailers = this[kTrailers] = Object.create(null); + trailers[name] = String(value); + } + } + + getHeader(name) { + var headers = this[kHeaders]; + if (headers === undefined) + return; + name = String(name).trim().toLowerCase(); + return headers[name]; + } + + getHeaderNames() { + var headers = this[kHeaders]; + if (headers === undefined) + return []; + return Object.keys(headers); + } + + getHeaders() { + var headers = this[kHeaders]; + return Object.assign({}, headers); + } + + hasHeader(name) { + var headers = this[kHeaders]; + if (headers === undefined) + return false; + name = String(name).trim().toLowerCase(); + return Object.prototype.hasOwnProperty.call(headers, name); + } + + removeHeader(name) { + var headers = this[kHeaders]; + if (headers === undefined) + return; + name = String(name).trim().toLowerCase(); + delete headers[name]; + } + setHeader(name, value) { var state = this[kState]; + if (state.headersSent === true) + throw new Error('Cannot set headers after ' + + 'the HTTP message has been sent'); + name = String(name).trim().toLowerCase(); + assertValidHeader(name, value); var headers = this[kHeaders]; - if (state.headersSent) { - throw new Error( - 'Cannot set headers after the HTTP message has been sent'); - } - setHeader(headers, name, value); - state.headerCount++; - return this; + if (headers === undefined) + headers = this[kHeaders] = Object.create(null); + headers[name] = String(value); } - setTrailer(name, value) { + flushHeaders() { var state = this[kState]; - var trailers = this[kTrailers]; - if (state.trailersSent) { - throw new Error( - 'Cannot set trailers after the HTTP message has been sent'); - } - setHeader(trailers, name, value); - state.trailerCount++; - return this; + if (state.headersSent === true) + return; + var statusCode = this[kState].statusCode; + this.writeHead(statusCode); } writeHead(statusCode, headers) { - var keys = Object.keys(headers); - var key = ''; - for (var i = 0; i < keys.length; i++) { - key = keys[i]; - this.setHeader(key, headers[key]); + var state = this[kState]; + if (state.headersSent === true) + return; + if (headers) { + var keys = Object.keys(headers); + var key = ''; + for (var i = 0; i < keys.length; i++) { + key = keys[i]; + this.setHeader(key, headers[key]); + } } this.statusCode = statusCode; this[kBeginSend](); - return this; } write(chunk, encoding, cb) { var stream = this[kStream]; - if (!stream) { + if (stream === undefined) { cb(new Error('HTTP/2 Stream has been closed')); return; } @@ -395,13 +411,14 @@ class Http2ServerResponse extends Stream { end(chunk, encoding, cb) { var stream = this[kStream]; - this.write(chunk, encoding, cb); + if (chunk) + this.write(chunk, encoding, cb); stream.end(); } destroy(err) { var stream = this[kStream]; - if (!stream) { + if (stream === undefined) { // nothing to do, already closed return; } @@ -410,9 +427,8 @@ class Http2ServerResponse extends Stream { setTimeout(msecs, callback) { var stream = this[kStream]; - if (!stream) return; + if (stream === undefined) return; stream.setTimeout(msecs, callback); - return this; } sendContinue(headers) { @@ -421,7 +437,7 @@ class Http2ServerResponse extends Stream { sendInfo(code, headers) { var state = this[kState]; - if (state.headersSent) { + if (state.headersSent === true) { throw new Error( 'Cannot send informational headers after the HTTP message' + 'has been sent'); @@ -429,7 +445,7 @@ class Http2ServerResponse extends Stream { if (headers && typeof headers !== 'object') throw new TypeError('headers must be an object'); var stream = this[kStream]; - if (!stream) return; + if (stream === undefined) return; code |= 0; if (code < 100 || code >= 200) throw new RangeError(`Invalid informational status code: ${code}`); @@ -439,12 +455,21 @@ class Http2ServerResponse extends Stream { stream.respond(headers); } + createPushRequest(headers) { + var stream = this[kStream]; + if (stream === undefined) { + throw new Error('HTTP/2 Stream has been closed'); + } + var request = new Http2PushRequest(stream, headers); + return request; + } + [kBeginSend]() { var state = this[kState]; var stream = this[kStream]; - if (!state.headersSent) { + if (state.headersSent === false) { state.headersSent = true; - const headers = llistToHeaders(this[kHeaders]); + const headers = this[kHeaders] || Object.create(null); headers[constants.HTTP2_HEADER_STATUS] = state.statusCode; stream.respond(headers); } @@ -452,7 +477,9 @@ class Http2ServerResponse extends Stream { [kFinish](code) { var state = this[kState]; - state.closeCode = code; + if (state.closed) + return; + state.closedCode = code; state.closed = true; this.end(); this[kStream] = undefined; @@ -461,10 +488,8 @@ class Http2ServerResponse extends Stream { function onServerStream(stream, headers, flags) { var server = this; - var request = - new Http2ServerRequest(stream); - var response = - new Http2ServerResponse(stream); + var request = new Http2ServerRequest(stream, headers); + var response = new Http2ServerResponse(stream); // Check for the CONNECT method var method = headers[constants.HTTP2_HEADER_METHOD]; @@ -497,4 +522,37 @@ function onServerStream(stream, headers, flags) { server.emit('request', request, response); } +class Http2PushRequest { + constructor(stream, headers) { + this[kStream] = stream; + this[kHeaders] = headers || Object.create(null); + this[kState] = { + headersSent: false + }; + } + + push(callback) { + var state = this[kState]; + if (state.headersSent === true) + throw new Error('Cannot push request headers ' + + 'after the HTTP message has been sent'); + var stream = this[kStream]; + if (stream === undefined) + throw new Error('HTTP/2 Stream has been closed'); + var headers = this[kHeaders]; + try { + stream.pushStream( + headers, + function(promisedStream, headers, options) { + var response = new Http2ServerResponse(promisedStream); + callback(null, response); + } + ); + state.headersSent = true; + } catch (error) { + callback(error); + } + } +} + module.exports = { onServerStream }; diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7693730ded..fd24b724e0 100755 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -49,6 +49,7 @@ const NGHTTP2_DEFAULT_WEIGHT = constants.NGHTTP2_DEFAULT_WEIGHT; const NGHTTP2_FLAG_END_STREAM = constants.NGHTTP2_FLAG_END_STREAM; const NGHTTP2_HCAT_HEADERS = constants.NGHTTP2_HCAT_HEADERS; const NGHTTP2_HCAT_PUSH_RESPONSE = constants.NGHTTP2_HCAT_PUSH_RESPONSE; +const NGHTTP2_HCAT_REQUEST = constants.NGHTTP2_HCAT_REQUEST; const NGHTTP2_HCAT_RESPONSE = constants.NGHTTP2_HCAT_RESPONSE; const NGHTTP2_INTERNAL_ERROR = constants.NGHTTP2_INTERNAL_ERROR; const NGHTTP2_NO_ERROR = constants.NGHTTP2_NO_ERROR; @@ -98,37 +99,41 @@ function onSessionHeaders(id, cat, flags, headers) { const eos = Boolean(flags & NGHTTP2_FLAG_END_STREAM); - if (stream === undefined && owner.type === NGHTTP2_SESSION_SERVER) { - stream = new ServerHttp2Stream(owner, id, { readable: !eos }); + if (stream === undefined) { + switch (owner.type) { + case NGHTTP2_SESSION_SERVER: + stream = new ServerHttp2Stream(owner, id, { readable: !eos }); + break; + case NGHTTP2_SESSION_CLIENT: + stream = new ClientHttp2Stream(owner, id, { readable: !eos }); + break; + default: + assert.fail(null, null, + 'Internal HTTP/2 Error. Invalid session type.'); + } streams.set(id, stream); owner.emit('stream', stream, headers, flags); - return; - } - - if (cat === NGHTTP2_HCAT_PUSH_RESPONSE) { - if (stream === undefined) { - stream = new ClientHttp2Stream(owner, { }); - streams.set(id, stream); + } else { + let event; + switch (cat) { + case NGHTTP2_HCAT_REQUEST: + event = 'request'; + break; + case NGHTTP2_HCAT_RESPONSE: + event = 'response'; + break; + case NGHTTP2_HCAT_PUSH_RESPONSE: + event = 'push'; + break; + case NGHTTP2_HCAT_HEADERS: + event = eos ? 'trailers' : 'headers'; + break; + default: + assert.fail(null, null, + 'Internal HTTP/2 Error. Invalid headers category.'); } - owner.emit('push', stream, headers, flags); - } - - let event; - switch (cat) { - case NGHTTP2_HCAT_RESPONSE: - event = 'response'; - break; - case NGHTTP2_HCAT_PUSH_RESPONSE: - event = 'push'; - break; - case NGHTTP2_HCAT_HEADERS: - event = eos ? 'trailers' : 'headers'; - break; - default: - assert.fail(null, null, - 'Internal HTTP/2 Error. Invalid headers category.'); + stream.emit(event, headers, flags); } - stream.emit(event, headers, flags); } // Called to determine if there are trailers to be sent at the end of a @@ -258,7 +263,7 @@ function request(headers, options) { if (options.endStream === undefined) options.endStream = false; - const stream = new ClientHttp2Stream(this, {}); + const stream = new ClientHttp2Stream(this, undefined, {}); const onConnect = requestOnConnect.bind(stream, headers, options); const state = this[kState]; @@ -529,6 +534,8 @@ function afterDoStreamWrite(status, handle, req) { function onHandleFinish() { const session = this[kSession]; + if (!session) + return; if (this.id === undefined) { this.on('connect', () => { const handle = session[kHandle]; @@ -809,7 +816,7 @@ class Http2Stream extends Duplex { if (statusCode === HTTP_STATUS_SWITCHING_PROTOCOLS) throw new RangeError( 'HTTP status code 101 (Switching Protocols) is forbidden in HTTP/2'); - if (statusCode < 100 || statusCode >= 200) + if (statusCode < 100 || statusCode > 999) throw new RangeError('Invalid informational status code'); } @@ -920,9 +927,11 @@ class ServerHttp2Stream extends Http2Stream { ServerHttp2Stream.prototype[kProceed] = ServerHttp2Stream.prototype.respond; class ClientHttp2Stream extends Http2Stream { - constructor(session, options) { + constructor(session, id, options) { super(session, options); this[kState].headersSent = true; + if (id !== undefined) + this[kInit](id); } } @@ -1222,8 +1231,9 @@ function createClientSession(options, socket) { socket.on('drain', socketOnDrain); session.on('error', clientSessionOnError); - session.on('stream', sessionOnStream); - session.on('selectPadding', sessionOnSelectPadding); + // TODO(jasnell): provide client specific implementations + // session.on('stream', sessionOnStream); + // session.on('selectPadding', sessionOnSelectPadding); socket[kSession] = session; diff --git a/src/node_http2_core.cc b/src/node_http2_core.cc index adbe09388d..ebf0449069 100644 --- a/src/node_http2_core.cc +++ b/src/node_http2_core.cc @@ -14,11 +14,14 @@ int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session, const nghttp2_frame* frame, void* user_data) { Nghttp2Session* handle = static_cast(user_data); - if (!handle->HasStream(frame->hd.stream_id)) { - handle->StreamInit(frame->hd.stream_id, frame->headers.cat); + int32_t stream_id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ? + frame->push_promise.promised_stream_id : + frame->hd.stream_id; + if (!handle->HasStream(stream_id)) { + handle->StreamInit(stream_id, frame->headers.cat); } else { std::shared_ptr stream_handle; - stream_handle = handle->FindStream(frame->hd.stream_id); + stream_handle = handle->FindStream(stream_id); assert(stream_handle); stream_handle->current_headers_category_ = frame->headers.cat; } @@ -33,7 +36,10 @@ int Nghttp2Session::OnHeaderCallback(nghttp2_session* session, void* user_data) { Nghttp2Session* handle = static_cast(user_data); std::shared_ptr stream_handle; - stream_handle = handle->FindStream(frame->hd.stream_id); + int32_t stream_id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ? + frame->push_promise.promised_stream_id : + frame->hd.stream_id; + stream_handle = handle->FindStream(stream_id); assert(stream_handle); nghttp2_header_list* header = header_free_list.pop(); @@ -52,6 +58,7 @@ int Nghttp2Session::OnFrameReceive(nghttp2_session* session, std::shared_ptr stream_handle; nghttp2_pending_cb_list* pending_cb; nghttp2_pending_headers_cb* cb; + int32_t stream_id; switch (frame->hd.type) { case NGHTTP2_DATA: stream_handle = handle->FindStream(frame->hd.stream_id); @@ -62,8 +69,12 @@ int Nghttp2Session::OnFrameReceive(nghttp2_session* session, handle->QueuePendingDataChunks(stream_handle.get(), frame->hd.flags); } break; + case NGHTTP2_PUSH_PROMISE: case NGHTTP2_HEADERS: - stream_handle = handle->FindStream(frame->hd.stream_id); + stream_id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ? + frame->push_promise.promised_stream_id : + frame->hd.stream_id; + stream_handle = handle->FindStream(stream_id); assert(stream_handle); cb = pending_headers_free_list.pop(); cb->handle = stream_handle; diff --git a/test/parallel/test-http2-compat-pushrequest.js b/test/parallel/test-http2-compat-pushrequest.js new file mode 100644 index 0000000000..6849433b4f --- /dev/null +++ b/test/parallel/test-http2-compat-pushrequest.js @@ -0,0 +1,89 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const h2 = require('http2'); + +// Push a request & response + +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.stream.id % 2 === 1); + + response.write('This is a client-initiated response'); + response.stream.on('finish', common.mustCall(function() { + server.close(); + })); + + const headers = { + ':path': '/pushed', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + + const pushRequest = response.createPushRequest(headers); + + pushRequest.push(common.mustCall(function(error, pushResponse) { + assert.strictEqual(error, null); + assert.ok(pushResponse.stream.id % 2 === 0); + + pushResponse.write('This is a server-initiated response'); + + // TODO(sebdeckers) Remove this forced delay workaround. + // pushResponse.end(); + // response.end(); + setTimeout(() => pushResponse.end(), 100); + setTimeout(() => response.end(), 200); + })); + })); + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + + const requestStream = client.request(headers); + + function onStream(pushStream, headers, flags) { + assert.strictEqual(headers[':path'], '/pushed'); + assert.strictEqual(headers[':method'], 'GET'); + assert.strictEqual(headers[':scheme'], 'http'); + assert.strictEqual(headers[':authority'], `localhost:${port}`); + assert.strictEqual(flags, h2.constants.NGHTTP2_FLAG_END_HEADERS); + + pushStream.on('data', common.mustCall(function(data) { + assert.strictEqual( + data.toString(), + 'This is a server-initiated response' + ); + })); + } + + requestStream.session.on('stream', common.mustCall(onStream)); + + requestStream.on('response', common.mustCall(function(headers, flags) { + assert.strictEqual(headers[':status'], '200'); + assert.ok(headers['date']); + assert.strictEqual(flags, h2.constants.NGHTTP2_FLAG_END_HEADERS); + })); + + requestStream.on('data', common.mustCall(function(data) { + assert.strictEqual( + data.toString(), + 'This is a client-initiated response' + ); + })); + + requestStream.on('end', common.mustCall(function() { + client.destroy(); + })); + requestStream.end(); + })); +})); diff --git a/test/parallel/test-http2-compat-serverrequest-headers.js b/test/parallel/test-http2-compat-serverrequest-headers.js new file mode 100644 index 0000000000..6897b039d3 --- /dev/null +++ b/test/parallel/test-http2-compat-serverrequest-headers.js @@ -0,0 +1,61 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const h2 = require('http2'); + +// Http2ServerRequest should have header helpers + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + const expected = { + ':path': '/foobar', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}`, + 'foo-bar': 'abc123' + }; + + 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']); + + const headers = request.headers; + for (const [name, value] of Object.entries(expected)) { + assert.strictEqual(headers[name], value); + } + + const rawHeaders = request.rawHeaders; + for (const [name, value] of Object.entries(expected)) { + const position = rawHeaders.indexOf(name); + assert.notStrictEqual(position, -1); + assert.strictEqual(rawHeaders[position + 1], value); + } + + response.stream.on('finish', common.mustCall(function() { + server.close(); + })); + response.end(' '); + })); + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/foobar', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}`, + 'foo-bar': 'abc123' + }; + 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-serverrequest.js b/test/parallel/test-http2-compat-serverrequest.js new file mode 100644 index 0000000000..cd93c1c596 --- /dev/null +++ b/test/parallel/test-http2-compat-serverrequest.js @@ -0,0 +1,46 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const h2 = require('http2'); + +// Http2ServerRequest should expose convenience properties + +const server = h2.createServer(); +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 + }; + + assert.strictEqual(request.statusCode, expected.statusCode); + assert.strictEqual(request.httpVersion, expected.version); + assert.strictEqual(request.httpVersionMajor, expected.httpVersionMajor); + assert.strictEqual(request.httpVersionMinor, expected.httpVersionMinor); + + response.stream.on('finish', common.mustCall(function() { + server.close(); + })); + response.end(' '); + })); + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/foobar', + ':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-flushheaders.js b/test/parallel/test-http2-compat-serverresponse-flushheaders.js new file mode 100644 index 0000000000..236f15b63c --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-flushheaders.js @@ -0,0 +1,42 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const h2 = require('http2'); + +// Http2ServerResponse.flushHeaders + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + response.flushHeaders(); + response.flushHeaders(); // Idempotent + response.writeHead(200, {'foo-bar': 'abc123'}); // Ignored + + response.stream.on('finish', common.mustCall(function() { + server.close(); + })); + 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('response', common.mustCall(function(headers, flags) { + assert.strictEqual(headers['foo-bar'], undefined); + assert.strictEqual(headers[':status'], '200'); + }, 1)); + 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 new file mode 100644 index 0000000000..bdf92f75b9 --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-headers.js @@ -0,0 +1,85 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const h2 = require('http2'); + +// Http2ServerResponse should support checking and reading custom headers + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + const real = 'foo-bar'; + const fake = 'bar-foo'; + const denormalised = ` ${real.toUpperCase()}\n\t`; + const expectedValue = 'abc123'; + + response.setHeader(real, expectedValue); + + assert.strictEqual(response.hasHeader(real), true); + assert.strictEqual(response.hasHeader(fake), false); + assert.strictEqual(response.hasHeader(denormalised), true); + assert.strictEqual(response.getHeader(real), expectedValue); + assert.strictEqual(response.getHeader(denormalised), expectedValue); + assert.strictEqual(response.getHeader(fake), undefined); + + response.removeHeader(fake); + assert.strictEqual(response.hasHeader(fake), false); + + response.setHeader(real, expectedValue); + assert.strictEqual(response.getHeader(real), expectedValue); + assert.strictEqual(response.hasHeader(real), true); + response.removeHeader(real); + assert.strictEqual(response.hasHeader(real), false); + + response.setHeader(denormalised, expectedValue); + assert.strictEqual(response.getHeader(denormalised), expectedValue); + assert.strictEqual(response.hasHeader(denormalised), true); + response.removeHeader(denormalised); + assert.strictEqual(response.hasHeader(denormalised), false); + + assert.throws(function() { + response.setHeader(':status', 'foobar'); + }, Error); + assert.throws(function() { + response.setHeader('connection', 'foobar'); + }, Error); + assert.throws(function() { + response.setHeader(real, null); + }, TypeError); + assert.throws(function() { + response.setHeader(real, undefined); + }, TypeError); + + response.setHeader(real, expectedValue); + const expectedHeaderNames = [real]; + assert.deepStrictEqual(response.getHeaderNames(), expectedHeaderNames); + const expectedHeaders = {[real]: expectedValue}; + assert.deepStrictEqual(response.getHeaders(), expectedHeaders); + + response.getHeaders()[fake] = fake; + assert.strictEqual(response.hasHeader(fake), false); + + response.stream.on('finish', common.mustCall(function() { + server.close(); + })); + 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(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-compat-serverresponse-statuscode.js b/test/parallel/test-http2-compat-serverresponse-statuscode.js new file mode 100644 index 0000000000..c3f72a293d --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-statuscode.js @@ -0,0 +1,68 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const h2 = require('http2'); + +// Http2ServerResponse should have a statusCode property + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + const expectedDefaultStatusCode = 200; + const realStatusCodes = { + continue: 100, + ok: 200, + multipleChoices: 300, + badRequest: 400, + internalServerError: 500 + }; + const fakeStatusCodes = { + tooLow: 99, + tooHigh: 1000, + backwardsCompatibility: 999 + }; + + assert.strictEqual(response.statusCode, expectedDefaultStatusCode); + + assert.doesNotThrow(function() { + response.statusCode = realStatusCodes.ok; + response.statusCode = realStatusCodes.multipleChoices; + response.statusCode = realStatusCodes.badRequest; + response.statusCode = realStatusCodes.internalServerError; + response.statusCode = fakeStatusCodes.backwardsCompatibility; + }); + + assert.throws(function() { + response.statusCode = realStatusCodes.continue; + }, RangeError); + assert.throws(function() { + response.statusCode = fakeStatusCodes.tooLow; + }, RangeError); + assert.throws(function() { + response.statusCode = fakeStatusCodes.tooHigh; + }, RangeError); + + response.stream.on('finish', common.mustCall(function() { + server.close(); + })); + 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(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-compat-serverresponse-writehead.js b/test/parallel/test-http2-compat-serverresponse-writehead.js new file mode 100644 index 0000000000..1d97cd6653 --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-writehead.js @@ -0,0 +1,45 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const h2 = require('http2'); + +// Http2ServerResponse.writeHead should override previous headers + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + const statusCode = 404; + const headers = {'foo-bar': 'abc123'}; + + response.setHeader('foo-bar', 'def456'); + response.writeHead(statusCode, headers); + response.writeHead(statusCode, headers); // Idempotent + + response.stream.on('finish', common.mustCall(function() { + server.close(); + })); + 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('response', common.mustCall(function(headers) { + assert.strictEqual(headers['foo-bar'], 'abc123'); + assert.strictEqual(headers[':status'], '404'); + }, 1)); + request.on('end', common.mustCall(function() { + client.destroy(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-create-client-connect.js b/test/parallel/test-http2-create-client-connect.js index 5a548dcb43..bb0f83afc7 100755 --- a/test/parallel/test-http2-create-client-connect.js +++ b/test/parallel/test-http2-create-client-connect.js @@ -76,7 +76,7 @@ const URL = url.URL; let count = items.length; const maybeClose = common.mustCall((client) => { - client.socket.destroy(); + client.destroy(); if (--count === 0) { setImmediate(() => server.close()); } diff --git a/test/parallel/test-http2-server-startup.js b/test/parallel/test-http2-server-startup.js index d811ba5bc5..8673c0fdd4 100755 --- a/test/parallel/test-http2-server-startup.js +++ b/test/parallel/test-http2-server-startup.js @@ -56,7 +56,7 @@ assert.doesNotThrow(() => { const port = server.address().port; client = net.connect(port, common.mustCall()); })); - timer = setTimeout(() => common.fail('server timeout failed'), + timer = setTimeout(() => assert.fail('server timeout failed'), common.platformTimeout(1100)); } @@ -77,6 +77,6 @@ assert.doesNotThrow(() => { client = tls.connect({port: port, rejectUnauthorized: false}, common.mustCall()); })); - timer = setTimeout(() => common.fail('server timeout failed'), + timer = setTimeout(() => assert.fail('server timeout failed'), common.platformTimeout(1100)); } From 5b709dc23f1a52aad938a74289d313f14e96aaa8 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Wed, 10 May 2017 09:38:10 +0800 Subject: [PATCH 2/4] Simplified push (Squash) --- lib/internal/http2/compat.js | 59 ++++++------------- ...mpat-serverresponse-createpushresponse.js} | 33 ++++++----- 2 files changed, 37 insertions(+), 55 deletions(-) rename test/parallel/{test-http2-compat-pushrequest.js => test-http2-compat-serverresponse-createpushresponse.js} (76%) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 756f1fb7e2..8184f0adf0 100755 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -314,13 +314,17 @@ class Http2ServerResponse extends Stream { } addTrailers(headers) { - for (var name of Object.keys(headers)) { - name = String(name).trim().toLowerCase(); + var trailers = this[kTrailers]; + var keys = Object.keys(headers); + var key = ''; + if (keys.length > 0) + return; + if (trailers === undefined) + trailers = this[kTrailers] = Object.create(null); + for (var i = 0; i < keys.length; i++) { + key = String(keys[i]).trim().toLowerCase(); var value = headers[name]; assertValidHeader(name, value); - var trailers = this[kTrailers]; - if (trailers === undefined) - trailers = this[kTrailers] = Object.create(null); trailers[name] = String(value); } } @@ -455,13 +459,19 @@ class Http2ServerResponse extends Stream { stream.respond(headers); } - createPushRequest(headers) { + createPushResponse(headers, callback) { var stream = this[kStream]; if (stream === undefined) { throw new Error('HTTP/2 Stream has been closed'); } - var request = new Http2PushRequest(stream, headers); - return request; + try { + stream.pushStream(headers, {}, function(stream, headers, options) { + var response = new Http2ServerResponse(stream); + callback(null, response); + }); + } catch (error) { + callback(error); + } } [kBeginSend]() { @@ -522,37 +532,4 @@ function onServerStream(stream, headers, flags) { server.emit('request', request, response); } -class Http2PushRequest { - constructor(stream, headers) { - this[kStream] = stream; - this[kHeaders] = headers || Object.create(null); - this[kState] = { - headersSent: false - }; - } - - push(callback) { - var state = this[kState]; - if (state.headersSent === true) - throw new Error('Cannot push request headers ' + - 'after the HTTP message has been sent'); - var stream = this[kStream]; - if (stream === undefined) - throw new Error('HTTP/2 Stream has been closed'); - var headers = this[kHeaders]; - try { - stream.pushStream( - headers, - function(promisedStream, headers, options) { - var response = new Http2ServerResponse(promisedStream); - callback(null, response); - } - ); - state.headersSent = true; - } catch (error) { - callback(error); - } - } -} - module.exports = { onServerStream }; diff --git a/test/parallel/test-http2-compat-pushrequest.js b/test/parallel/test-http2-compat-serverresponse-createpushresponse.js similarity index 76% rename from test/parallel/test-http2-compat-pushrequest.js rename to test/parallel/test-http2-compat-serverresponse-createpushresponse.js index 6849433b4f..97cf0b1fa4 100644 --- a/test/parallel/test-http2-compat-pushrequest.js +++ b/test/parallel/test-http2-compat-serverresponse-createpushresponse.js @@ -24,20 +24,25 @@ server.listen(0, common.mustCall(function() { ':authority': `localhost:${port}` }; - const pushRequest = response.createPushRequest(headers); - - pushRequest.push(common.mustCall(function(error, pushResponse) { - assert.strictEqual(error, null); - assert.ok(pushResponse.stream.id % 2 === 0); - - pushResponse.write('This is a server-initiated response'); - - // TODO(sebdeckers) Remove this forced delay workaround. - // pushResponse.end(); - // response.end(); - setTimeout(() => pushResponse.end(), 100); - setTimeout(() => response.end(), 200); - })); + response.createPushResponse( + headers, + common.mustCall(function(error, pushResponse) { + assert.strictEqual(error, null); + assert.ok(pushResponse.stream.id % 2 === 0); + + pushResponse.write('This is a server-initiated response'); + + // TODO(sebdeckers) Remove this forced delay workaround. + // See possibly related bugs: + // - https://github.com/nodejs/http2/issues/72 + // - https://github.com/nodejs/http2/issues/77 + + // pushResponse.end(); + // response.end(); + setTimeout(() => pushResponse.end(), 100); + setTimeout(() => response.end(), 200); + }) + ); })); const url = `http://localhost:${port}`; From 5385a0a44670e08e623e9b8808e19627f385364e Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Thu, 11 May 2017 08:55:40 +0800 Subject: [PATCH 3/4] Throw like no one is catching (Squash) --- lib/internal/http2/compat.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 8184f0adf0..d15467844a 100755 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -464,14 +464,10 @@ class Http2ServerResponse extends Stream { if (stream === undefined) { throw new Error('HTTP/2 Stream has been closed'); } - try { - stream.pushStream(headers, {}, function(stream, headers, options) { - var response = new Http2ServerResponse(stream); - callback(null, response); - }); - } catch (error) { - callback(error); - } + stream.pushStream(headers, {}, function(stream, headers, options) { + var response = new Http2ServerResponse(stream); + callback(null, response); + }); } [kBeginSend]() { From 6765d00472076553c5f7fbd5fa7c437bc74f6eff Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Fri, 12 May 2017 11:16:04 +0800 Subject: [PATCH 4/4] Separate stream & push events (Squash) --- test/parallel/test-http2-server-push-stream.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-http2-server-push-stream.js b/test/parallel/test-http2-server-push-stream.js index ea6e1e6146..fe4c2ceb5c 100644 --- a/test/parallel/test-http2-server-push-stream.js +++ b/test/parallel/test-http2-server-push-stream.js @@ -34,10 +34,15 @@ server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${port}`); const req = client.request(headers); - client.on('push', common.mustCall((stream, headers, flag) => { - assert.strictEqual(headers[':status'], '200'); - assert.strictEqual(headers['content-type'], 'text/html'); - assert.strictEqual(headers['x-push-data'], 'pushed by server'); + client.on('stream', common.mustCall((stream, headers, flags) => { + assert.strictEqual(headers[':scheme'], 'http'); + assert.strictEqual(headers[':path'], '/foobar'); + assert.strictEqual(headers[':authority'], `localhost:${port}`); + stream.on('push', common.mustCall((headers, flags) => { + assert.strictEqual(headers[':status'], '200'); + assert.strictEqual(headers['content-type'], 'text/html'); + assert.strictEqual(headers['x-push-data'], 'pushed by server'); + })); })); let data = '';