From 59bca6c2d1cbba06c6463fb7374e9c069e9f2343 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Sat, 7 Jan 2017 23:31:17 +0100 Subject: [PATCH 1/2] Treat null tlsconfig as missing --- lib/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/index.js b/lib/index.js index 04b0b4f..e9aa6c2 100644 --- a/lib/index.js +++ b/lib/index.js @@ -14,7 +14,7 @@ function Server(tlsconfig, requestListener) { tlsconfig = undefined; } - if (typeof tlsconfig === 'object') { + if (typeof tlsconfig === 'object' && tlsconfig !== null) { this.removeAllListeners('connection'); https.Server.call(this, tlsconfig, requestListener); From ea3a71f1d2c0ff1c6a3c9a029263228af052b19a Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Sat, 7 Jan 2017 23:32:05 +0100 Subject: [PATCH 2/2] return http.Server when TLS config is missing The common pattern in the implementation of Server (including the http.Server and https.Server objects from the Node.js core) is the following (note that `this` is completely ignored): if (!(this instanceof Server)) return new Server(...arguments); There are three relevant `Server` implementations, with the following inheritance tree: - tls.Server +-- http.Server +-- https.Server +-- httpolyglot.Server When `httpolyglot.Server` is constructed without TLS config, it falls back to constructing itself using `http.Server`: // this is an instance of httpolyglot.Server http.Server.call(this, requestListener); But `this` is not an instance of `http.Server`, so a new `http.Server` instance is created (and discarded) and `this` is not modified. So, when `listen()` is called, the server starts listening on a port but does not have any request handlers. Consequently, when a http(s) request is sent to the server, the connection is accepted but the request is never processed. To fix this, be explicit and return the new instance of `http.Server`. --- lib/index.js | 2 +- test/test-no-https.js | 45 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100755 test/test-no-https.js diff --git a/lib/index.js b/lib/index.js index e9aa6c2..d7e6f70 100644 --- a/lib/index.js +++ b/lib/index.js @@ -36,7 +36,7 @@ function Server(tlsconfig, requestListener) { this.allowHalfOpen = true; this.httpAllowHalfOpen = false; } else - http.Server.call(this, requestListener); + return new http.Server(requestListener); } inherits(Server, https.Server); diff --git a/test/test-no-https.js b/test/test-no-https.js new file mode 100755 index 0000000..fe0615c --- /dev/null +++ b/test/test-no-https.js @@ -0,0 +1,45 @@ +var fs = require('fs'); +var http = require('http'); +var https = require('https'); +var assert = require('assert'); + +var common = require(__dirname + '/common'); +var httpolyglot = require(__dirname + '/../lib/index'); + +var srv = httpolyglot.createServer(null, common.mustCall(function(req, res) { + res.end(req.socket.encrypted ? 'https' : 'http'); +}, 1)); +assert(srv instanceof http.Server); +srv.listen(0, '127.0.0.1', common.mustCall(function() { + var port = this.address().port; + var count = 2; + function done() { + if (--count === 0) { + srv.close(); + } + } + + http.get({ + host: '127.0.0.1', + port: port + }, common.mustCall(function(res) { + var body = ''; + res.on('data', function(data) { + body += data; + }).on('end', common.mustCall(function() { + assert.strictEqual(body, 'http'); + done(); + })); + })); + + https.get({ + host: '127.0.0.1', + port: port, + rejectUnauthorized: false + }, function() { + assert(false, 'The request should have failed for the lack of TLS config'); + }).on('error', common.mustCall(function(err) { + assert.strictEqual(err.code, 'ECONNRESET'); + done(); + })); +}));