Merge pull request #215 from Hornwitser/various-fixes

Various unrelated fixes and improvements

.gitignore

@@ -65,3 +65,4 @@ sharedPlugins/*/DISABLED
 
 # IDE
 .idea/
+.vs/

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
 ----------------

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) {

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
 }

config.json.test

@@ -42,6 +42,5 @@
 	"enableCrossServerShout": true,
 	"mirrorAllChat": false,
 	"autosaveInterval": 600000,
-	"disablePrometheusPushgateway": false,
 	"disableImportsOfEverythingExceptElectricity": false
 }

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({

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)

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)
 		});
 	});
 });

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()))

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;

master.js

@@ -14,14 +14,9 @@ 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));
 
-// 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");
@@ -35,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");
@@ -85,21 +76,12 @@ 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_";
 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));
@@ -284,9 +266,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] = {};
 		}
@@ -362,7 +341,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) {
@@ -489,8 +468,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){
@@ -998,6 +976,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;
@@ -1048,5 +1035,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();
+	});
 }

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/")

package.json

@@ -5,8 +5,10 @@
   "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",
+    "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",

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 "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
@@ -119,7 +103,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){
@@ -151,7 +135,6 @@ function pollInventories(outputFile){
 
 module.exports = {
 	handleInventory: handleInventory,
-	parseJsString: parseJsString,
 	pollInventories: pollInventories,
 	insertItemsFromObject: insertItemsFromObject,
 }

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");
@@ -141,7 +142,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);
@@ -156,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");
-		});
-	});
-});
+});

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;
 	}

static/nav.html

@@ -13,7 +13,7 @@
     </div>
     <%# 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

static/nodes.html

@@ -28,7 +28,7 @@
 						<div id="${id}" class="slaveBox card mb-3">
                             <div class="card-body">
                                 <h5 class="card-title"><a href='nodeDetails?instanceID=${id}'>
-                                    <h2>${slave.instanceName}</h2>
+                                    ${slave.instanceName}
                                 </a></h5>
                             </div>
                                 <ul class="list-group list-group-flush">