diff --git a/packages/lib/file_ops.js b/packages/lib/file_ops.js index c04077be..cbed8df5 100644 --- a/packages/lib/file_ops.js +++ b/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, }; diff --git a/packages/slave/src/Slave.js b/packages/slave/src/Slave.js index 4c28b605..c8b85f57 100644 --- a/packages/slave/src/Slave.js +++ b/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; diff --git a/test/lib/file_ops.js b/test/lib/file_ops.js index 21823231..416c90a0 100644 --- a/test/lib/file_ops.js +++ b/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_"); + }); + }); + }); diff --git a/test/slave.js b/test/slave.js index 36772faa..0cb23498 100644 --- a/test/slave.js +++ b/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");