Move checkFilename and cleanFilename to libFileOps

These are also useful for managing mod zip files on the master.

packages/lib/file_ops.js

@@ -108,9 +108,94 @@ async function safeOutputFile(file, data, options={}) {
 	await fs.rename(temporary, file);
 }
 
+/**
+ * Check if a string is a valid file name
+ *
+ * @param {string} name - Name to check
+ * @throws Error if the name is unsuitable.
+ */
+function checkFilename(name) {
+	// All of these are bad in Windows only, except for /, . and ..
+	// See: https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
+	const badChars = /[<>:"\/\\|?*\x00-\x1f]/g;
+	const badEnd = /[. ]$/;
+
+	const oneToNine = [1, 2, 3, 4, 5, 6, 7, 8, 9];
+	const badNames = [
+		// Relative path components
+		".", "..",
+
+		// Reserved filenames in Windows
+		"CON", "PRN", "AUX", "NUL",
+		...oneToNine.map(n => `COM${n}`),
+		...oneToNine.map(n => `LPT${n}`),
+	];
+
+	if (typeof name !== "string") {
+		throw new Error("must be a string");
+	}
+
+	if (name === "") {
+		throw new Error("cannot be empty");
+	}
+
+	if (badChars.test(name)) {
+		throw new Error('cannot contain <>:"\\/|=* or control characters');
+	}
+
+	if (badNames.includes(name.toUpperCase())) {
+		throw new Error(
+			"cannot be named any of . .. CON PRN AUX NUL COM1-9 and LPT1-9"
+		);
+	}
+
+	if (badEnd.test(name)) {
+		throw new Error("cannot end with . or space");
+	}
+}
+
+/**
+ * Clean up string to be suitable for use as filename
+ *
+ * @param {string} name - Arbitrary name string
+ * @returns {string} Filename suitable to use in the filesystem
+ */
+function cleanFilename(name) {
+	// copied from checkFilename
+	const badChars = /[<>:"\/\\|?*\x00-\x1f]/g;
+	const badEnd = /[. ]$/;
+
+	const oneToNine = [1, 2, 3, 4, 5, 6, 7, 8, 9];
+	const badNames = [
+		// Relative path components
+		".", "..",
+
+		// Reserved filenames in Windows
+		"CON", "PRN", "AUX", "NUL",
+		...oneToNine.map(n => `COM${n}`),
+		...oneToNine.map(n => `LPT${n}`),
+	];
+
+	if (typeof name !== "string") {
+		throw new Error("name must be a string");
+	}
+
+	if (name === "" || badNames.includes(name.toUpperCase())) {
+		name += "_";
+	}
+
+	name = name.replace(badChars, "_");
+	name = name.replace(badEnd, "_");
+
+	return name;
+}
+
+
 module.exports = {
 	getNewestFile,
 	findUnusedName,
 	getTempFile,
 	safeOutputFile,
+	checkFilename,
+	cleanFilename,
 };

packages/slave/src/Slave.js

@@ -25,86 +25,9 @@ const InstanceConnection = require("./InstanceConnection");
 const finished = util.promisify(stream.finished);
 
 
-function checkFilename(name) {
-	// All of these are bad in Windows only, except for /, . and ..
-	// See: https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
-	const badChars = /[<>:"\/\\|?*\x00-\x1f]/g;
-	const badEnd = /[. ]$/;
-
-	const oneToNine = [1, 2, 3, 4, 5, 6, 7, 8, 9];
-	const badNames = [
-		// Relative path components
-		".", "..",
-
-		// Reserved filenames in Windows
-		"CON", "PRN", "AUX", "NUL",
-		...oneToNine.map(n => `COM${n}`),
-		...oneToNine.map(n => `LPT${n}`),
-	];
-
-	if (typeof name !== "string") {
-		throw new Error("must be a string");
-	}
-
-	if (name === "") {
-		throw new Error("cannot be empty");
-	}
-
-	if (badChars.test(name)) {
-		throw new Error('cannot contain <>:"\\/|=* or control characters');
-	}
-
-	if (badNames.includes(name.toUpperCase())) {
-		throw new Error(
-			"cannot be named any of . .. CON PRN AUX NUL COM1-9 and LPT1-9"
-		);
-	}
-
-	if (badEnd.test(name)) {
-		throw new Error("cannot end with . or space");
-	}
-}
-
-/**
- * Clean up string to be suitable for use as filename
- *
- * @param {string} name - Arbitrary name string
- * @returns {string} Filename suitable to use in the filesystem
- * @private
- */
-function cleanFilename(name) {
-	// copied from checkFilename
-	const badChars = /[<>:"\/\\|?*\x00-\x1f]/g;
-	const badEnd = /[. ]$/;
-
-	const oneToNine = [1, 2, 3, 4, 5, 6, 7, 8, 9];
-	const badNames = [
-		// Relative path components
-		".", "..",
-
-		// Reserved filenames in Windows
-		"CON", "PRN", "AUX", "NUL",
-		...oneToNine.map(n => `COM${n}`),
-		...oneToNine.map(n => `LPT${n}`),
-	];
-
-	if (typeof name !== "string") {
-		throw new Error("name must be a string");
-	}
-
-	if (name === "" || badNames.includes(name.toUpperCase())) {
-		name += "_";
-	}
-
-	name = name.replace(badChars, "_");
-	name = name.replace(badEnd, "_");
-
-	return name;
-}
-
 function checkRequestSaveName(name) {
 	try {
-		checkFilename(name);
+		libFileOps.checkFilename(name);
 	} catch (err) {
 		throw new libErrors.RequestError(`Save name ${err.message}`);
 	}
@@ -243,9 +166,9 @@ class Slave extends libLink.Link {
 	}
 
 	async _createNewInstanceDir(name) {
-		name = cleanFilename(name);
+		name = libFileOps.cleanFilename(name);
 		try {
-			checkFilename(name);
+			libFileOps.checkFilename(name);
 		} catch (err) {
 			throw new Error(`Instance folder was unepectedly invalid: name ${err.message}`);
 		}
@@ -837,8 +760,5 @@ ${err.stack}`
 module.exports = Slave;
 
 // For testing only
-module.exports._checkFilename = checkFilename;
-module.exports._cleanFilename = cleanFilename;
 module.exports._discoverInstances = discoverInstances;
-module.exports._Slave = Slave;
 

test/lib/file_ops.js

@@ -84,4 +84,101 @@ describe("lib/file_ops", function() {
 			assert.equal(await fs.readFile(target, "utf8"), "current");
 		});
 	});
+
+	describe("checkFilename()", function() {
+		it("should allow a basic name", function() {
+			libFileOps.checkFilename("file");
+		});
+
+		function check(item, msg) {
+			assert.throws(() => libFileOps.checkFilename(item), new Error(msg));
+		}
+
+		it("should throw on non-string", function() {
+			check(undefined, "must be a string");
+			check(null, "must be a string");
+			check({}, "must be a string");
+			check([], "must be a string");
+			check(0, "must be a string");
+			check(false, "must be a string");
+		});
+
+		it("should throw on empty name", function() {
+			check("", "cannot be empty");
+		});
+
+		it("should throw on <>:\"\\/|?* \\x00\\r\\n\\t", function() {
+			for (let char of '<>:"\\/|?*\x00\r\n\t') {
+				check(char, 'cannot contain <>:"\\/|=* or control characters');
+			}
+		});
+
+		it("should throw on CON, PRN, AUX, NUL, COM1, LPT1", function() {
+			for (let bad of ["CON", "PRN", "AUX", "NUL", "COM1", "LPT1"]) {
+				check(bad, "cannot be named any of . .. CON PRN AUX NUL COM1-9 and LPT1-9");
+			}
+		});
+
+		it("should throw on . and ..", function() {
+			for (let bad of [".", ".."]) {
+				check(bad, "cannot be named any of . .. CON PRN AUX NUL COM1-9 and LPT1-9");
+			}
+		});
+
+		it("should throw on names ending with . or space", function() {
+			check("a ", "cannot end with . or space");
+			check("a.", "cannot end with . or space");
+		});
+	});
+
+	describe("cleanFilename()", function() {
+		function clean(item, expected) {
+			assert.equal(libFileOps.cleanFilename(item), expected);
+		}
+
+		it("should allow a basic name", function() {
+			clean("file", "file");
+		});
+
+		function check(item, msg) {
+			assert.throws(() => libFileOps.cleanFilename(item), new Error(msg));
+		}
+
+		it("should throw on non-string", function() {
+			check(undefined, "name must be a string");
+			check(null, "name must be a string");
+			check({}, "name must be a string");
+			check([], "name must be a string");
+			check(0, "name must be a string");
+			check(false, "name must be a string");
+		});
+
+		it("should clean empty name", function() {
+			clean("", "_");
+		});
+
+		it("should clean <>:\"\\/|?* \\x00\\r\\n\\t", function() {
+			for (let char of '<>:"\\/|?*\x00\r\n\t') {
+				clean(char, "_");
+			}
+		});
+
+		it("should clean CON, PRN, AUX, NUL, COM1, LPT1", function() {
+			for (let bad of ["CON", "PRN", "AUX", "NUL", "COM1", "LPT1"]) {
+				clean(bad, `${bad}_`);
+			}
+		});
+
+		it("should clean . and ..", function() {
+			for (let bad of [".", ".."]) {
+				clean(bad, `${bad}_`);
+			}
+		});
+
+		it("should clean names ending with . or space", function() {
+			clean("a ", "a_");
+			clean("a.", "a_");
+		});
+	});
+
 });

test/slave.js

@@ -13,102 +13,6 @@ describe("Slave testing", function() {
 		libConfig.InstanceConfig.finalize();
 	});
 
-	describe("checkFilename()", function() {
-		it("should allow a basic name", function() {
-			Slave._checkFilename("file");
-		});
-
-		function check(item, msg) {
-			assert.throws(() => Slave._checkFilename(item), new Error(msg));
-		}
-
-		it("should throw on non-string", function() {
-			check(undefined, "must be a string");
-			check(null, "must be a string");
-			check({}, "must be a string");
-			check([], "must be a string");
-			check(0, "must be a string");
-			check(false, "must be a string");
-		});
-
-		it("should throw on empty name", function() {
-			check("", "cannot be empty");
-		});
-
-		it("should throw on <>:\"\\/|?* \\x00\\r\\n\\t", function() {
-			for (let char of '<>:"\\/|?*\x00\r\n\t') {
-				check(char, 'cannot contain <>:"\\/|=* or control characters');
-			}
-		});
-
-		it("should throw on CON, PRN, AUX, NUL, COM1, LPT1", function() {
-			for (let bad of ["CON", "PRN", "AUX", "NUL", "COM1", "LPT1"]) {
-				check(bad, "cannot be named any of . .. CON PRN AUX NUL COM1-9 and LPT1-9");
-			}
-		});
-
-		it("should throw on . and ..", function() {
-			for (let bad of [".", ".."]) {
-				check(bad, "cannot be named any of . .. CON PRN AUX NUL COM1-9 and LPT1-9");
-			}
-		});
-
-		it("should throw on names ending with . or space", function() {
-			check("a ", "cannot end with . or space");
-			check("a.", "cannot end with . or space");
-		});
-	});
-
-	describe("cleanFilename()", function() {
-		function clean(item, expected) {
-			assert.equal(Slave._cleanFilename(item), expected);
-		}
-
-		it("should allow a basic name", function() {
-			clean("file", "file");
-		});
-
-		function check(item, msg) {
-			assert.throws(() => Slave._cleanFilename(item), new Error(msg));
-		}
-
-		it("should throw on non-string", function() {
-			check(undefined, "name must be a string");
-			check(null, "name must be a string");
-			check({}, "name must be a string");
-			check([], "name must be a string");
-			check(0, "name must be a string");
-			check(false, "name must be a string");
-		});
-
-		it("should clean empty name", function() {
-			clean("", "_");
-		});
-
-		it("should clean <>:\"\\/|?* \\x00\\r\\n\\t", function() {
-			for (let char of '<>:"\\/|?*\x00\r\n\t') {
-				clean(char, "_");
-			}
-		});
-
-		it("should clean CON, PRN, AUX, NUL, COM1, LPT1", function() {
-			for (let bad of ["CON", "PRN", "AUX", "NUL", "COM1", "LPT1"]) {
-				clean(bad, `${bad}_`);
-			}
-		});
-
-		it("should clean . and ..", function() {
-			for (let bad of [".", ".."]) {
-				clean(bad, `${bad}_`);
-			}
-		});
-
-		it("should clean names ending with . or space", function() {
-			clean("a ", "a_");
-			clean("a.", "a_");
-		});
-	});
-
 	describe("symlinkMods()", function() {
 		let testDir = path.join("temp", "test", "symlink");