From 7072b94355e1a8608a18079e402163fa41ce57d5 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 26 Sep 2022 00:34:25 +0200 Subject: [PATCH] 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 | 3 ++- packages/lib/data/ModInfo.js | 16 +++++++++------- packages/lib/data/version.js | 10 ++++++++++ packages/master/src/ControlConnection.js | 12 ++++++------ packages/master/src/Master.js | 20 ++++++++++---------- packages/master/src/routes.js | 5 ++--- test/integration/clusterio.js | 1 - 7 files changed, 39 insertions(+), 28 deletions(-) diff --git a/packages/ctl/src/commands.js b/packages/ctl/src/commands.js index 3ee812c7..f4a31510 100644 --- a/packages/ctl/src/commands.js +++ b/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) { diff --git a/packages/lib/data/ModInfo.js b/packages/lib/data/ModInfo.js index ab2a3a50..4ad3873b 100644 --- a/packages/lib/data/ModInfo.js +++ b/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, diff --git a/packages/lib/data/version.js b/packages/lib/data/version.js index ff533ee8..bda52be7 100644 --- a/packages/lib/data/version.js +++ b/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, }; diff --git a/packages/master/src/ControlConnection.js b/packages/master/src/ControlConnection.js index 6e9cd9a2..48834a90 100644 --- a/packages/master/src/ControlConnection.js +++ b/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); diff --git a/packages/master/src/Master.js b/packages/master/src/Master.js index abc67bcc..6175ff46 100644 --- a/packages/master/src/Master.js +++ b/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); } diff --git a/packages/master/src/routes.js b/packages/master/src/routes.js index 0318bcea..e50d5caa 100644 --- a/packages/master/src/routes.js +++ b/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()); diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index f1ecbb1c..2751ab87 100644 --- a/test/integration/clusterio.js +++ b/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`, );