Derive mod filename from name and version

Enforce the filename of uploaded mods match {name}_{version}.zip with
name and version taken from the zip file's info.json entry.  This allows
it to be used as a unique key for the mod and solves part of the issues
with mods having different number of leading zeros in their version.

packages/ctl/src/commands.js

@@ -1204,7 +1204,8 @@ modCommands.add(new libCommand.Command({
 		}
 
 		if (result.body.mods && result.body.mods.length) {
-			logger.info(`Successfully uploaded ${result.body.mods[0].filename}`);
+			const mod = new libData.ModInfo(result.body.mods[0]);
+			logger.info(`Successfully uploaded ${mod.filename}`);
 		}
 
 		if ((result.body.errors || []).length || (result.body.request_errors || []).length) {

packages/lib/data/ModInfo.js

@@ -7,7 +7,7 @@ const libHash = require("../hash");
 const libSchema = require("../schema");
 const { findRoot } = require("../zip_ops");
 
-const { integerModVersion, integerFactorioVersion } = require("./version");
+const { integerModVersion, integerFactorioVersion, modVersionRegExp } = require("./version");
 
 
 /**
@@ -99,10 +99,12 @@ class ModInfo {
 	dependencies = ["base"];
 
 	/**
-	 * Name of the file containing this mod.
+	 * Expected name of zip file containing this mod.
 	 * @type {number}
 	 */
-	filename = "";
+	get filename() {
+		return `${this.name}_${this.version}.zip`;
+	}
 
 	/**
 	 * Size of the mod in bytes
@@ -145,7 +147,6 @@ class ModInfo {
 		type: "object",
 		properties: {
 			...this.infoJsonSchema.properties,
-			"filename": { type: "string" },
 			"size": { type: "integer" },
 			"sha1": { type: "string" },
 			"is_deleted": { type: "boolean" },
@@ -167,7 +168,6 @@ class ModInfo {
 		if (json.dependencies) { this.dependencies = json.dependencies; }
 
 		// Additional data
-		if (json.filename) { this.filename = json.filename; }
 		if (json.size) { this.size = json.size; }
 		if (json.sha1) { this.sha1 = json.sha1; }
 		if (json.is_deleted) { this.isDeleted = json.is_deleted; }
@@ -187,7 +187,6 @@ class ModInfo {
 		if (this.dependencies.length !== 1 || this.dependencies[0] !== "base") {
 			json.dependencies = this.dependencies;
 		}
-		if (this.filename) { json.filename = this.filename; }
 		if (this.size) { json.size = this.size; }
 		if (this.sha1) { json.sha1 = this.sha1; }
 		if (this.isDeleted) { json.is_deleted = this.isDeleted; }
@@ -215,10 +214,13 @@ class ModInfo {
 			throw new Error("Mod's info.json is not valid");
 		}
 
+		if (!modVersionRegExp.test(modInfo.version)) {
+			throw new Error(`Mod's version (${modInfo.version}) is invalid`);
+		}
+
 		return new this({
 			...modInfo,
 
-			filename: path.basename(modPath),
 			size: (await fs.stat(modPath)).size,
 			sha1: await libHash.hashFile(modPath),
 			is_deleted: false,

packages/lib/data/version.js

@@ -9,6 +9,7 @@
  * number of leading zeros in them.
  * @param {string} version - string representation of version.
  * @returns {number} numeric representation.
+ * @private
  */
 function integerModVersion(version) {
 	const [major, minor, sub] = version.split(".").map(n => Number.parseInt(n, 10));
@@ -22,13 +23,22 @@ function integerModVersion(version) {
  * number of leading zeros in them.
  * @param {string} version - string representation of version.
  * @returns {number} numeric representation.
+ * @private
  */
 function integerFactorioVersion(version) {
 	const [major, minor, sub] = version.split(".").map(n => Number.parseInt(n, 10));
 	return major * 0x100000000 + minor * 0x10000 + (sub || 0); // Can't use bitwise here because this is 48-bits.
 }
 
+/**
+ * Matches valid mod versions.
+ * @type {RegExp}
+ * @private
+ */
+const modVersionRegExp = /^\d+\.\d+\.\d+$/;
+
 module.exports = {
 	integerModVersion,
 	integerFactorioVersion,
+	modVersionRegExp,
 };

packages/master/src/ControlConnection.js

@@ -531,10 +531,10 @@ class ControlConnection extends BaseConnection {
 
 	async getModRequestHandler(message) {
 		let { name, version } = message.data;
-		let key = `${name}_${version}`;
-		let mod = this._master.mods.get(key);
+		let filename = `${name}_${version}.zip`;
+		let mod = this._master.mods.get(filename);
 		if (!mod) {
-			throw new libErrors.RequestError(`Mod ${key} does not exist`);
+			throw new libErrors.RequestError(`Mod ${filename} does not exist`);
 		}
 		return {
 			mod: mod.toJSON(),
@@ -639,10 +639,10 @@ class ControlConnection extends BaseConnection {
 
 	async downloadModRequestHandler(message) {
 		let { name, version } = message.data;
-		let key = `${name}_${version}`;
-		let mod = this._master.mods.get(key);
+		let filename = `${name}_${version}.zip`;
+		let mod = this._master.mods.get(filename);
 		if (!mod) {
-			throw new libErrors.RequestError(`Mod ${key} does not exist`);
+			throw new libErrors.RequestError(`Mod ${filename} does not exist`);
 		}
 		let modPath = path.join(this._master.config.get("master.mods_directory"), mod.filename);
 

packages/master/src/Master.js

@@ -420,15 +420,15 @@ class Master {
 				continue;
 			}
 
-			let key = `${modInfo.name}_${modInfo.version}`;
-			if (mods.has(key)) {
+			if (modInfo.filename !== entry.name) {
 				logger.error(
-					`Ignoring Mod ${entry.name} as it contains the same name and ` +
-					`version as ${mods.get(key).filename}`
+					`Mod ${entry.name}'s filename does not match the expected ` +
+					`name ${modInfo.filename}, it will be ignored.`
 				);
 				continue;
 			}
-			mods.set(key, modInfo);
+
+			mods.set(modInfo.filename, modInfo);
 		}
 		return mods;
 	}
@@ -454,15 +454,15 @@ class Master {
 	}
 
 	async deleteMod(name, version) {
-		let key = `${name}_${version}`;
-		let mod = this.mods.get(key);
+		let filename = `${name}_${version}.zip`;
+		let mod = this.mods.get(filename);
 		if (!mod) {
-			throw new Error(`Mod ${key} does not exist`);
+			throw new Error(`Mod ${filename} does not exist`);
 		}
 
 		let modsDirectory = this.config.get("master.mods_directory");
-		await fs.unlink(path.join(modsDirectory, mod.filename));
-		this.mods.delete(key);
+		await fs.unlink(path.join(modsDirectory, filename));
+		this.mods.delete(filename);
 		mod.isDeleted = true;
 		this.modUpdated(mod);
 	}

packages/master/src/routes.js

@@ -507,9 +507,8 @@ async function uploadMod(req, res) {
 			await finished(writeStream);
 
 			const modInfo = await libData.ModInfo.fromModFile(path.join(modsDirectory, tempFilename));
-			modInfo.filename = filename;
-			await fs.rename(path.join(modsDirectory, tempFilename), path.join(modsDirectory, filename));
-			req.app.locals.master.mods.set(`${modInfo.name}_${modInfo.version}`, modInfo);
+			await fs.rename(path.join(modsDirectory, tempFilename), path.join(modsDirectory, modInfo.filename));
+			req.app.locals.master.mods.set(modInfo.filename, modInfo);
 			req.app.locals.master.modUpdated(modInfo);
 			mods.push(modInfo.toJSON());
 

test/integration/clusterio.js

@@ -915,7 +915,6 @@ describe("Integration of Clusterio", function() {
 					"description: An empty mod for testing\n" +
 					"factorio_version: 1.1\n" +
 					"dependencies:\n" +
-					"filename: empty_mod_1.0.0.zip\n" +
 					`size: ${stat.size}\n` +
 					`sha1: ${hash}\n`,
 				);