Fix file upload error when remote attachment has no stored hash

chrome/content/zotero/xpcom/storage/zfs.js

@@ -374,8 +374,10 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 		}
 		body = body.join('&');
 		
+		var req;
+		while (true) {
 			try {
-			var req = yield this.apiClient.makeRequest(
+				req = yield this.apiClient.makeRequest(
 					"POST",
 					uri,
 					{
@@ -398,10 +400,24 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 				throw e;
 			}
 			
-		var result = yield this._handleUploadAuthorizationFailure(req, item);
+			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;
+			}
+		}
 		
 		try {
 			var json = JSON.parse(req.responseText);
@@ -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({

test/tests/zfsTest.js

@@ -817,8 +817,76 @@ 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();
 			var zfs = new Zotero.Sync.Storage.Mode.ZFS({