From e17cfabeff6c9180337d715255861cae57e210e5 Mon Sep 17 00:00:00 2001 From: Peter Marton Date: Thu, 19 Oct 2017 20:16:02 +0200 Subject: [PATCH 1/2] http: add IncomingMessage and ServerResponse options to Server --- doc/api/http.md | 9 +++- doc/api/https.md | 4 +- lib/_http_common.js | 10 +++- lib/_http_server.js | 23 +++++++-- lib/https.js | 6 +++ ...st-http-server-options-incoming-message.js | 41 +++++++++++++++ ...est-http-server-options-server-response.js | 35 +++++++++++++ ...t-https-server-options-incoming-message.js | 51 +++++++++++++++++++ ...st-https-server-options-server-response.js | 47 +++++++++++++++++ 9 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-http-server-options-incoming-message.js create mode 100644 test/parallel/test-http-server-options-server-response.js create mode 100644 test/parallel/test-https-server-options-incoming-message.js create mode 100644 test/parallel/test-https-server-options-server-response.js diff --git a/doc/api/http.md b/doc/api/http.md index 02c78550e0d703..3884ac4dcb06a3 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1661,10 +1661,17 @@ A collection of all the standard HTTP response status codes, and the short description of each. For example, `http.STATUS_CODES[404] === 'Not Found'`. -## http.createServer([requestListener]) +## http.createServer([options][, requestListener]) +- `options` {Object} + * `IncomingMessage` {http.IncomingMessage} Specifies the IncomingMessage class to + be used. Useful for extending the original `IncomingMessage`. + Defaults to: `IncomingMessage` + * `ServerResponse` {http.ServerResponse} Specifies the ServerResponse class to + be used. Useful for extending the original `ServerResponse`. + Defaults to: `ServerResponse` - `requestListener` {Function} * Returns: {http.Server} diff --git a/doc/api/https.md b/doc/api/https.md index cb22873a01e1ba..29f5d35ab87233 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -65,7 +65,8 @@ See [`http.Server#keepAliveTimeout`][]. -- `options` {Object} Accepts `options` from [`tls.createServer()`][] and [`tls.createSecureContext()`][]. +- `options` {Object} Accepts `options` from [`tls.createServer()`][], + [`tls.createSecureContext()`][] and [`http.createServer()`][]. - `requestListener` {Function} A listener to be added to the `request` event. Example: @@ -258,6 +259,7 @@ const req = https.request(options, (res) => { [`http.Server#setTimeout()`]: http.html#http_server_settimeout_msecs_callback [`http.Server#timeout`]: http.html#http_server_timeout [`http.Server`]: http.html#http_class_http_server +[`http.createServer()`]: http.html#httpcreateserveroptions-requestlistener [`http.close()`]: http.html#http_server_close_callback [`http.get()`]: http.html#http_http_get_options_callback [`http.request()`]: http.html#http_http_request_options_callback diff --git a/lib/_http_common.js b/lib/_http_common.js index b4caf5939e5afc..a7e8b0c59b8854 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -34,6 +34,7 @@ const { const debug = require('util').debuglog('http'); +const kIncomingMessage = Symbol('IncomingMessage'); const kOnHeaders = HTTPParser.kOnHeaders | 0; const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0; const kOnBody = HTTPParser.kOnBody | 0; @@ -73,7 +74,11 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method, parser._url = ''; } - parser.incoming = new IncomingMessage(parser.socket); + // Parser is also used by http client + var ParserIncomingMessage = parser.socket && parser.socket.server ? + parser.socket.server[kIncomingMessage] : IncomingMessage; + + parser.incoming = new ParserIncomingMessage(parser.socket); parser.incoming.httpVersionMajor = versionMajor; parser.incoming.httpVersionMinor = versionMinor; parser.incoming.httpVersion = `${versionMajor}.${versionMinor}`; @@ -300,5 +305,6 @@ module.exports = { freeParser, httpSocketSetup, methods, - parsers + parsers, + kIncomingMessage }; diff --git a/lib/_http_server.js b/lib/_http_server.js index c60119822a98d5..9541993df53321 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -33,6 +33,7 @@ const { continueExpression, chunkExpression, httpSocketSetup, + kIncomingMessage, _checkInvalidHeaderChar: checkInvalidHeaderChar } = require('_http_common'); const { OutgoingMessage } = require('_http_outgoing'); @@ -41,9 +42,12 @@ const { defaultTriggerAsyncIdScope, getOrSetAsyncId } = require('internal/async_hooks'); +const { IncomingMessage } = require('_http_incoming'); const errors = require('internal/errors'); const Buffer = require('buffer').Buffer; +const kServerResponse = Symbol('ServerResponse'); + const STATUS_CODES = { 100: 'Continue', 101: 'Switching Protocols', @@ -263,9 +267,19 @@ function writeHead(statusCode, reason, obj) { // Docs-only deprecated: DEP0063 ServerResponse.prototype.writeHeader = ServerResponse.prototype.writeHead; +function Server(options, requestListener) { + if (!(this instanceof Server)) return new Server(options, requestListener); + + if (typeof options === 'function') { + requestListener = options; + options = {}; + } else if (options == null || typeof options === 'object') { + options = util._extend({}, options); + } + + this[kIncomingMessage] = options.IncomingMessage || IncomingMessage; + this[kServerResponse] = options.ServerResponse || ServerResponse; -function Server(requestListener) { - if (!(this instanceof Server)) return new Server(requestListener); net.Server.call(this, { allowHalfOpen: true }); if (requestListener) { @@ -587,7 +601,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) { } } - var res = new ServerResponse(req); + var res = new server[kServerResponse](req); res._onPendingData = updateOutgoingData.bind(undefined, socket, state); res.shouldKeepAlive = keepAlive; @@ -690,5 +704,6 @@ module.exports = { STATUS_CODES, Server, ServerResponse, - _connectionListener: connectionListener + _connectionListener: connectionListener, + kServerResponse }; diff --git a/lib/https.js b/lib/https.js index 5013791fe2de25..741ce84d2f8820 100644 --- a/lib/https.js +++ b/lib/https.js @@ -36,6 +36,9 @@ const { inherits } = util; const debug = util.debuglog('https'); const { urlToOptions, searchParamsSymbol } = require('internal/url'); const errors = require('internal/errors'); +const { IncomingMessage, ServerResponse } = require('http'); +const { kIncomingMessage } = require('_http_common'); +const { kServerResponse } = require('_http_server'); function Server(opts, requestListener) { if (!(this instanceof Server)) return new Server(opts, requestListener); @@ -57,6 +60,9 @@ function Server(opts, requestListener) { opts.ALPNProtocols = ['http/1.1']; } + this[kIncomingMessage] = opts.IncomingMessage || IncomingMessage; + this[kServerResponse] = opts.ServerResponse || ServerResponse; + tls.Server.call(this, opts, _connectionListener); this.httpAllowHalfOpen = false; diff --git a/test/parallel/test-http-server-options-incoming-message.js b/test/parallel/test-http-server-options-incoming-message.js new file mode 100644 index 00000000000000..a4bfa1b7646fc6 --- /dev/null +++ b/test/parallel/test-http-server-options-incoming-message.js @@ -0,0 +1,41 @@ +'use strict'; + +/** + * This test covers http.Server({ IncomingMessage }) option: + * With IncomingMessage option the server should use + * the new class for creating req Object instead of the default + * http.IncomingMessage. + */ +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +class MyIncomingMessage extends http.IncomingMessage { + getUserAgent() { + return this.headers['user-agent'] || 'unknown'; + } +} + +const server = http.Server({ + IncomingMessage: MyIncomingMessage +}, common.mustCall(function(req, res) { + assert.strictEqual(req.getUserAgent(), 'node-test'); + res.statusCode = 200; + res.end(); +})); +server.listen(); + +server.on('listening', function makeRequest() { + http.get({ + port: this.address().port, + headers: { + 'User-Agent': 'node-test' + } + }, (res) => { + assert.strictEqual(res.statusCode, 200); + res.on('end', () => { + server.close(); + }); + res.resume(); + }); +}); diff --git a/test/parallel/test-http-server-options-server-response.js b/test/parallel/test-http-server-options-server-response.js new file mode 100644 index 00000000000000..f5adf39bed6d16 --- /dev/null +++ b/test/parallel/test-http-server-options-server-response.js @@ -0,0 +1,35 @@ +'use strict'; + +/** + * This test covers http.Server({ ServerResponse }) option: + * With ServerResponse option the server should use + * the new class for creating res Object instead of the default + * http.ServerResponse. + */ +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +class MyServerResponse extends http.ServerResponse { + status(code) { + return this.writeHead(code, { 'Content-Type': 'text/plain' }); + } +} + +const server = http.Server({ + ServerResponse: MyServerResponse +}, common.mustCall(function(req, res) { + res.status(200); + res.end(); +})); +server.listen(); + +server.on('listening', function makeRequest() { + http.get({ port: this.address().port }, (res) => { + assert.strictEqual(res.statusCode, 200); + res.on('end', () => { + server.close(); + }); + res.resume(); + }); +}); diff --git a/test/parallel/test-https-server-options-incoming-message.js b/test/parallel/test-https-server-options-incoming-message.js new file mode 100644 index 00000000000000..102ee56751b800 --- /dev/null +++ b/test/parallel/test-https-server-options-incoming-message.js @@ -0,0 +1,51 @@ +'use strict'; + +/** + * This test covers http.Server({ IncomingMessage }) option: + * With IncomingMessage option the server should use + * the new class for creating req Object instead of the default + * http.IncomingMessage. + */ +const common = require('../common'); +const fixtures = require('../common/fixtures'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const http = require('http'); +const https = require('https'); + +class MyIncomingMessage extends http.IncomingMessage { + getUserAgent() { + return this.headers['user-agent'] || 'unknown'; + } +} + +const server = https.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + ca: fixtures.readKey('ca1-cert.pem'), + IncomingMessage: MyIncomingMessage +}, common.mustCall(function(req, res) { + assert.strictEqual(req.getUserAgent(), 'node-test'); + res.statusCode = 200; + res.end(); +})); +server.listen(); + +server.on('listening', function makeRequest() { + https.get({ + port: this.address().port, + rejectUnauthorized: false, + headers: { + 'User-Agent': 'node-test' + } + }, (res) => { + assert.strictEqual(res.statusCode, 200); + res.on('end', () => { + server.close(); + }); + res.resume(); + }); +}); diff --git a/test/parallel/test-https-server-options-server-response.js b/test/parallel/test-https-server-options-server-response.js new file mode 100644 index 00000000000000..8745415f8b6596 --- /dev/null +++ b/test/parallel/test-https-server-options-server-response.js @@ -0,0 +1,47 @@ +'use strict'; + +/** + * This test covers http.Server({ ServerResponse }) option: + * With ServerResponse option the server should use + * the new class for creating res Object instead of the default + * http.ServerResponse. + */ +const common = require('../common'); +const fixtures = require('../common/fixtures'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const http = require('http'); +const https = require('https'); + +class MyServerResponse extends http.ServerResponse { + status(code) { + return this.writeHead(code, { 'Content-Type': 'text/plain' }); + } +} + +const server = https.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + ca: fixtures.readKey('ca1-cert.pem'), + ServerResponse: MyServerResponse +}, common.mustCall(function(req, res) { + res.status(200); + res.end(); +})); +server.listen(); + +server.on('listening', function makeRequest() { + https.get({ + port: this.address().port, + rejectUnauthorized: false + }, (res) => { + assert.strictEqual(res.statusCode, 200); + res.on('end', () => { + server.close(); + }); + res.resume(); + }); +}); From 5a1794141cb27e94b61c1c2d74896d0c2f5a03c4 Mon Sep 17 00:00:00 2001 From: Peter Marton Date: Fri, 27 Oct 2017 09:18:59 +0200 Subject: [PATCH 2/2] http2: add Http2ServerRequest and Http2ServerResponse options to Server --- doc/api/http2.md | 6 ++ lib/internal/http2/core.js | 15 +++- ...ttp2-https-fallback-http-server-options.js | 90 +++++++++++++++++++ 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http2-https-fallback-http-server-options.js diff --git a/doc/api/http2.md b/doc/api/http2.md index e79e46ac3e69b8..a9eaa448e870c7 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1726,6 +1726,12 @@ changes: used to determine the padding. See [Using options.selectPadding][]. * `settings` {[Settings Object][]} The initial settings to send to the remote peer upon connection. + * `Http1IncomingMessage` {http.IncomingMessage} Specifies the IncomingMessage class to + used for HTTP/1 fallback. Useful for extending the original `http.IncomingMessage`. + Defaults to: `http.IncomingMessage` + * `Http1ServerResponse` {http.ServerResponse} Specifies the ServerResponse class to + used for HTTP/1 fallback. Useful for extending the original `http.ServerResponse`. + Defaults to: `http.ServerResponse` * `onRequestHandler` {Function} See [Compatibility API][] * Returns: {Http2Server} diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7c5792c6ce101d..634435c9d8be12 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -5,6 +5,7 @@ require('internal/util').assertCrypto(); const { async_id_symbol } = process.binding('async_wrap'); +const http = require('http'); const binding = process.binding('http2'); const assert = require('assert'); const { Buffer } = require('buffer'); @@ -69,6 +70,8 @@ const NETServer = net.Server; const TLSServer = tls.Server; const kInspect = require('internal/util').customInspectSymbol; +const { kIncomingMessage } = require('_http_common'); +const { kServerResponse } = require('_http_server'); const kAlpnProtocol = Symbol('alpnProtocol'); const kAuthority = Symbol('authority'); @@ -2454,8 +2457,11 @@ function connectionListener(socket) { if (socket.alpnProtocol === false || socket.alpnProtocol === 'http/1.1') { // Fallback to HTTP/1.1 - if (options.allowHTTP1 === true) + if (options.allowHTTP1 === true) { + socket.server[kIncomingMessage] = options.Http1IncomingMessage; + socket.server[kServerResponse] = options.Http1ServerResponse; return httpConnectionListener.call(this, socket); + } // Let event handler deal with the socket if (!this.emit('unknownProtocol', socket)) socket.destroy(); @@ -2486,6 +2492,13 @@ function initializeOptions(options) { options.allowHalfOpen = true; assertIsObject(options.settings, 'options.settings'); options.settings = Object.assign({}, options.settings); + + // Used only with allowHTTP1 + options.Http1IncomingMessage = options.Http1IncomingMessage || + http.IncomingMessage; + options.Http1ServerResponse = options.Http1ServerResponse || + http.ServerResponse; + return options; } diff --git a/test/parallel/test-http2-https-fallback-http-server-options.js b/test/parallel/test-http2-https-fallback-http-server-options.js new file mode 100644 index 00000000000000..20e2b122a24e8c --- /dev/null +++ b/test/parallel/test-http2-https-fallback-http-server-options.js @@ -0,0 +1,90 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +const fixtures = require('../common/fixtures'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const url = require('url'); +const tls = require('tls'); +const http2 = require('http2'); +const https = require('https'); +const http = require('http'); + +const key = fixtures.readKey('agent8-key.pem'); +const cert = fixtures.readKey('agent8-cert.pem'); +const ca = fixtures.readKey('fake-startcom-root-cert.pem'); + +function onRequest(request, response) { + const { socket: { alpnProtocol } } = request.httpVersion === '2.0' ? + request.stream.session : request; + response.status(200); + response.end(JSON.stringify({ + alpnProtocol, + httpVersion: request.httpVersion, + userAgent: request.getUserAgent() + })); +} + +class MyIncomingMessage extends http.IncomingMessage { + getUserAgent() { + return this.headers['user-agent'] || 'unknown'; + } +} + +class MyServerResponse extends http.ServerResponse { + status(code) { + return this.writeHead(code, { 'Content-Type': 'application/json' }); + } +} + +// HTTP/2 & HTTP/1.1 server +{ + const server = http2.createSecureServer( + { + cert, + key, allowHTTP1: true, + Http1IncomingMessage: MyIncomingMessage, + Http1ServerResponse: MyServerResponse + }, + common.mustCall(onRequest, 1) + ); + + server.listen(0); + + server.on('listening', common.mustCall(() => { + const { port } = server.address(); + const origin = `https://localhost:${port}`; + + // HTTP/1.1 client + https.get( + Object.assign(url.parse(origin), { + secureContext: tls.createSecureContext({ ca }), + headers: { 'User-Agent': 'node-test' } + }), + common.mustCall((response) => { + assert.strictEqual(response.statusCode, 200); + assert.strictEqual(response.statusMessage, 'OK'); + assert.strictEqual( + response.headers['content-type'], + 'application/json' + ); + + response.setEncoding('utf8'); + let raw = ''; + response.on('data', (chunk) => { raw += chunk; }); + response.on('end', common.mustCall(() => { + const { alpnProtocol, httpVersion, userAgent } = JSON.parse(raw); + assert.strictEqual(alpnProtocol, false); + assert.strictEqual(httpVersion, '1.1'); + assert.strictEqual(userAgent, 'node-test'); + + server.close(); + })); + }) + ); + })); +}