From 8614e73aa8aa14afed2cc439e1d278707cc4d9ad Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Fri, 7 Aug 2020 18:18:31 -0400
Subject: [PATCH] Throw clearer error when 'storage' is a broken symlink

Instead of '(NS_ERROR_FILE_ALREADY_EXISTS) [nsIFile.create]', throw
"Broken symlink at <path>".

Closes #1834
---
 chrome/content/zotero/xpcom/file.js | 49 ++++++++++++++++++++++++-----
 test/tests/fileTest.js              | 32 +++++++++++++++++++
 2 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js
index b53fe14e4b..c4a80770b0 100644
--- a/chrome/content/zotero/xpcom/file.js
+++ b/chrome/content/zotero/xpcom/file.js
@@ -984,24 +984,59 @@ Zotero.File = new function(){
 	this.createDirectoryIfMissing = function (dir) {
 		dir = this.pathToFile(dir);
 		if (!dir.exists() || !dir.isDirectory()) {
-			if (dir.exists() && !dir.isDirectory()) {
-				dir.remove(null);
+			if (dir.exists()) {
+				if (!dir.isDirectory()) {
+					dir.remove(null);
+				}
+			}
+			else {
+				let isSymlink = false;
+				// isSymlink() fails if the directory doesn't exist, but is true if it's a broken
+				// symlink, in which case exists() returns false
+				try {
+					isSymlink = dir.isSymlink();
+				}
+				catch (e) {}
+				if (isSymlink) {
+					throw new Error(`Broken symlink at ${dir.path}`);
+				}
 			}
 			dir.create(Components.interfaces.nsIFile.DIRECTORY_TYPE, 0o755);
 		}
 	}
 	
 	
-	this.createDirectoryIfMissingAsync = function (path) {
-		return Zotero.Promise.resolve(
-			OS.File.makeDir(
+	this.createDirectoryIfMissingAsync = async function (path) {
+		try {
+			await OS.File.makeDir(
 				path,
 				{
-					ignoreExisting: true,
+					ignoreExisting: false,
 					unixMode: 0o755
 				}
 			)
-		);
+		}
+		catch (e) {
+			// If there's a broken symlink at the given path, makeDir() will throw becauseExists,
+			// but exists() will return false
+			if (e.becauseExists) {
+				if (await OS.File.exists(path)) {
+					return;
+				}
+				let isSymlink = false;
+				// Confirm with nsIFile that it's a symlink
+				try {
+					isSymlink = this.pathToFile(path).isSymlink();
+				}
+				catch (e) {
+					Zotero.logError(e);
+				}
+				if (isSymlink) {
+					throw new Error(`Broken symlink at ${path}`);
+				}
+			}
+			throw e;
+		}
 	}
 	
 	
diff --git a/test/tests/fileTest.js b/test/tests/fileTest.js
index db0cf461ce..1cb024d686 100644
--- a/test/tests/fileTest.js
+++ b/test/tests/fileTest.js
@@ -239,6 +239,38 @@ describe("Zotero.File", function () {
 		})
 	})
 	
+	describe("#createDirectoryIfMissing()", function () {
+		it("should throw error on broken symlink", async function () {
+			if (Zotero.isWin) {
+				this.skip();
+			};
+			
+			var tmpPath = await getTempDirectory();
+			var destPath = OS.Path.join(tmpPath, 'missing');
+			var linkPath = OS.Path.join(tmpPath, 'link');
+			await OS.File.unixSymLink(destPath, linkPath);
+			
+			assert.throws(() => Zotero.File.createDirectoryIfMissing(linkPath), /^Broken symlink/);
+		});
+	});
+	
+	describe("#createDirectoryIfMissingAsync()", function () {
+		it("should throw error on broken symlink", async function () {
+			if (Zotero.isWin) {
+				this.skip();
+			};
+			
+			var tmpPath = await getTempDirectory();
+			var destPath = OS.Path.join(tmpPath, 'missing');
+			var linkPath = OS.Path.join(tmpPath, 'link');
+			await OS.File.unixSymLink(destPath, linkPath);
+			
+			var e = await getPromiseError(Zotero.File.createDirectoryIfMissingAsync(linkPath));
+			assert.ok(e);
+			assert.match(e.message, /^Broken symlink/);
+		});
+	});
+	
 	describe("#zipDirectory()", function () {
 		it("should compress a directory recursively", function* () {
 			var tmpPath = Zotero.getTempDirectory().path;