From 0d934c846fab99dde5d223f5b2c4a78a25b91bcc Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sat, 15 Mar 2025 22:44:24 -0400
Subject: [PATCH] Fix file downloads timing out after 30 seconds

We switched file downloads to use XHR to fix downloads failing via
authenticated proxies in fx115+, but that made them subject to our
default 30-second timeout in `Zotero.HTTP.request()`. That timeout uses
the XHR `timeout` property, which applies to the whole request, even if
data is actively being downloaded. As a result, file downloads would
fail for people downloading large files or on very slow connections.

This implements manual connect and inactivity timeouts when using
`Zotero.HTTP.download()`. Currently, these both use `options.timeout`,
or the default 30 seconds, but we could probably take separate options
and have lower defaults now that they no longer apply to the whole
request.
---
 chrome/content/zotero/xpcom/http.js | 84 +++++++++++++++++++++++++----
 1 file changed, 75 insertions(+), 9 deletions(-)

diff --git a/chrome/content/zotero/xpcom/http.js b/chrome/content/zotero/xpcom/http.js
index e8b0b13fc8..29deb7593b 100644
--- a/chrome/content/zotero/xpcom/http.js
+++ b/chrome/content/zotero/xpcom/http.js
@@ -389,15 +389,54 @@ Zotero.HTTP = new function() {
 			// Convert numbers to string to make Sinon happy
 			xmlhttp.setRequestHeader(header, value);
 		}
-
-		// Set timeout
+		
+		const defaultTimeout = 30000;
+		let requestTimeout;
+		let connectTimeout;
+		let inactivityTimeout;
 		if (options.timeout !== 0) {
-			xmlhttp.timeout = options.timeout || 30000;
+			// For downloads, manually implement connect and inactivity timeouts, since the XHR
+			// `timeout` property applies to the whole request, even if data is being downloaded
+			if (options.isDownload) {
+				// TODO: Try a lower default connect timeout and take a separate option?
+				connectTimeout = options.timeout || defaultTimeout;
+				inactivityTimeout = options.timeout || defaultTimeout;
+			}
+			else {
+				requestTimeout = options.timeout || defaultTimeout;
+			}
+		}
+		let connectTimerID = null;
+		let inactivityTimerID = null;
+		let timedOutAfter;
+		
+		function clearConnectTimer() {
+			if (connectTimerID) {
+				clearTimeout(connectTimerID);
+				connectTimerID = null;
+			}
+		}
+		
+		function resetInactivityTimer() {
+			clearTimeout(inactivityTimerID);
+			inactivityTimerID = setTimeout(() => {
+				Zotero.warn(`Inactivity timeout for ${method} ${dispURL} -- aborting request`);
+				timedOutAfter = inactivityTimeout;
+				xmlhttp.abort();
+			}, inactivityTimeout);
+		}
+		
+		if (requestTimeout) {
+			xmlhttp.timeout = requestTimeout;
+			xmlhttp.ontimeout = function() {
+				deferred.reject(new Zotero.HTTP.TimeoutException(requestTimeout));
+			};
+		}
+		else if (inactivityTimeout) {
+			xmlhttp.onprogress = function () {
+				resetInactivityTimer();
+			};
 		}
-
-		xmlhttp.ontimeout = function() {
-			deferred.reject(new Zotero.HTTP.TimeoutException(options.timeout));
-		};
 		
 		// Provide caller with a callback to cancel a request in progress
 		if (options.cancellerReceiver) {
@@ -411,7 +450,17 @@ Zotero.HTTP = new function() {
 			});
 		}
 		
+		if (connectTimeout || inactivityTimeout) {
+			xmlhttp.onloadstart = () => {
+				clearConnectTimer();
+				resetInactivityTimer();
+			};
+		}
+		
 		xmlhttp.onloadend = async function() {
+			clearConnectTimer();
+			clearTimeout(inactivityTimerID);
+			
 			var status = redirectStatus || xmlhttp.status;
 			
 			try {
@@ -513,6 +562,11 @@ Zotero.HTTP = new function() {
 				}
 				Zotero.debug(msg, 1);
 				
+				if (timedOutAfter) {
+					deferred.reject(new Zotero.HTTP.TimeoutException(timedOutAfter));
+					return;
+				}
+				
 				if (xmlhttp.status == 0) {
 					try {
 						this.checkSecurity(channel, { isProxyAuthRequest: options.isProxyAuthRequest });
@@ -538,19 +592,30 @@ Zotero.HTTP = new function() {
 		}
 		
 		// Send binary data
+		let body;
 		if (compressedBody) {
 			let numBytes = compressedBody.length;
 			let ui8Data = new Uint8Array(numBytes);
 			for (let i = 0; i < numBytes; i++) {
 				ui8Data[i] = compressedBody.charCodeAt(i) & 0xff;
 			}
-			xmlhttp.send(ui8Data);
+			body = ui8Data;
 		}
 		// Send regular request
 		else {
-			xmlhttp.send(options.body || null);
+			body = options.body || null;
 		}
 		
+		if (connectTimeout) {
+			connectTimerID = setTimeout(() => {
+				Zotero.warn(`Connect timeout for ${method} ${dispURL} -- aborting request`);
+				timedOutAfter = connectTimeout;
+				xmlhttp.abort();
+			}, connectTimeout);
+		}
+		
+		xmlhttp.send(body);
+		
 		return deferred.promise;
 	};
 	
@@ -598,6 +663,7 @@ Zotero.HTTP = new function() {
 			uri,
 			{
 				...options,
+				isDownload: true,
 				responseType: 'blob',
 				// Downloads can have channel notification callbacks, etc., so always do them for real
 				noMock: true