From 0398bd10f83369ebe5feca146bc20b787f8b4333 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sat, 3 Aug 2019 12:50:45 +0200 Subject: [PATCH 01/15] Remove debug code Remove debug switch and console.log guarded by it to log every post request to /api/getId as it's not needed. --- master.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/master.js b/master.js index 858d0843..8ecc8584 100644 --- a/master.js +++ b/master.js @@ -14,9 +14,6 @@ node master.js // const updater = require("./updater.js"); // updater.update().then(console.log); -// configgy stuff -debug = false; - // argument parsing const args = require('minimist')(process.argv.slice(2)); @@ -284,9 +281,6 @@ app.post("/api/getID", authenticate.middleware, function(req,res) { // time us a unix timestamp we can use to check for how long the server has been unresponsive // we should save that somewhere and give appropriate response if(req.body){ - if(debug){ - console.log(req.body) - } if(!slaves[req.body.unique]){ slaves[req.body.unique] = {}; } From 38462d30699a925a6f116dab8deb8a498647bd67 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sat, 3 Aug 2019 19:22:29 +0200 Subject: [PATCH 02/15] Set User-Agent according to GitHub's request GitHub requests that the user agent be set to either the name of the user or the application[1]. Set user agent to the name of this application as they request. [1] https://developer.github.com/v3/#user-agent-required --- client.js | 2 +- lib/manager/modManager.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client.js b/client.js index 6434ba22..bb82a891 100644 --- a/client.js +++ b/client.js @@ -271,7 +271,7 @@ if (!command || command == "help" || command == "--help") { } else if (command == "download") { console.log("Downloading mods..."); // get JSON data about releases - let res = syncRequest('GET', 'https://api.github.com/repos/Danielv123/factorioClusterioMod/releases', {"headers":{"User-Agent":"Fuck you for requiring user agents!"}}); + let res = syncRequest('GET', 'https://api.github.com/repos/Danielv123/factorioClusterioMod/releases', {"headers":{"User-Agent":"factorioClusterio"}}); let url = JSON.parse(res.getBody())[0].assets[0].browser_download_url; let name = JSON.parse(res.getBody())[0].assets[0].name; if(url) { diff --git a/lib/manager/modManager.js b/lib/manager/modManager.js index 999c81d8..b975db96 100644 --- a/lib/manager/modManager.js +++ b/lib/manager/modManager.js @@ -89,7 +89,7 @@ module.exports = function(config){ } // get JSON data about GH releases - /*let res = syncRequest('GET', 'https://api.github.com/repos/Danielv123/factorioClusterioMod/releases', {"headers":{"User-Agent":"Fuck you for requiring user agents!"}}); + /*let res = syncRequest('GET', 'https://api.github.com/repos/Danielv123/factorioClusterioMod/releases', {"headers":{"User-Agent":"factorioClusterio"}}); let url = JSON.parse(res.getBody())[0].assets[0].browser_download_url; let name = JSON.parse(res.getBody())[0].assets[0].name; console.log(JSON.parse(res.getBody())) @@ -159,7 +159,7 @@ module.exports = function(config){ // treat it as a github releases download let apiURL = "https://api.github.com/repos/" + term.split("/")[3] + "/" + term.split("/")[4] + "/releases"; // https://api.github.com/repos/Danielv123/factorioClusterioMod/releases - let res = syncRequest('GET', apiURL, {"headers":{"User-Agent":"Fuck you for requiring user agents!"}}); + let res = syncRequest('GET', apiURL, {"headers":{"User-Agent":"factorioClusterio"}}); let url = JSON.parse(res.getBody())[0].assets[0].browser_download_url; let name = JSON.parse(res.getBody())[0].assets[0].name; if(url) { @@ -251,7 +251,7 @@ module.exports = function(config){ downloadFromGitURL: function(url){ console.log("Downloading mod from "+url); // get JSON data about releases - let res = syncRequest('GET', url, {"headers":{"User-Agent":"Please don't require setting a user agent"}}); + let res = syncRequest('GET', url, {"headers":{"User-Agent":"factorioClusterio"}}); let downloadUrl = JSON.parse(res.getBody())[0].assets[0].browser_download_url; let name = JSON.parse(res.getBody())[0].assets[0].name; console.log(JSON.parse(res.getBody())) @@ -270,7 +270,7 @@ module.exports = function(config){ module.exports.findUpdates = function(){ console.log("Downloading mods..."); // get JSON data about releases - let res = syncRequest('GET', 'https://api.github.com/repos/Danielv123/factorioClusterioMod/releases', {"headers":{"User-Agent":"Fuck you for requiring user agents!"}}); + let res = syncRequest('GET', 'https://api.github.com/repos/Danielv123/factorioClusterioMod/releases', {"headers":{"User-Agent":"factorioClusterio"}}); let url = JSON.parse(res.getBody())[0].assets[0].browser_download_url; let name = JSON.parse(res.getBody())[0].assets[0].name; console.log(JSON.parse(res.getBody())) From 0a9ddef1201f4c6e0a08a52a5f075b0fe4009ea5 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 13:44:41 +0200 Subject: [PATCH 03/15] Check for globals leaking during test Add --check-leaks to mocha to have it report globals that leak during the testing. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2a652d46..118ca713 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "repository": "https://github.com/clusterio/factorioClusterio", "main": "master.js", "scripts": { - "test": "mocha --exit -R spec \"./{,!(node_modules)/**/}*.spec.js\" || true", + "test": "mocha --exit --check-leaks -R spec \"./{,!(node_modules)/**/}*.spec.js\" || true", "cover": "nyc npm run-script test", "ci-cover": "nyc --reporter=lcovonly npm run-script test", "start": "echo Run node client.js start [instance] to start slaves && node master.js", From 0d4ad2f07541589368943d73c51b0bdfea71dc93 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 13:51:01 +0200 Subject: [PATCH 04/15] Fix global leaks detected by mocha Fix globals leaked in stringUtils, isJson and static/nav.html. --- lib/stringUtils.js | 3 +-- sharedPlugins/inventoryImports/isJson.js | 2 +- static/nav.html | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/stringUtils.js b/lib/stringUtils.js index f6f591c3..7febd82b 100644 --- a/lib/stringUtils.js +++ b/lib/stringUtils.js @@ -3,8 +3,7 @@ function hashCode(string){ var hash = 0; if (string.length == 0) return hash; for (let i = 0; i < string.length; i++) { - char = string.charCodeAt(i); - hash = ((hash << 5) - hash) + char; + hash = ((hash << 5) - hash) + string.charCodeAt(i); hash = hash & hash; // Convert to 32bit integer } return hash; diff --git a/sharedPlugins/inventoryImports/isJson.js b/sharedPlugins/inventoryImports/isJson.js index 597564df..c0a1b3c1 100644 --- a/sharedPlugins/inventoryImports/isJson.js +++ b/sharedPlugins/inventoryImports/isJson.js @@ -2,7 +2,7 @@ module.exports = function(x){ if(x === false || x === true || x === null) return true; // === false is valid JSON, so is true and null var isNotJson = false try{ - z = JSON.parse(x); + JSON.parse(x); } catch (e) { isNotJson = true; } diff --git a/static/nav.html b/static/nav.html index 0787e5d6..6c359198 100644 --- a/static/nav.html +++ b/static/nav.html @@ -13,7 +13,7 @@ <%# Very ugly ejs code to hide the Players button if the playerManager plugin isn`t installed %> <% - sidebarPluginButtonsHtml = ""; + let sidebarPluginButtonsHtml = ""; for(let o in locals.masterPlugins){ let plug = locals.masterPlugins[o]; if(plug.main From 7dcd46a99a1c38b47d0d212bf925dff5cbe81d82 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 17:10:31 +0200 Subject: [PATCH 05/15] Add .vs to .gitignore Prevent Visual Studio workspace files from getting accidentally added to the project. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 670f836b..d3285b41 100644 --- a/.gitignore +++ b/.gitignore @@ -65,3 +65,4 @@ sharedPlugins/*/DISABLED # IDE .idea/ +.vs/ From ed54db0db9e1eca03b152af64b1803d92493a6ef Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Thu, 22 Aug 2019 15:48:04 +0200 Subject: [PATCH 06/15] Handle errors during master startup Print a message urging unhandled exceptions thrown by startup to be reported to the GitHub issue tracker and initiate shutdown. --- CHANGELOG.md | 1 + master.js | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65db5d1b..148b985d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Version 2.0.0 - Fixed sslCert and sslPrivKey entries being ignored in the config. - Changed ssl key creation to be done at startup instead of on npm install. +- Added error handling during master startup. Breaking Changes ---------------- diff --git a/master.js b/master.js index 8ecc8584..714b98ce 100644 --- a/master.js +++ b/master.js @@ -1042,5 +1042,12 @@ async function startServer() { module.exports = app; if (module === require.main) { - startServer(); + startServer().catch(err => { + console.error( + "Unexpected error occured while starting master, please report\n"+ + "it to https://github.com/clusterio/factorioClusterio/issues" + ); + console.error(err); + return shutdown(); + }); } From 3e80e40223404d20704457bb90816ca8195d6966 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 21:10:57 +0200 Subject: [PATCH 07/15] Move database and masterMods folder creation Move the code creating the database and masterMods folders into the startServer function in master.js. --- master.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/master.js b/master.js index 714b98ce..94615de0 100644 --- a/master.js +++ b/master.js @@ -17,8 +17,6 @@ node master.js // argument parsing const args = require('minimist')(process.argv.slice(2)); -// Library for create folder recursively if it does not exist -const mkdirp = require("mkdirp"); const averagedTimeSeries = require("averaged-timeseries"); const deepmerge = require("deepmerge"); const path = require("path"); @@ -32,10 +30,6 @@ const request = require("request"); // constants console.log(`Requiring config from ${args.config || './config'}`); const config = require(args.config || './config'); -config.databaseDirectory = args.databaseDirectory || config.databaseDirectory || "./database"; -const masterModFolder = path.join(config.databaseDirectory, "/masterMods/"); -mkdirp.sync(config.databaseDirectory); -mkdirp.sync(masterModFolder); // homebrew modules const generateSSLcert = require("lib/generateSSLcert"); @@ -82,8 +76,6 @@ require("./routes.js")(app); require("./routes/api/getPictures.js")(app); // Set folder to serve static content from (the website) app.use(express.static('static')); -// mod downloads -app.use(express.static(masterModFolder)); // set up logging software const prometheusPrefix = "clusterio_"; @@ -356,7 +348,7 @@ POST Check if a mod has been uploaded to the master before. Only checks against */ app.post("/api/checkMod", authenticate.middleware, function(req,res) { endpointHitCounter.labels(req.route.path).inc(); - let files = fs.readdirSync(masterModFolder); + let files = fs.readdirSync(path.join(config.databaseDirectory, "masterMods")); let found = false; files.forEach(file => { if(file == req.body.modName) { @@ -992,6 +984,15 @@ async function startServer() { process.on('SIGINT', shutdown); // ctrl + c process.on('SIGHUP', shutdown); // terminal closed + config.databaseDirectory = args.databaseDirectory || config.databaseDirectory || "./database"; + await fs.ensureDir(config.databaseDirectory); + + const masterModFolder = path.join(config.databaseDirectory, "masterMods"); + await fs.ensureDir(masterModFolder); + + // mod downloads + app.use(express.static(masterModFolder)); + // Make sure we're actually going to listen on a port let httpPort = args.masterPort || config.masterPort; let httpsPort = args.sslPort || config.sslPort; From ea2f291025fd428a53f6570a33fdfb54f8a923e8 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 21:35:23 +0200 Subject: [PATCH 08/15] Remove reference to old authentication scheme Remove long comment and commented out code about an old sanity check in /api/place. --- master.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/master.js b/master.js index 94615de0..8c9b8d7c 100644 --- a/master.js +++ b/master.js @@ -475,8 +475,7 @@ app.post("/api/place", authenticate.middleware, function(req, res) { } if(x.instanceID && x.instanceName - && !isNaN(Number(x.count))// This is in no way a proper authentication or anything, its just to make sure everybody are registered as slaves before modifying the cluster (or not, to maintain backwards compat) - /*&& stringUtils.hashCode(slaves[x.unique].rconPassword) == x.unique*/ + && !isNaN(Number(x.count)) && x.name && typeof x.name == "string"){ if(config.logItemTransfers){ From ab2a377ca5eeed0e8c70bcb9f0eeccaad0af237e Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 21:39:09 +0200 Subject: [PATCH 09/15] Remove Prometheus pushgateway code Remove code pushing metrics to a hardcoded Prometheus pushgateway that is no longer in operation. There's no point supporting the use of a pushgateway as there's already a full client implemented and since the master server is not ephemeral or batch job it shouldn't be using a pushgateway in the first place. --- config.json.dist | 1 - config.json.test | 1 - master.js | 7 ------- 3 files changed, 9 deletions(-) diff --git a/config.json.dist b/config.json.dist index d7f6a6b3..9af0ace7 100644 --- a/config.json.dist +++ b/config.json.dist @@ -57,7 +57,6 @@ "mirrorAllChat": false, "__comment7": "Autosave interval for items in the master storage in milliseconds. Defaults to one minute when set to 0 or not set.", "autosaveInterval": 600000, - "disablePrometheusPushgateway": false, "__comment8": "This setting only affects the master. It makes every import for things that arent 'electricity' return a maximum of 0. Effectively disables teleport chests.", "disableImportsOfEverythingExceptElectricity": false } diff --git a/config.json.test b/config.json.test index 3ea35cbd..d3ecb3f7 100644 --- a/config.json.test +++ b/config.json.test @@ -42,6 +42,5 @@ "enableCrossServerShout": true, "mirrorAllChat": false, "autosaveInterval": 600000, - "disablePrometheusPushgateway": false, "disableImportsOfEverythingExceptElectricity": false } diff --git a/master.js b/master.js index 8c9b8d7c..c74e6934 100644 --- a/master.js +++ b/master.js @@ -82,13 +82,6 @@ const prometheusPrefix = "clusterio_"; const Prometheus = require('prom-client'); const expressPrometheus = require('express-prometheus-request-metrics'); Prometheus.collectDefaultMetrics({ timeout: 10000 }); // collects RAM usage etc every 10 s -if(!config.disablePrometheusPushgateway){ - const pushgateway = new Prometheus.Pushgateway('http://hme.danielv.no:9091'); - setInterval(() => { - registerMoreMetrics(); - pushgateway.push({ jobName: 'clusterio', groupings: {instance: config.publicIP + ":" + config.masterPort, owner: config.username}}, function(err, resp, body) {}) - }, 15000) -} // collect express request durations ms app.use(expressPrometheus(Prometheus)); From 940a7b9180443648b3983829609b15876b4e723a Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 22:47:35 +0200 Subject: [PATCH 10/15] Refactor getFactorioLocale spec Fix the incorrect error validation, be more lenient about empty values and convert the checks to use async functions instead of error prone callbacks. Remove excessive getFactorioLocale checks in master.spec. --- lib/getFactorioLocale.spec.js | 114 +++++++++++++++++----------------- master.spec.js | 15 +---- 2 files changed, 59 insertions(+), 70 deletions(-) diff --git a/lib/getFactorioLocale.spec.js b/lib/getFactorioLocale.spec.js index 084ce986..a9ba1f3b 100644 --- a/lib/getFactorioLocale.spec.js +++ b/lib/getFactorioLocale.spec.js @@ -1,74 +1,74 @@ const assert = require("assert"); -const getFactorioLocale = require("./getFactorioLocale"); +const util = require("util"); -describe("getFactorioLocale.js", () => { - describe("asObject()", () => { - it("requires 2 strings and a function as parameters", () => { - let validate = (err) => { - return err == "Error: wrong parameters provided" +const getFactorioLocale = require("./getFactorioLocale"); +const asObjectAsync = util.promisify(getFactorioLocale.asObject); + +describe("getFactorioLocale.js", function() { + describe("asObject()", function() { + it("requires 2 strings and a function as parameters", function() { + function validate(err) { + return err.message == "Error: wrong parameters provided" } let message = "getFactorioLocale is responding incorrectly to invalid arguments" let callback = (err,object) => true; - + assert.throws(() => getFactorioLocale.asObject(), validate, message); assert.throws(() => getFactorioLocale.asObject("hello", "I forgot my callback"), validate, message); assert.throws(() => getFactorioLocale.asObject(undefined, undefined, callback), validate, message); - + // the proper way to do it - assert.doesNotThrow(() => getFactorioLocale.asObject("factorio_0.15.27", "en", callback), validate, message); + getFactorioLocale.asObject("factorio_0.15.27", "en", callback); }); - it("returns a nested object with base game locale information", (done) => { - getFactorioLocale.asObject("factorio", "en", callback); - - function callback(err, object){ - assert.equal(typeof object, "object"); - assert.equal(object["entity-name"]["fish"], "Fish"); - assert.equal(object["entity-name"]["small-lamp"], "Lamp"); - Object.keys(object).forEach(key => { - // first level of the nested object is always an object - assert.equal(typeof object[key], "object"); - - Object.keys(object[key]).forEach(key2 => { - // second level of the nested object is always a string, nearly always truthy - if(key2 != "so-long-and-thanks-for-all-the-fish"){ - assert.ok(object[key][key2]); - assert.equal(typeof object[key][key2], "string"); - } - }); + it("returns a nested object with base game locale information", async function() { + let object = await asObjectAsync("factorio", "en"); + + assert.equal(typeof object, "object"); + assert.equal(object["entity-name"]["fish"], "Fish"); + assert.equal(object["entity-name"]["small-lamp"], "Lamp"); + let empty_keys = 0; + Object.keys(object).forEach(key => { + // first level of the nested object is always an object + assert.equal(typeof object[key], "object"); + + Object.keys(object[key]).forEach(key2 => { + // second level of the nested object is always a string, nearly always truthy + if (!object[key][key2]) { + empty_keys++; + } + assert.equal(typeof object[key][key2], "string"); }); - done(); - } + + }); + assert(empty_keys < 10, `Got unusually high amount (${empty_keys}) of empty keys`); }); - it("has some duplicate keys", (done) => { - getFactorioLocale.asObject("factorio", "en", callback); - - function callback(err, object){ - let arrayOfKeys = []; - // collect all keys in a single array - Object.keys(object).forEach(key => { - arrayOfKeys.push(key); - Object.keys(object[key]).forEach(key2 => { - arrayOfKeys.push(key2); - }); + it("has some duplicate keys", async function() { + let object = await asObjectAsync("factorio", "en"); + + let arrayOfKeys = []; + // collect all keys in a single array + Object.keys(object).forEach(key => { + arrayOfKeys.push(key); + Object.keys(object[key]).forEach(key2 => { + arrayOfKeys.push(key2); }); - - // check for duplicates - let duplicateKeys = []; - let i = 0; - arrayOfKeys.forEach(key => { - let o = 0; - arrayOfKeys.forEach(key2 => { - if(i != o && key == key2){ - duplicateKeys.push(key); - } - o++ - }); - i++; + }); + + // check for duplicates + let duplicateKeys = []; + let i = 0; + arrayOfKeys.forEach(key => { + let o = 0; + arrayOfKeys.forEach(key2 => { + if(i != o && key == key2){ + duplicateKeys.push(key); + } + o++ }); - // we got 704 but this is a very unexact number - assert(duplicateKeys.length > 10) - done(); - } + i++; + }); + // we got 704 but this is a very unexact number + assert(duplicateKeys.length > 10) }); }); }); \ No newline at end of file diff --git a/master.spec.js b/master.spec.js index 7ad872d5..a7fac60a 100644 --- a/master.spec.js +++ b/master.spec.js @@ -26,22 +26,11 @@ describe('Master server endpoint testing', function() { it('should get the basegame factorio locale', async function() { let res = await get('/api/getFactorioLocale'); let object = res.body; - // test that it is actual factorio locale, copied from getFactorioLocale.spec.js + + // test that it is looks like a factorio locale assert.equal(typeof object, "object"); assert.equal(object["entity-name"]["fish"], "Fish"); assert.equal(object["entity-name"]["small-lamp"], "Lamp"); - Object.keys(object).forEach(key => { - // first level of the nested object is always an object - assert.equal(typeof object[key], "object"); - - Object.keys(object[key]).forEach(key2 => { - // second level of the nested object is always a string, nearly always truthy - if(key2 != "so-long-and-thanks-for-all-the-fish") { - assert.ok(object[key][key2], `${key}.${key2} is falsy (${object[key][key2]})`); - assert.equal(typeof object[key][key2], "string"); - } - }); - }); }); }); // describe("#GET /api/") From a39a17b220aa483c11c673447587a42b6eaf842c Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 23:09:59 +0200 Subject: [PATCH 11/15] Throw Error objects in inventoryImports/interfacingFunctions Change throwing of strings objcts to throw proper Error objects. --- sharedPlugins/inventoryImports/interfacingFunctions.js | 4 ++-- sharedPlugins/inventoryImports/interfacingFunctions.spec.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sharedPlugins/inventoryImports/interfacingFunctions.js b/sharedPlugins/inventoryImports/interfacingFunctions.js index b5ed13ff..3f7f2903 100644 --- a/sharedPlugins/inventoryImports/interfacingFunctions.js +++ b/sharedPlugins/inventoryImports/interfacingFunctions.js @@ -11,7 +11,7 @@ function parseJsString(string){ // since eval is unsafe, maybe use https://www.npmjs.com/package/eval-sanitizer if(string.includes("require") || string.includes(";") || string.includes("eval")){ - throw "parseJsString might have gotten something that could be a xss attempt"; + throw new Error("parseJsString might have gotten something that could be a xss attempt"); } let inventory = ""; eval("inventory = " + string); @@ -119,7 +119,7 @@ function insertItemsFromObject(confirmedItems, playerName){ // we gotta construct a string because I can't find any nice library for this if(playerName && confirmedItems){ if(typeof playerName != "string") { - throw "playerName is: '"+typeof playerName+"' instead of string!"; + throw new Error("playerName is: '"+typeof playerName+"' instead of string!"); } let itemTable = "{"; Object.keys(confirmedItems).forEach(function(name){ diff --git a/sharedPlugins/inventoryImports/interfacingFunctions.spec.js b/sharedPlugins/inventoryImports/interfacingFunctions.spec.js index e8237f71..31fa3af7 100644 --- a/sharedPlugins/inventoryImports/interfacingFunctions.spec.js +++ b/sharedPlugins/inventoryImports/interfacingFunctions.spec.js @@ -141,7 +141,7 @@ describe("inventoryImports/interfacingFunctions.js", function(){ it("Throws if player name is not a string", function(){ assert.throws(function(){ let x = functions.insertItemsFromObject(testObject, testObject); - }, "playerName is: 'object' instead of string!"); + }, Error("playerName is: 'object' instead of string!")); }); it("Returns a big factorio command as string", function(){ let x = functions.insertItemsFromObject(testObject, playerName); From 5a9e2a8825ab571beff7687d95c970bdab25d2d7 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 23:12:13 +0200 Subject: [PATCH 12/15] Fix inventoryImports/interfacingFunctions test Fix test in inventoryImports/interfacingFunctions breaking due to masterAuthSecret not being defined in the config. --- sharedPlugins/inventoryImports/interfacingFunctions.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/sharedPlugins/inventoryImports/interfacingFunctions.spec.js b/sharedPlugins/inventoryImports/interfacingFunctions.spec.js index 31fa3af7..17eb4996 100644 --- a/sharedPlugins/inventoryImports/interfacingFunctions.spec.js +++ b/sharedPlugins/inventoryImports/interfacingFunctions.spec.js @@ -14,6 +14,7 @@ describe("inventoryImports/interfacingFunctions.js", function(){ let config = { masterIP:"localhost", masterPort:8080, + masterAuthToken:"", } it("returns false if given invalid input", function(){ assert(functions.handleInventory("this is a string") === false, "when given strings it should return false and do nothing"); From fb2519cb50dba8d16aa061a2918b9ede5cec5e30 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 23 Aug 2019 23:35:24 +0200 Subject: [PATCH 13/15] Add fast-test and fast-cover npm scripts Add alternate test and coverage scripts that skips tests that are slow. --- lib/factorioServer/index.spec.js | 2 ++ lib/generateSSLcert.spec.js | 1 + package.json | 2 ++ 3 files changed, 5 insertions(+) diff --git a/lib/factorioServer/index.spec.js b/lib/factorioServer/index.spec.js index 1caf2afb..ce62386f 100644 --- a/lib/factorioServer/index.spec.js +++ b/lib/factorioServer/index.spec.js @@ -73,6 +73,7 @@ describe("class factorioServer/index.js", ()=>{ }); describe(".createMap(mapName.zip)", ()=>{ it("creates a map file as instanceDirectory/saves/map.zip", async function(){ + /* istanbul ignore if */ if (process.env.FAST_TEST) this.skip(); this.timeout(20000); // these test often takes a lot of time in the map creation step (4500 ms on i3 dev system) let relativePath = path.relative('.', __dirname); let server = new factorioServer({ @@ -91,6 +92,7 @@ describe("class factorioServer/index.js", ()=>{ return true; }); it("supports custom filenames", async function(){ + /* istanbul igoner if */ if (process.env.FAST_TEST) this.skip(); this.timeout(20000); let relativePath = path.relative('.', __dirname); let server = new factorioServer({ diff --git a/lib/generateSSLcert.spec.js b/lib/generateSSLcert.spec.js index b8e9306d..6f69efae 100644 --- a/lib/generateSSLcert.spec.js +++ b/lib/generateSSLcert.spec.js @@ -6,6 +6,7 @@ var generateSSLcert = require("lib/generateSSLcert.js") describe("generateSSLcert.js(options)", ()=>{ it("Creates a folder with a .crt and .key file in it", async function() { + /* istanbul ignore if */ if (process.env.FAST_TEST) this.skip(); let certPath = "lib/certTestPlsDelete/cert.crt"; let privKeyPath = "lib/certTestPlsDelete/cert.key"; assert(!await fs.exists(certPath)); // if it already exsits, the test might pass because of leftover data (which is bad) diff --git a/package.json b/package.json index 118ca713..c53760a2 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,9 @@ "main": "master.js", "scripts": { "test": "mocha --exit --check-leaks -R spec \"./{,!(node_modules)/**/}*.spec.js\" || true", + "fast-test": "FAST_TEST=y mocha --check-leaks --exit -R spec \"./{,!(node_modules)/**/}*.spec.js\" || true", "cover": "nyc npm run-script test", + "fast-cover": "nyc npm run-script fast-test", "ci-cover": "nyc --reporter=lcovonly npm run-script test", "start": "echo Run node client.js start [instance] to start slaves && node master.js", "install": "node lib/npmPostinstall.js", From 9c9b4463a4953f393f652827e8368bdf3b3af4a5 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sat, 24 Aug 2019 11:35:57 +0200 Subject: [PATCH 14/15] Fix nested headers in static/nodes Fix instance names being nested in both h2 and h5 headers, which is not allowed in html. --- static/nodes.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/nodes.html b/static/nodes.html index e544dafe..a34632eb 100644 --- a/static/nodes.html +++ b/static/nodes.html @@ -28,7 +28,7 @@
-

${slave.instanceName}

+ ${slave.instanceName}
    From 3b08b73c371c7e838756054b43899ed14f3d5428 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sat, 24 Aug 2019 11:55:19 +0200 Subject: [PATCH 15/15] Remove unused function parseJsString Remove unused and dangerous parseJsString function from inventoryImports. --- .../inventoryImports/interfacingFunctions.js | 17 --------------- .../interfacingFunctions.spec.js | 21 +------------------ 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/sharedPlugins/inventoryImports/interfacingFunctions.js b/sharedPlugins/inventoryImports/interfacingFunctions.js index 3f7f2903..4b4736c6 100644 --- a/sharedPlugins/inventoryImports/interfacingFunctions.js +++ b/sharedPlugins/inventoryImports/interfacingFunctions.js @@ -3,22 +3,6 @@ const needle = require("needle"); const pluginConfig = require("./config") const isJson = require("./isJson") -// returns JS object, throws if there are syntax errors in input string -function parseJsString(string){ - // eval because my lua is writing a JS object instead of traditional JSON - // this was done because I hate escaping strings and there are more quotes - // in JSON than JS objects. - - // since eval is unsafe, maybe use https://www.npmjs.com/package/eval-sanitizer - if(string.includes("require") || string.includes(";") || string.includes("eval")){ - throw new Error("parseJsString might have gotten something that could be a xss attempt"); - } - let inventory = ""; - eval("inventory = " + string); - /*console.log(string) - console.log(inventory)*/ - return inventory; -} function handleInventory(json, config, callback){ /* json is one of these two samples @@ -151,7 +135,6 @@ function pollInventories(outputFile){ module.exports = { handleInventory: handleInventory, - parseJsString: parseJsString, pollInventories: pollInventories, insertItemsFromObject: insertItemsFromObject, } \ No newline at end of file diff --git a/sharedPlugins/inventoryImports/interfacingFunctions.spec.js b/sharedPlugins/inventoryImports/interfacingFunctions.spec.js index 17eb4996..3b1da5a4 100644 --- a/sharedPlugins/inventoryImports/interfacingFunctions.spec.js +++ b/sharedPlugins/inventoryImports/interfacingFunctions.spec.js @@ -157,23 +157,4 @@ describe("inventoryImports/interfacingFunctions.js", function(){ assert(isFactorioCommand(x) == true); }); }); - describe("parseJsString(string) => object", function(){ - it("converts a string with JS object notation to object", function(){ - let x = functions.parseJsString("{hello:'thisAString!', yes:function(){return 'JS string!'}, not:'json'}"); - assert(!!x == true, "Should under no circumstance return a falsey value, even !!{} is truthy"); - assert(typeof x == "object", "parseJsString should return an object in this case"); - assert(typeof x.yes == "function", "parseJsString is not JSON, so it should be able to contain functions"); - }); - it("*Tries* to avoid XSS attempts by throwing on certain symbols", function(){ - assert.throws(function(){ - let x = functions.parseJsString("{};throw 'fail'"); - }, function(err) { - let error = err+"" - /* istanbul ignore else */ - if(error.includes('parseJsString might have gotten something that could be a xss attempt')){ - return true; - } - }, "should throw when ; are passed because those are the simplest form of xss"); - }); - }); -}); \ No newline at end of file +});