From bccf5ff0b2ed5f3afde168c807abf7da9d84be0c Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 9 Jun 2020 01:17:33 -0400
Subject: [PATCH] Fix endless WebDAV loops if server has wrong mtimes but hash
 matches

Possibly caused by a third-party client uploading mtimes that then
aren't synced, or that differ from what get synced. When we detect this,
try to correct it by updating mtimes on WebDAV and the API to match the
local file.

https://forums.zotero.org/discussion/83554/zotero-loop-syncs-2000-items
---
 chrome/content/zotero/xpcom/storage/webdav.js | 22 +++++--
 test/tests/webdavTest.js                      | 57 ++++++++++++++++---
 2 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/chrome/content/zotero/xpcom/storage/webdav.js b/chrome/content/zotero/xpcom/storage/webdav.js
index 70264119b1..0f8c03aa6a 100644
--- a/chrome/content/zotero/xpcom/storage/webdav.js
+++ b/chrome/content/zotero/xpcom/storage/webdav.js
@@ -446,7 +446,10 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
 						yield item.saveTx({ skipAll: true });
 						// skipAll doesn't mark as unsynced, so do that separately
 						yield item.updateSynced(false);
-						return new Zotero.Sync.Storage.Result;
+						return new Zotero.Sync.Storage.Result({
+							localChanges: true,
+							syncRequired: true
+						});
 					}
 				}
 				
@@ -463,9 +466,20 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
 				if (smtime != mtime) {
 					let shash = item.attachmentSyncedHash;
 					if (shash && metadata.md5 && shash == metadata.md5) {
-						Zotero.debug("Last synced mod time for item " + item.libraryKey
-							+ " doesn't match time on storage server but hash does -- ignoring");
-						return new Zotero.Sync.Storage.Result;
+						Zotero.debug(`Last synced mod time for item ${item.libraryKey} doesn't `
+							+ "match time on storage server but hash does -- using local file mtime");
+						
+						yield this._setStorageFileMetadata(item);
+						item.attachmentSyncedModificationTime = fmtime;
+						item.attachmentSyncState = "in_sync";
+						yield item.saveTx({ skipAll: true });
+						// skipAll doesn't mark as unsynced, so do that separately
+						yield item.updateSynced(false);
+						
+						return new Zotero.Sync.Storage.Result({
+							localChanges: true,
+							syncRequired: true
+						});
 					}
 					
 					Zotero.logError("Conflict -- last synced file mod time for item "
diff --git a/test/tests/webdavTest.js b/test/tests/webdavTest.js
index ddc3c4eb6f..0cf25d466d 100644
--- a/test/tests/webdavTest.js
+++ b/test/tests/webdavTest.js
@@ -517,11 +517,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
 			yield item.saveTx();
 			var mtime = yield item.attachmentModificationTime;
 			var hash = yield item.attachmentHash;
-			var path = item.getFilePath();
-			var filename = 'test.png';
-			var size = (yield OS.File.stat(path)).size;
-			var contentType = 'image/png';
-			var fileContents = yield Zotero.File.getContentsAsync(path);
 			
 			setResponse({
 				method: "GET",
@@ -537,15 +532,61 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
 			
 			assertRequestCount(1);
 			
-			assert.isFalse(result.localChanges);
+			assert.isTrue(result.localChanges);
 			assert.isFalse(result.remoteChanges);
-			assert.isFalse(result.syncRequired);
+			assert.isTrue(result.syncRequired);
 			
 			// Check local object
 			assert.equal(item.attachmentSyncedModificationTime, mtime);
 			assert.equal(item.attachmentSyncedHash, hash);
 			assert.isFalse(item.synced);
-		})
+		});
+		
+		it("should skip upload and update mtimes if synced mtime doesn't match WebDAV mtime but file hash does", async function () {
+			var engine = await setup();
+			
+			var file = OS.Path.join(getTestDataDirectory().path, 'test.png');
+			var item = await Zotero.Attachments.importFromFile({ file });
+			await item.saveTx();
+			var fmtime = await item.attachmentModificationTime;
+			var hash = await item.attachmentHash;
+			
+			var mtime = 123456789000;
+			var mtime2 = 123456799000;
+			item.attachmentSyncedModificationTime = mtime;
+			item.attachmentSyncedHash = hash;
+			item.attachmentSyncState = 'to_upload';
+			item.synced = true;
+			await item.saveTx();
+			
+			setResponse({
+				method: "GET",
+				url: `zotero/${item.key}.prop`,
+				status: 200,
+				text: '<properties version="1">'
+					+ `<mtime>${mtime2}</mtime>`
+					+ `<hash>${hash}</hash>`
+					+ '</properties>'
+			});
+			setResponse({
+				method: "PUT",
+				url: `zotero/${item.key}.prop`,
+				status: 204
+			});
+			
+			var result = await engine.start();
+			
+			assertRequestCount(2);
+			
+			assert.isTrue(result.localChanges);
+			assert.isFalse(result.remoteChanges);
+			assert.isTrue(result.syncRequired);
+			
+			// Check local object
+			assert.equal(item.attachmentSyncedModificationTime, fmtime);
+			assert.equal(item.attachmentSyncedHash, hash);
+			assert.isFalse(item.synced);
+		});
 		
 		
 		// As a security measure, Nextcloud sets a regular cookie and two SameSite cookies and