From a9cd47dc3f4bb27ced10d4fb7a5c85f62f7b3371 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 19 Jun 2018 18:26:34 -0400
Subject: [PATCH] Avoid hang on invalid attachment during import

E.g., if a .pdf is really an HTML file, we try to load it in a hidden
browser (because we properly detect the content type), but then the .pdf
extension causes the hidden browser to launch it via the OS and the
hidden browser never finishes loading it. This adds a 5-second timeout
to abort the process.
---
 chrome/content/zotero/xpcom/attachments.js | 126 ++++++++++++---------
 1 file changed, 73 insertions(+), 53 deletions(-)

diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
index 1f3e3533ea..075463ca09 100644
--- a/chrome/content/zotero/xpcom/attachments.js
+++ b/chrome/content/zotero/xpcom/attachments.js
@@ -1550,63 +1550,83 @@ Zotero.Attachments = new function(){
 		}
 		
 		// Otherwise, load in a hidden browser to get the charset, and then index the document
-		var deferred = Zotero.Promise.defer();
-		var browser = Zotero.Browser.createHiddenBrowser(
-			null,
-			// Disable JavaScript, since it can cause imports that include HTML files to hang
-			// (from network requests that fail?)
-			{ allowJavaScript: false }
-		);
-		
-		if (item.attachmentCharset) {
-			var onpageshow = function(){
-				// ignore spurious about:blank loads
-				if(browser.contentDocument.location.href == "about:blank") return;
-				
-				browser.removeEventListener("pageshow", onpageshow, false);
-				
-				Zotero.Fulltext.indexDocument(browser.contentDocument, itemID)
-				.then(deferred.resolve, deferred.reject)
-				.finally(function () {
-					Zotero.Browser.deleteHiddenBrowser(browser);
-				});
-			};
-			browser.addEventListener("pageshow", onpageshow, false);
-		}
-		else {
-			let callback = Zotero.Promise.coroutine(function* (charset, args) {
-				// ignore spurious about:blank loads
-				if(browser.contentDocument.location.href == "about:blank") return;
-				
-				try {
-					if (charset) {
-						charset = Zotero.CharacterSets.toCanonical(charset);
-						if (charset) {
-							item.attachmentCharset = charset;
-							yield item.saveTx({
-								skipNotifier: true
-							});
-						}
+		return new Zotero.Promise(function (resolve, reject) {
+			var browser = Zotero.Browser.createHiddenBrowser(
+				null,
+				// Disable JavaScript, since it can cause imports that include HTML files to hang
+				// (from network requests that fail?)
+				{ allowJavaScript: false }
+			);
+			
+			var pageshown = false;
+			
+			if (item.attachmentCharset) {
+				var onpageshow = async function () {
+					// ignore spurious about:blank loads
+					if(browser.contentDocument.location.href == "about:blank") return;
+					
+					pageshown = true;
+					
+					browser.removeEventListener("pageshow", onpageshow, false);
+					
+					try {
+						await Zotero.Fulltext.indexDocument(browser.contentDocument, itemID);
+						resolve();
 					}
+					catch (e) {
+						reject(e);
+					}
+					finally {
+						Zotero.Browser.deleteHiddenBrowser(browser);
+					}
+				};
+				browser.addEventListener("pageshow", onpageshow, false);
+			}
+			else {
+				let callback = async function (charset, args) {
+					// ignore spurious about:blank loads
+					if(browser.contentDocument.location.href == "about:blank") return;
 					
-					yield Zotero.Fulltext.indexDocument(browser.contentDocument, item.id);
-					Zotero.Browser.deleteHiddenBrowser(browser);
+					pageshown = true;
 					
-					deferred.resolve();
+					try {
+						if (charset) {
+							charset = Zotero.CharacterSets.toCanonical(charset);
+							if (charset) {
+								item.attachmentCharset = charset;
+								await item.saveTx({
+									skipNotifier: true
+								});
+							}
+						}
+						
+						await Zotero.Fulltext.indexDocument(browser.contentDocument, item.id);
+						resolve();
+					}
+					catch (e) {
+						reject(e);
+					}
+					finally {
+						Zotero.Browser.deleteHiddenBrowser(browser);
+					}
+				};
+				Zotero.File.addCharsetListener(browser, callback, item.id);
+			}
+			
+			var url = Components.classes["@mozilla.org/network/protocol;1?name=file"]
+						.getService(Components.interfaces.nsIFileProtocolHandler)
+						.getURLSpecFromFile(file);
+			browser.loadURI(url);
+			
+			// Avoid a hang if a pageshow is never called on the hidden browser (which can happen
+			// if a .pdf file is really HTML, which can also result in the file being launched,
+			// which we should try to fix)
+			setTimeout(function () {
+				if (!pageshown) {
+					reject(new Error("pageshow not called in hidden browser"));
 				}
-				catch (e) {
-					deferred.reject(e);
-				}
-			});
-			Zotero.File.addCharsetListener(browser, callback, item.id);
-		}
-		
-		var url = Components.classes["@mozilla.org/network/protocol;1?name=file"]
-					.getService(Components.interfaces.nsIFileProtocolHandler)
-					.getURLSpecFromFile(file);
-		browser.loadURI(url);
-		
-		return deferred.promise;
+			}, 5000);
+		});
 	});
 	
 	/**