diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index f5ce4ad3..507bae82 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -499,7 +499,7 @@ export default class Host extends lib.Link { let instancesDir = this.config.get("host.instances_directory"); for (let i = 0; i < 10; i++) { // Limit attempts in case this is somehow an infinite loop - let candidateDir = path.join(instancesDir, await lib.findUnusedName(instancesDir, name)); + let candidateDir = path.join(instancesDir, i === 0 ? name : `${name}-${i + 1}`); try { await fs.mkdir(candidateDir); } catch (err: any) { @@ -1064,6 +1064,32 @@ export default class Host extends lib.Link { await this.sendSaveListUpdate(instanceId, path.join(instance.path, "saves")); } + async _moveOrCopySave(sourcePath: string, targetPath: string, copy: boolean) { + const { dir, name, ext } = path.parse(targetPath); + for (let i = 0; i < 10; i++) { + targetPath = path.format({ dir, name: i === 0 ? name : `${name}-${i + 1}`, ext }); + try { + if (copy) { + await fs.copyFile( + sourcePath, + targetPath, + fs.constants.COPYFILE_EXCL | fs.constants.COPYFILE_FICLONE + ); + } else { + // Unlike rename, link fails if the target exists. + await fs.link(sourcePath, targetPath); + await fs.unlink(sourcePath); + } + return path.basename(targetPath); + } catch (err: any) { + if (err.code !== "EEXIST") { + throw err; + } + } + } + throw new lib.RequestError("Unable to transfer save, retry threshold reached"); + } + async handleInstanceTransferSaveRequest(request: lib.InstanceTransferSaveRequest) { let { sourceName, targetName, copy, sourceInstanceId, targetInstanceId } = request; checkRequestSaveName(sourceName); @@ -1073,29 +1099,19 @@ export default class Host extends lib.Link { // For consistency with remote transfer initiated through pullSave the // target is renamed if it already exists. - targetName = await lib.findUnusedName( - path.join(targetInstance.path, "saves"), targetName, ".zip" - ); - try { - if (copy) { - await fs.copyFile( - path.join(sourceInstance.path, "saves", sourceName), - path.join(targetInstance.path, "saves", targetName), - fs.constants.COPYFILE_FICLONE - ); - } else { - await fs.rename( - path.join(sourceInstance.path, "saves", sourceName), - path.join(targetInstance.path, "saves", targetName), - ); - } + targetName = await this._moveOrCopySave( + path.join(sourceInstance.path, "saves", sourceName), + path.join(targetInstance.path, "saves", targetName), + copy, + ); } catch (err: any) { if (err.code === "ENOENT") { throw new lib.RequestError(`${sourceName} does not exist`); } throw err; } + await this.sendSaveListUpdate(sourceInstanceId, path.join(sourceInstance.path, "saves")); await this.sendSaveListUpdate(targetInstanceId, path.join(targetInstance.path, "saves")); diff --git a/packages/lib/src/file_ops.ts b/packages/lib/src/file_ops.ts index 4b3028ca..de8b30c4 100644 --- a/packages/lib/src/file_ops.ts +++ b/packages/lib/src/file_ops.ts @@ -70,48 +70,7 @@ export async function directorySize(directory: string) { } /** - * Modifies name in case it already exisist at the given directory - * - * Checks the directory passed if it already contains a file or folder with - * the given name, if it does not returns name unmodified, otherwise it - * returns a modified name that does not exist in the directory. - * - * Warning: this function should not be relied upon to be accurate for - * security sensitive applications. The selection process is inherently - * racy and a file or folder may have been created in the folder by the time - * this function returns. - * - * @param directory - directory to check in. - * @param name - file name to check for, may have extension. - * @param extension - dot extension used for the file name. - * @returns modified name with extension that likely does - * not exist in the folder - */ -export async function findUnusedName(directory: string, name: string, extension = "") { - if (extension && name.endsWith(extension)) { - name = name.slice(0, -extension.length); - } - - while (true) { - try { - await fs.access(path.join(directory, `${name}${extension}`)); - } catch (err: any) { - if (err.code === "ENOENT") { - return `${name}${extension}`; - } - } - - let match = /^(.*?)(-(\d+))?$/.exec(name)!; - if (!match[2]) { - name = `${match[1]}-2`; - } else { - name = `${match[1]}-${Number.parseInt(match[3], 10) + 1}`; - } - } -} - -/** - * Generate collision resistante temporary file name + * Generate collision resistant temporary file name * * Warning: this function should not be relied upon to be accurate for * security sensitive applications. The selection process is inherently @@ -125,8 +84,7 @@ export async function findUnusedName(directory: string, name: string, extension */ export async function getTempFile(prefix = "tmp.", suffix = "", tmpdir = "./") { let fileName = path.join(prefix + crypto.randomBytes(16).toString("hex") + suffix); - let freeFile = await findUnusedName(tmpdir, fileName); - let fullPath = path.join(tmpdir, freeFile); + let fullPath = path.join(tmpdir, fileName); return fullPath; } diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index 08ee24b7..b6ed3627 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -1469,6 +1469,16 @@ describe("Integration of Clusterio", function() { await deleteSave(pri, "transfer.zip"); await deleteSave(sec, "transfer.zip"); }); + it("should auto-rename copy if target exists", async function() { + await uploadSave(pri, "transfer.zip", "transfer.zip"); + await uploadSave(sec, "transfer.zip", "transfer.zip"); + await execCtlProcess(`instance save transfer ${pri} transfer.zip ${sec} --copy`); + await assert.doesNotReject(fs.access(path.join(priSaves, "transfer.zip")), "save not at pri"); + await assert.doesNotReject(fs.access(path.join(secSaves, "transfer-2.zip")), "save not at sec"); + await deleteSave(pri, "transfer.zip"); + await deleteSave(sec, "transfer.zip"); + await deleteSave(sec, "transfer-2.zip"); + }); it("should fail if save does not exist", async function() { await assert.rejects(execCtl(`instance save transfer ${pri} not-here.zip ${sec}`)); }); diff --git a/test/lib/file_ops.js b/test/lib/file_ops.js index dfaf905f..3e8dd21f 100644 --- a/test/lib/file_ops.js +++ b/test/lib/file_ops.js @@ -17,13 +17,6 @@ describe("lib/file_ops", function() { await fs.writeFile(path.join(baseDir, "test", "file.txt"), "contents"); await fs.writeFile(path.join(baseDir, "test", "another file.txt"), "more contents"); - await fs.mkdir(path.join(baseDir, "find"), { recursive: true }); - await fs.writeFile(path.join(baseDir, "find", "file"), "contents"); - await fs.writeFile(path.join(baseDir, "find", "file.txt"), "contents"); - await fs.writeFile(path.join(baseDir, "find", "foo-1"), "contents"); - await fs.writeFile(path.join(baseDir, "find", "foo-2"), "contents"); - await fs.writeFile(path.join(baseDir, "find", "bar-1.txt"), "contents"); - await fs.writeFile(path.join(baseDir, "find", "bar-2.txt"), "contents"); } before(setupTestingEnv); @@ -58,34 +51,6 @@ describe("lib/file_ops", function() { }); }); - describe("findUnusedName()", function() { - it("should return named unchanged if it does not exist", async function() { - let cases = [ - [["file"], "file"], - [["file", ".txt"], "file.txt"], - [["file.txt", ".txt"], "file.txt"], - ]; - for (let [args, expected] of cases) { - let actual = await lib.findUnusedName(path.join(baseDir, "test", "folder"), ...args); - assert.equal(actual, expected); - } - }); - it("should return changed name if it does exist", async function() { - let cases = [ - [["file"], "file-2"], - [["file", ".txt"], "file-2.txt"], - [["file.txt", ".txt"], "file-2.txt"], - [["foo-1"], "foo-3"], - [["bar-1", ".txt"], "bar-3.txt"], - [["bar-1.txt", ".txt"], "bar-3.txt"], - ]; - for (let [args, expected] of cases) { - let actual = await lib.findUnusedName(path.join(baseDir, "find"), ...args); - assert.equal(actual, expected); - } - }); - }); - describe("safeOutputFile()", function() { async function assertNoTemporary(target) { let { dir, name } = path.parse(target);