From c9694e93b02ea3bec41fb17d258b5b69a43a8186 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sun, 22 Jan 2017 15:30:18 -0500
Subject: [PATCH] Fix file upload error when remote attachment has no stored
 hash

---
 chrome/content/zotero/xpcom/storage/zfs.js | 71 +++++++++++++---------
 test/tests/zfsTest.js                      | 70 ++++++++++++++++++++-
 2 files changed, 112 insertions(+), 29 deletions(-)

diff --git a/chrome/content/zotero/xpcom/storage/zfs.js b/chrome/content/zotero/xpcom/storage/zfs.js
index dc6cd1c1f9..7a9d505828 100644
--- a/chrome/content/zotero/xpcom/storage/zfs.js
+++ b/chrome/content/zotero/xpcom/storage/zfs.js
@@ -374,33 +374,49 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 		}
 		body = body.join('&');
 		
-		try {
-			var req = yield this.apiClient.makeRequest(
-				"POST",
-				uri,
-				{
-					body,
-					headers,
-					// This should include all errors in _handleUploadAuthorizationFailure()
-					successCodes: [200, 201, 204, 403, 404, 412, 413],
-					debug: true
-				}
-			);
-		}
-		catch (e) {
-			if (e instanceof Zotero.HTTP.UnexpectedStatusException) {
-				let msg = "Unexpected status code " + e.status + " in " + funcName
-					 + " (" + item.libraryKey + ")";
-				Zotero.logError(msg);
-				Zotero.debug(e.xmlhttp.getAllResponseHeaders());
-				throw new Error(Zotero.Sync.Storage.defaultError);
+		var req;
+		while (true) {
+			try {
+				req = yield this.apiClient.makeRequest(
+					"POST",
+					uri,
+					{
+						body,
+						headers,
+						// This should include all errors in _handleUploadAuthorizationFailure()
+						successCodes: [200, 201, 204, 403, 404, 412, 413],
+						debug: true
+					}
+				);
+			}
+			catch (e) {
+				if (e instanceof Zotero.HTTP.UnexpectedStatusException) {
+					let msg = "Unexpected status code " + e.status + " in " + funcName
+						 + " (" + item.libraryKey + ")";
+					Zotero.logError(msg);
+					Zotero.debug(e.xmlhttp.getAllResponseHeaders());
+					throw new Error(Zotero.Sync.Storage.defaultError);
+				}
+				throw e;
+			}
+			
+			let result = yield this._handleUploadAuthorizationFailure(req, item);
+			if (result instanceof Zotero.Sync.Storage.Result) {
+				return result;
+			}
+			// If remote attachment exists but has no hash (which can happen for an old (pre-4.0?)
+			// attachment with just an mtime, or after a storage purge), send again with If-None-Match
+			else if (result == "ERROR_412_WITHOUT_VERSION") {
+				if (headers["If-None-Match"]) {
+					throw new Error("412 returned for request with If-None-Match");
+				}
+				delete headers["If-Match"];
+				headers["If-None-Match"] = "*";
+				Zotero.debug("Retrying with If-None-Match");
+			}
+			else {
+				break;
 			}
-			throw e;
-		}
-		
-		var result = yield this._handleUploadAuthorizationFailure(req, item);
-		if (result instanceof Zotero.Sync.Storage.Result) {
-			return result;
 		}
 		
 		try {
@@ -468,10 +484,9 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 			throw new Error(Zotero.Sync.Storage.defaultError);
 		}
 		else if (req.status == 412) {
-			Zotero.debug("412 BUT WE'RE COOL");
 			let version = req.getResponseHeader('Last-Modified-Version');
 			if (!version) {
-				throw new Error("Last-Modified-Version header not provided");
+				return "ERROR_412_WITHOUT_VERSION";
 			}
 			if (version > item.version) {
 				return new Zotero.Sync.Storage.Result({
diff --git a/test/tests/zfsTest.js b/test/tests/zfsTest.js
index 14a7a5ce76..271a3be48e 100644
--- a/test/tests/zfsTest.js
+++ b/test/tests/zfsTest.js
@@ -817,7 +817,75 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () {
 			assert.isFalse(result.localChanges);
 			assert.isFalse(result.remoteChanges);
 			assert.isTrue(result.syncRequired);
-		})
+		});
+		
+		it("should retry with If-None-Match on 412 with missing remote hash", function* () {
+			var { engine, client, caller } = yield setup();
+			var zfs = new Zotero.Sync.Storage.Mode.ZFS({
+				apiClient: client
+			})
+			
+			var file = getTestDataDirectory();
+			file.append('test.png');
+			var item = yield Zotero.Attachments.importFromFile({ file });
+			item.version = 5;
+			item.synced = true;
+			item.attachmentSyncedModificationTime = Date.now();
+			item.attachmentSyncedHash = 'bd4c33e03798a7e8bc0b46f8bda74fac'
+			yield item.saveTx();
+			
+			var contentType = 'image/png';
+			var prefix = Zotero.Utilities.randomString();
+			var suffix = Zotero.Utilities.randomString();
+			var uploadKey = Zotero.Utilities.randomString(32, 'abcdef0123456789');
+			
+			var called = 0;
+			server.respond(function (req) {
+				if (req.method == "POST"
+						&& req.url == `${baseURL}users/1/items/${item.key}/file`
+						&& !req.requestBody.includes('upload=')
+						&& req.requestHeaders["If-Match"] == item.attachmentSyncedHash) {
+					called++;
+					req.respond(
+						412,
+						{
+							"Content-Type": "application/json"
+						},
+						"If-Match set but file does not exist"
+					);
+				}
+				else if (req.method == "POST"
+						&& req.url == `${baseURL}users/1/items/${item.key}/file`
+						&& !req.requestBody.includes('upload=')
+						&& req.requestHeaders["If-None-Match"] == "*") {
+					assert.equal(called++, 1)
+					req.respond(
+						200,
+						{
+							"Content-Type": "application/json"
+						},
+						JSON.stringify({
+							url: baseURL + "pretend-s3/1",
+							contentType: contentType,
+							prefix: prefix,
+							suffix: suffix,
+							uploadKey: uploadKey
+						})
+					);
+				}
+			})
+			
+			var stub = sinon.stub(zfs, "_uploadFile");
+			
+			yield zfs._processUploadFile({
+				name: item.libraryKey
+			});
+			
+			assert.equal(called, 2);
+			assert.ok(stub.called);
+			
+			stub.restore();
+		});
 		
 		it("should handle 413 on quota limit", function* () {
 			var { engine, client, caller } = yield setup();