From ddb1a280cb77576c5f390316f030bf76a61e82b1 Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Sat, 10 Mar 2018 13:54:51 +0200 Subject: [PATCH] Allow overriding arrays in config, warn about incorrect types Fixes #2176 --- src/helper.js | 19 ++++- test/tests/mergeConfig.js | 149 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 test/tests/mergeConfig.js diff --git a/src/helper.js b/src/helper.js index 73e5b2f6..3c1954b5 100644 --- a/src/helper.js +++ b/src/helper.js @@ -30,6 +30,7 @@ const Helper = { getVersion, getGitCommit, ip2hex, + mergeConfig, password: { hash: passwordHash, @@ -89,7 +90,7 @@ function setHome(newPath) { log.warn("Using default configuration..."); } - this.config = _.merge(this.config, userConfig); + mergeConfig(this.config, userConfig); } if (!this.config.displayNetwork && !this.config.lockNetwork) { @@ -174,3 +175,19 @@ function passwordHash(password) { function passwordCompare(password, expected) { return bcrypt.compare(password, expected); } + +function mergeConfig(oldConfig, newConfig) { + return _.mergeWith(oldConfig, newConfig, (objValue, srcValue, key) => { + // Do not override config variables if the type is incorrect (e.g. object changed into a string) + if (typeof objValue !== "undefined" && objValue !== null && typeof objValue !== typeof srcValue) { + log.warn(`Incorrect type for "${colors.bold(key)}", please verify your config.`); + + return objValue; + } + + // For arrays, simply override the value with user provided one. + if (_.isArray(objValue)) { + return srcValue; + } + }); +} diff --git a/test/tests/mergeConfig.js b/test/tests/mergeConfig.js new file mode 100644 index 00000000..0cde7a0c --- /dev/null +++ b/test/tests/mergeConfig.js @@ -0,0 +1,149 @@ +"use strict"; + +const expect = require("chai").expect; +const mergeConfig = require("../../src/helper").mergeConfig; + +describe("mergeConfig", function() { + it("should mutate object", function() { + const config = { + ip: "default", + }; + + expect(mergeConfig(config, { + ip: "overridden", + })).to.deep.equal({ + ip: "overridden", + }); + + expect(config).to.deep.equal({ + ip: "overridden", + }); + }); + + it("should merge new properties", function() { + expect(mergeConfig({ + ip: "default", + newProp: "this should appear too", + }, { + ip: "overridden", + })).to.deep.equal({ + ip: "overridden", + newProp: "this should appear too", + }); + }); + + it("should extend objects", function() { + expect(mergeConfig({ + tlsOptions: {}, + }, { + tlsOptions: { + user: "test", + thing: 123, + }, + })).to.deep.equal({ + tlsOptions: { + user: "test", + thing: 123, + }, + }); + }); + + it("should allow changing nulls", function() { + expect(mergeConfig({ + oidentd: null, + }, { + oidentd: "some path", + })).to.deep.equal({ + oidentd: "some path", + }); + }); + + it("should keep new properties inside of objects", function() { + expect(mergeConfig({ + nestedOnce: { + ip: "default", + }, + nestedTwice: { + thing: "default", + nested: { + otherThing: "also default", + newThing: "but also this", + }, + }, + }, { + nestedOnce: {}, + nestedTwice: { + nested: { + otherThing: "overridden", + }, + }, + })).to.deep.equal({ + nestedOnce: { + ip: "default", + }, + nestedTwice: { + thing: "default", + nested: { + otherThing: "overridden", + newThing: "but also this", + }, + }, + }); + }); + + it("should not merge arrays", function() { + expect(mergeConfig({ + test: ["sqlite", "text"], + }, { + test: ["sqlite"], + })).to.deep.equal({ + test: ["sqlite"], + }); + + expect(mergeConfig({ + test: ["sqlite", "text"], + }, { + test: [], + })).to.deep.equal({ + test: [], + }); + }); + + it("should change order in arrays", function() { + expect(mergeConfig({ + test: ["sqlite", "text"], + }, { + test: ["text", "sqlite"], + })).to.deep.equal({ + test: ["text", "sqlite"], + }); + }); + + it("should only merge same type", function() { + const originalLog = log.info; + + log.warn = () => {}; + + expect(mergeConfig({ + shouldBeObject: { + thing: "yes", + }, + }, { + shouldBeObject: "bad type", + })).to.deep.equal({ + shouldBeObject: { + thing: "yes", + }, + }); + + expect(mergeConfig({ + shouldBeString: "string", + }, { + shouldBeString: 1234567, + })).to.deep.equal({ + shouldBeString: "string", + }); + + log.warn = originalLog; + }); +});