renameAttachmentFile: Check rename() return value

And actually return -1 if it returns false.

Before this fix, attempting to rename an attachment file to a name that already
exists on disk would never return -1 as the docs say it should. Instead:

1. rename() would fail and return false
2. newName would be set to false
3. renameAttachmentFile() would pass false as the second argument to
   OS.Path.join()
4. OS.Path.join() would ignore it because it was falsy and return the attachment
   directory path without any modification
5. relinkAttachmentFile() would be called with path set to the attachment
   directory
6. relinkAttachmentFile() would notice that path's dirname wasn't the attachment
   directory - it was the attachment directory's parent - and attempt to copy it
   and its contents, recursively, into itself, using copyToFollowingLinks()

...which created a directory structure on disk over 100 directories deep -
not deeper only because the OS started returning errors due to paths exceeding
32,767 characters (the limit on my filesystem).
This commit is contained in:
Abe Jellinek 2023-07-27 11:44:36 -04:00
parent 00683732d8
commit 16547f64c2

View file

@ -2801,6 +2801,9 @@ Zotero.Item.prototype.renameAttachmentFile = async function (newName, overwrite
unique
}
);
if (newName === false) {
return -1;
}
let destPath = OS.Path.join(OS.Path.dirname(origPath), newName);
await this.relinkAttachmentFile(destPath);