From 4e2d17a074f44f520a063f67dc6c08406dc7248a Mon Sep 17 00:00:00 2001 From: Scott Griffy Date: Sat, 3 Dec 2016 21:55:45 +0000 Subject: [PATCH 1/7] feat(server): Added 'listenAddress' option and updated unit tests to work correctly with it reference: #2477 --- docs/config/01-configuration-file.md | 6 ++++++ lib/config.js | 1 + lib/constants.js | 1 + lib/server.js | 6 +++--- test/unit/server.spec.js | 9 ++++++++- 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/config/01-configuration-file.md b/docs/config/01-configuration-file.md index 69939c78f..12641af02 100644 --- a/docs/config/01-configuration-file.md +++ b/docs/config/01-configuration-file.md @@ -368,6 +368,12 @@ Please note just about all frameworks in Karma require an additional plugin/fram Additional information can be found in [plugins]. +## listenAddress +**Type:** String + +**Default:** `'0.0.0.0' or LISTEN_ADDR` + +**Description:** Address that the server will listen on. Change to 'localhost' to only listen to the loopback, or '::' to listen on all IPv6 interfaces ## hostname **Type:** String diff --git a/lib/config.js b/lib/config.js index 008e981b0..7d69513cf 100644 --- a/lib/config.js +++ b/lib/config.js @@ -280,6 +280,7 @@ var Config = function () { this.frameworks = [] this.protocol = 'http:' this.port = constant.DEFAULT_PORT + this.listenAddress = constant.DEFAULT_LISTEN_ADDR this.hostname = constant.DEFAULT_HOSTNAME this.httpsServerConfig = {} this.basePath = '' diff --git a/lib/constants.js b/lib/constants.js index 6008a7309..6da52abd0 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -7,6 +7,7 @@ exports.VERSION = pkg.version exports.DEFAULT_PORT = process.env.PORT || 9876 exports.DEFAULT_HOSTNAME = process.env.IP || 'localhost' +exports.DEFAULT_LISTEN_ADDR = process.env.LISTEN_ADDR || '0.0.0.0' // log levels exports.LOG_DISABLE = 'OFF' diff --git a/lib/server.js b/lib/server.js index 0c91bdbf3..b68631b0e 100644 --- a/lib/server.js +++ b/lib/server.js @@ -160,7 +160,7 @@ Server.prototype._start = function (config, launcher, preprocess, fileList, if (e.code === 'EADDRINUSE') { self.log.warn('Port %d in use', config.port) config.port++ - webServer.listen(config.port) + webServer.listen(config.port, config.listenAddress) } else { throw e } @@ -171,9 +171,9 @@ Server.prototype._start = function (config, launcher, preprocess, fileList, self._injector.invoke(watcher.watch) } - webServer.listen(config.port, function () { + webServer.listen(config.port, config.listenAddress, function () { self.log.info('Karma v%s server started at %s//%s:%s%s', constant.VERSION, - config.protocol, config.hostname, config.port, config.urlRoot) + config.protocol, config.listenAddress, config.port, config.urlRoot) self.emit('listening', config.port) if (config.browsers && config.browsers.length) { diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js index 722cb80f7..dcb6c96e7 100644 --- a/test/unit/server.spec.js +++ b/test/unit/server.spec.js @@ -77,7 +77,14 @@ describe('server', () => { webServerOnError = handler } }, - listen: sinon.spy((port, callback) => { + listen: sinon.spy((port, arg2, arg3) => { + var callback = null + if (typeof (arg2) === 'function') { + callback = arg2 + } else + if (typeof (arg3) === 'function') { + callback = arg3 + } callback && callback() }), removeAllListeners: () => {}, From c4fc0a718111c93cf7e3e749e9e416eacb1828af Mon Sep 17 00:00:00 2001 From: Scott Griffy Date: Tue, 6 Dec 2016 05:58:41 +0000 Subject: [PATCH 2/7] fix(test): making the if look better and removing uneccessary parens --- test/unit/server.spec.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js index dcb6c96e7..783a9675e 100644 --- a/test/unit/server.spec.js +++ b/test/unit/server.spec.js @@ -79,10 +79,9 @@ describe('server', () => { }, listen: sinon.spy((port, arg2, arg3) => { var callback = null - if (typeof (arg2) === 'function') { + if (typeof arg2 === 'function') { callback = arg2 - } else - if (typeof (arg3) === 'function') { + } else if (typeof arg3 === 'function') { callback = arg3 } callback && callback() From 75e43066f17d423b1ff20fbb52f8de80477f1971 Mon Sep 17 00:00:00 2001 From: Scott Griffy Date: Tue, 6 Dec 2016 21:56:40 +0000 Subject: [PATCH 3/7] fix(server): warn the user if listenAddress is set but hostname isn't --- lib/config.js | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/config.js b/lib/config.js index 7d69513cf..a79774531 100644 --- a/lib/config.js +++ b/lib/config.js @@ -104,7 +104,7 @@ var normalizeProxyPath = function (proxyPath) { return normalizedProxyPath } -var normalizeConfig = function (config, configFilePath) { +var normalizeConfig = function (config, configFilePath, configModule, cliOptions) { var basePathResolve = function (relativePath) { if (helper.isUrlAbsolute(relativePath)) { return relativePath @@ -199,6 +199,23 @@ var normalizeConfig = function (config, configFilePath) { throw new TypeError('Invalid configuration: formatError option must be a function.') } + // if the user changed listenAddres, but didn't set a hostname, warn them + var testConfig = new Config() + testConfig.hostname = null + testConfig.listenAddress = null + if (configModule != null) { + configModule(testConfig) + } + if (cliOptions != null) { + testConfig.set(cliOptions) + } + if (testConfig.hostname == null) { + if (testConfig.listenAddress != null) { + log.warn('ListenAddress is set but hostname isn\'t, if your browsers fail ' + + 'to connect, consider setting the hostname option.') + } + } + var defaultClient = config.defaultClient || {} Object.keys(defaultClient).forEach(function (key) { var option = config.client[key] @@ -388,7 +405,7 @@ var parseConfig = function (configFilePath, cliOptions) { // configure the logger as soon as we can logger.setup(config.logLevel, config.colors, config.loggers) - return normalizeConfig(config, configFilePath) + return normalizeConfig(config, configFilePath, configModule, cliOptions) } // PUBLIC API From 5e29fd908d4bf65956c31143a448a3dd5f44039b Mon Sep 17 00:00:00 2001 From: Scott Griffy Date: Tue, 6 Dec 2016 23:34:50 +0000 Subject: [PATCH 4/7] fix(config): some typos --- lib/config.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/config.js b/lib/config.js index a79774531..be6197e46 100644 --- a/lib/config.js +++ b/lib/config.js @@ -199,7 +199,7 @@ var normalizeConfig = function (config, configFilePath, configModule, cliOptions throw new TypeError('Invalid configuration: formatError option must be a function.') } - // if the user changed listenAddres, but didn't set a hostname, warn them + // if the user changed listenAddress, but didn't set a hostname, warn them var testConfig = new Config() testConfig.hostname = null testConfig.listenAddress = null @@ -209,11 +209,9 @@ var normalizeConfig = function (config, configFilePath, configModule, cliOptions if (cliOptions != null) { testConfig.set(cliOptions) } - if (testConfig.hostname == null) { - if (testConfig.listenAddress != null) { - log.warn('ListenAddress is set but hostname isn\'t, if your browsers fail ' + - 'to connect, consider setting the hostname option.') - } + if (testConfig.hostname == null && testConfig.listenAddress != null) { + log.warn('ListenAddress is set but hostname isn\'t. If your browsers fail ' + + 'to connect, consider setting the hostname option.') } var defaultClient = config.defaultClient || {} From d8eadba83d0a4ac7f32807c8e4c736e3b1c44e3d Mon Sep 17 00:00:00 2001 From: Scott Griffy Date: Wed, 7 Dec 2016 19:21:24 +0000 Subject: [PATCH 5/7] fix(config): better checking for overwritten options in config --- lib/config.js | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/config.js b/lib/config.js index be6197e46..18e81a8ac 100644 --- a/lib/config.js +++ b/lib/config.js @@ -104,7 +104,7 @@ var normalizeProxyPath = function (proxyPath) { return normalizedProxyPath } -var normalizeConfig = function (config, configFilePath, configModule, cliOptions) { +var normalizeConfig = function (config, configFilePath) { var basePathResolve = function (relativePath) { if (helper.isUrlAbsolute(relativePath)) { return relativePath @@ -199,21 +199,6 @@ var normalizeConfig = function (config, configFilePath, configModule, cliOptions throw new TypeError('Invalid configuration: formatError option must be a function.') } - // if the user changed listenAddress, but didn't set a hostname, warn them - var testConfig = new Config() - testConfig.hostname = null - testConfig.listenAddress = null - if (configModule != null) { - configModule(testConfig) - } - if (cliOptions != null) { - testConfig.set(cliOptions) - } - if (testConfig.hostname == null && testConfig.listenAddress != null) { - log.warn('ListenAddress is set but hostname isn\'t. If your browsers fail ' + - 'to connect, consider setting the hostname option.') - } - var defaultClient = config.defaultClient || {} Object.keys(defaultClient).forEach(function (key) { var option = config.client[key] @@ -388,6 +373,15 @@ var parseConfig = function (configFilePath, cliOptions) { } var config = new Config() + + // save and reset hostname and listenAddress so we can detect if the user + // changed them + var defaultHostname = config.hostname + config.hostname = null + var defaultListenAddress = config.listenAddress + config.listenAddress = null + + // add the user's configuration in config.set(cliOptions) try { @@ -400,10 +394,24 @@ var parseConfig = function (configFilePath, cliOptions) { // merge the config from config file and cliOptions (precedence) config.set(cliOptions) + // if the user changed listenAddress, but didn't set a hostname, warn them + if (config.hostname == null && config.listenAddress != null) { + log.warn('ListenAddress was set to %s but hostname was left as the default: ' + + '%s. If your browsers fail to connect, consider changing the hostname option.', + config.listenAddress, defaultHostname) + } + // restore values that weren't overwritten by the user + if (config.hostname == null) { + config.hostname = defaultHostname + } + if (config.listenAddress == null) { + config.listenAddress = defaultListenAddress + } + // configure the logger as soon as we can logger.setup(config.logLevel, config.colors, config.loggers) - return normalizeConfig(config, configFilePath, configModule, cliOptions) + return normalizeConfig(config, configFilePath) } // PUBLIC API From a0bf79e2429cd528f0f1b094ab6159fe04c11580 Mon Sep 17 00:00:00 2001 From: Scott Griffy Date: Wed, 7 Dec 2016 19:56:17 +0000 Subject: [PATCH 6/7] fix(config): safer checks for unset options --- lib/config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/config.js b/lib/config.js index 18e81a8ac..089125bcf 100644 --- a/lib/config.js +++ b/lib/config.js @@ -395,16 +395,16 @@ var parseConfig = function (configFilePath, cliOptions) { config.set(cliOptions) // if the user changed listenAddress, but didn't set a hostname, warn them - if (config.hostname == null && config.listenAddress != null) { + if (config.hostname === null && config.listenAddress !== null) { log.warn('ListenAddress was set to %s but hostname was left as the default: ' + '%s. If your browsers fail to connect, consider changing the hostname option.', config.listenAddress, defaultHostname) } // restore values that weren't overwritten by the user - if (config.hostname == null) { + if (config.hostname === null) { config.hostname = defaultHostname } - if (config.listenAddress == null) { + if (config.listenAddress === null) { config.listenAddress = defaultListenAddress } From d931ecf9be7872c51361b2b41a80bb33fa6642f2 Mon Sep 17 00:00:00 2001 From: Scott Griffy Date: Sat, 10 Dec 2016 21:54:17 +0000 Subject: [PATCH 7/7] feat(test): added a unit test for listenAddress --- test/unit/server.spec.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js index 783a9675e..dc86788fd 100644 --- a/test/unit/server.spec.js +++ b/test/unit/server.spec.js @@ -27,6 +27,7 @@ describe('server', () => { {frameworks: [], port: 9876, autoWatch: true, + listenAddress: '127.0.0.1', hostname: 'localhost', urlRoot: '/', browsers: ['fake'], @@ -141,6 +142,20 @@ describe('server', () => { expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher) }) + it('should listen on the listenAddress in the config', () => { + server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy) + + expect(mockWebServer.listen).not.to.have.been.called + expect(webServerOnError).not.to.be.null + + expect(mockConfig.listenAddress).to.be.equal('127.0.0.1') + + fileListOnResolve() + + expect(mockWebServer.listen).to.have.been.calledWith(9876, '127.0.0.1') + expect(mockConfig.listenAddress).to.be.equal('127.0.0.1') + }) + it('should try next port if already in use', () => { server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)