From 7eeb666894ebedcdda816a9bba79767072611ffc Mon Sep 17 00:00:00 2001
From: Abe Jellinek <jellinek@berkeley.edu>
Date: Wed, 30 Apr 2025 01:06:40 -0400
Subject: [PATCH] Skip invalid relative path when changing LABD (#5239)

---
 .../preferences/preferences_advanced.js       | 49 +++++++------------
 test/tests/preferences_advancedTest.js        | 24 +++++++++
 2 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/chrome/content/zotero/preferences/preferences_advanced.js b/chrome/content/zotero/preferences/preferences_advanced.js
index 0eab089f58..3531df6e28 100644
--- a/chrome/content/zotero/preferences/preferences_advanced.js
+++ b/chrome/content/zotero/preferences/preferences_advanced.js
@@ -593,7 +593,7 @@ Zotero_Preferences.Attachment_Base_Directory = {
 	},
 	
 	
-	changePath: Zotero.Promise.coroutine(function* (basePath) {
+	changePath: async function (basePath) {
 		Zotero.debug(`New base directory is ${basePath}`);
 		
 		if (Zotero.File.directoryContains(Zotero.DataDirectory.dir, basePath)) {
@@ -605,38 +605,37 @@ Zotero_Preferences.Attachment_Base_Directory = {
 			);
 		}
 		
-		// Find all current attachments with relative attachment paths
-		var sql = "SELECT itemID FROM itemAttachments WHERE linkMode=? AND path LIKE ?";
-		var params = [
-			Zotero.Attachments.LINK_MODE_LINKED_FILE,
-			Zotero.Attachments.BASE_PATH_PLACEHOLDER + "%"
-		];
-		var oldRelativeAttachmentIDs = yield Zotero.DB.columnQueryAsync(sql, params);
-		
-		//Find all attachments on the new base path
+		// Find all attachments on the new base path
 		var sql = "SELECT itemID FROM itemAttachments WHERE linkMode=?";
 		var params = [Zotero.Attachments.LINK_MODE_LINKED_FILE];
-		var allAttachments = yield Zotero.DB.columnQueryAsync(sql, params);
+		var allAttachments = await Zotero.DB.columnQueryAsync(sql, params);
 		var newAttachmentPaths = {};
 		var numNewAttachments = 0;
 		var numOldAttachments = 0;
-		for (let i=0; i<allAttachments.length; i++) {
-			let attachmentID = allAttachments[i];
+		for (let attachmentID of allAttachments) {
 			let attachmentPath;
-			let relPath = false
+			let relPath;
 			
 			try {
-				let attachment = yield Zotero.Items.getAsync(attachmentID);
+				let attachment = await Zotero.Items.getAsync(attachmentID);
 				// This will return FALSE for relative paths if base directory
 				// isn't currently set
 				attachmentPath = attachment.getFilePath();
 				// Get existing relative path
 				let storedPath = attachment.attachmentPath;
 				if (storedPath.startsWith(Zotero.Attachments.BASE_PATH_PLACEHOLDER)) {
-					relPath = storedPath.substr(Zotero.Attachments.BASE_PATH_PLACEHOLDER.length);
+					relPath = storedPath.substring(Zotero.Attachments.BASE_PATH_PLACEHOLDER.length);
 					// Use platform-specific slashes, which PathUtils.joinRelative() requires below
 					relPath = Zotero.Attachments.fixPathSlashes(relPath);
 				}
+
+				// If a file with the same relative path exists within the new base directory,
+				// don't touch the attachment, since it will continue to work
+				if (await IOUtils.exists(PathUtils.joinRelative(basePath, relPath))) {
+					Zotero.debug(`${relPath} found within new base path -- skipping`);
+					numNewAttachments++;
+					continue;
+				}
 			}
 			catch (e) {
 				// Don't deal with bad attachment paths. Just skip them.
@@ -644,16 +643,6 @@ Zotero_Preferences.Attachment_Base_Directory = {
 				continue;
 			}
 			
-			// If a file with the same relative path exists within the new base directory,
-			// don't touch the attachment, since it will continue to work
-			if (relPath) {
-				if (yield IOUtils.exists(PathUtils.joinRelative(basePath, relPath))) {
-					Zotero.debug(`${relPath} found within new base path -- skipping`);
-					numNewAttachments++;
-					continue;
-				}
-			}
-			
 			// Files within the new base directory need to be updated to use
 			// relative paths (or, if the new base directory is an ancestor or
 			// descendant of the old one, new relative paths)
@@ -674,7 +663,7 @@ Zotero_Preferences.Attachment_Base_Directory = {
 			}
 		}
 		
-		//Confirm change of the base path
+		// Confirm change of the base path
 		var ps = Services.prompt;
 		
 		var chooseStrPrefix = 'attachmentBasePath.chooseNewPath.';
@@ -730,7 +719,7 @@ Zotero_Preferences.Attachment_Base_Directory = {
 		Zotero.Prefs.set('saveRelativeAttachmentPath', true);
 		// Resave all attachments on base path (so that their paths become relative)
 		// and all other relative attachments (so that their paths become absolute)
-		yield Zotero.Utilities.Internal.forEachChunkAsync(
+		await Zotero.Utilities.Internal.forEachChunkAsync(
 			Object.keys(newAttachmentPaths),
 			100,
 			function (chunk) {
@@ -747,12 +736,12 @@ Zotero_Preferences.Attachment_Base_Directory = {
 							skipDateModifiedUpdate: true
 						});
 					}
-				})
+				});
 			}
 		);
 		
 		return true;
-	}),
+	},
 	
 	
 	clearPath: Zotero.Promise.coroutine(function* () {
diff --git a/test/tests/preferences_advancedTest.js b/test/tests/preferences_advancedTest.js
index f158815b30..2256da10c1 100644
--- a/test/tests/preferences_advancedTest.js
+++ b/test/tests/preferences_advancedTest.js
@@ -128,6 +128,30 @@ describe("Advanced Preferences", function () {
 					Zotero.Attachments.BASE_PATH_PLACEHOLDER + 'test.png'
 				);
 			});
+
+			it("should ignore attachment with invalid relative path", async function () {
+				var file = getTestDataDirectory();
+				file.append('test.pdf');
+				file = file.path;
+				
+				var attachment = createUnsavedDataObject('item', { itemType: 'attachment' });
+				attachment.attachmentLinkMode = Zotero.Attachments.LINK_MODE_LINKED_FILE;
+				attachment.attachmentPath = 'attachments:/test.pdf'; // Invalid
+				await attachment.saveTx();
+
+				var basePath = getTestDataDirectory().path;
+				await setBaseDirectory(basePath);
+
+				var newBasePath = await getTempDirectory();
+				await IOUtils.copy(file, PathUtils.joinRelative(newBasePath, 'test.pdf'));
+
+				await setBaseDirectory(newBasePath);
+
+				assert.equal(
+					attachment.attachmentPath,
+					Zotero.Attachments.BASE_PATH_PLACEHOLDER + '/test.pdf'
+				);
+			});
 		})
 	})
 })