diff --git a/chrome/content/zotero/xpcom/http.js b/chrome/content/zotero/xpcom/http.js index c73b7d4777..0dde7a325b 100644 --- a/chrome/content/zotero/xpcom/http.js +++ b/chrome/content/zotero/xpcom/http.js @@ -9,7 +9,38 @@ Zotero.HTTP = new function () { var { SecurityInfo } = ChromeUtils.importESModule("resource://gre/modules/SecurityInfo.sys.mjs"); var { NetUtil } = ChromeUtils.importESModule("resource://gre/modules/NetUtil.sys.mjs"); - + + + /** + * Parse a URI (nsIURI or string), extract any embedded credentials, and + * return the URL as a credential-free string + * + * Mozilla percent-encodes periods in the username component of nsIURIs (%2E), which is + * technically valid but breaks Basic auth against most servers, so we undo that here. + * + * @param {nsIURI|String} uri + * @return {{ url: String, username: String|null, password: String|null }} + */ + function _parseURI(uri) { + if (!(uri instanceof Components.interfaces.nsIURI)) { + try { + uri = Services.io.newURI(uri); + } + catch (e) { + return { url: uri, username: null, password: null }; + } + } + let username = uri.username || null; + let password = null; + if (username) { + username = username.replace(/%2E/, '.'); + password = uri.password || null; + uri = uri.mutate().setUserPass('').finalize(); + } + return { url: uri.spec, username, password }; + } + + /** * Exception returned for unexpected status when promise* is used * @constructor @@ -154,72 +185,11 @@ Zotero.HTTP = new function () { * code is received (or a code not in options.successCodes if provided). */ this.request = async function (method, url, options = {}) { - var errorDelayGenerator; - - while (true) { - try { - let req = await this._requestInternal(...arguments); - return req; - } - catch (e) { - if (e instanceof this.UnexpectedStatusException) { - _checkConnection(e.xmlhttp, url); - - if (e.is5xx()) { - Zotero.logError(e); - // Check for Retry-After header on 503 and wait the specified amount of time - if (e.xmlhttp.status == 503 && (await _checkRetry(e.xmlhttp))) { - continue; - } - // Don't retry if errorDelayMax is 0 - if (options.errorDelayMax === 0 || Zotero.HTTP.disableErrorRetry) { - throw e; - } - // Automatically retry other 5xx errors by default - if (!errorDelayGenerator) { - // Keep trying for up to an hour - errorDelayGenerator = Zotero.Utilities.Internal.delayGenerator( - options.errorDelayIntervals || _errorDelayIntervals, - options.errorDelayMax !== undefined - ? options.errorDelayMax - : _errorDelayMax - ); - } - let delayPromise = errorDelayGenerator.next().value; - let keepGoing; - // Provide caller with a callback to cancel while waiting to retry - if (options.cancellerReceiver) { - let resolve; - let reject; - let cancelPromise = new Zotero.Promise((res, rej) => { - resolve = res; - reject = function () { - rej(new Zotero.HTTP.CancelledException); - }; - }); - options.cancellerReceiver(reject); - try { - keepGoing = await Promise.race([delayPromise, cancelPromise]); - } - catch (e) { - Zotero.debug("Request cancelled"); - throw e; - } - resolve(); - } - else { - keepGoing = await delayPromise; - } - if (!keepGoing) { - Zotero.logError("Failed too many times"); - throw e; - } - continue; - } - } - throw e; - } - } + return _retryOnServerError( + () => this._requestInternal(...arguments), + url, + options + ); }; @@ -227,18 +197,8 @@ Zotero.HTTP = new function () { * Most of the logic for request() is here, with request() handling automatic 5xx retries */ this._requestInternal = async function (method, url, options = {}) { - if (url instanceof Components.interfaces.nsIURI) { - // Extract username and password from URI and undo Mozilla's excessive percent-encoding - options.username = url.username || null; - if (options.username) { - options.username = options.username.replace(/%2E/, '.'); - options.password = url.password || null; - url = url.mutate().setUserPass('').finalize(); - } - - url = url.spec; - } - + ({ url, username: options.username, password: options.password } = _parseURI(url)); + var dispURL = url; // Add username:******** to display URL @@ -309,6 +269,7 @@ Zotero.HTTP = new function () { isFile = channel instanceof Components.interfaces.nsIFileChannel; var redirectStatus; var redirectLocation; + var redirectChannel; if(channel instanceof Components.interfaces.nsIHttpChannelInternal) { channel.forceAllowThirdPartyCookie = true; @@ -340,6 +301,10 @@ Zotero.HTTP = new function () { asyncOnChannelRedirect: function (oldChannel, newChannel, flags, callback) { redirectStatus = (flags & Ci.nsIChannelEventSink.REDIRECT_PERMANENT) ? 301 : 302; redirectLocation = newChannel.URI.spec; + try { + redirectChannel = oldChannel.QueryInterface(Ci.nsIHttpChannel); + } + catch (e) {} oldChannel.cancel(Cr.NS_BINDING_ABORTED); callback.onRedirectVerifyCallback(Cr.NS_BINDING_ABORTED); } @@ -392,52 +357,13 @@ Zotero.HTTP = new function () { } const defaultTimeout = 30000; - let requestTimeout; - let connectTimeout; - let inactivityTimeout; if (options.timeout !== 0) { - // 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) { + let requestTimeout = options.timeout || defaultTimeout; xmlhttp.timeout = requestTimeout; xmlhttp.ontimeout = function () { deferred.reject(new Zotero.HTTP.TimeoutException(requestTimeout)); }; } - else if (inactivityTimeout) { - xmlhttp.onprogress = function () { - resetInactivityTimer(); - }; - } // Provide caller with a callback to cancel a request in progress if (options.cancellerReceiver) { @@ -451,17 +377,7 @@ Zotero.HTTP = new function () { }); } - if (connectTimeout || inactivityTimeout) { - xmlhttp.onloadstart = () => { - clearConnectTimer(); - resetInactivityTimer(); - }; - } - xmlhttp.onloadend = async function () { - clearConnectTimer(); - clearTimeout(inactivityTimerID); - var status = redirectStatus || xmlhttp.status; var success; @@ -511,19 +427,18 @@ Zotero.HTTP = new function () { success = status >= 200 && status < 300; } - // Create a fake XMLHttpRequest object for an unfollowed or canceled redirect, since - // the real one won't be accurate. Only `status` and `getResponseHeader('Location')` - // are available. + // Create a fake XMLHttpRequest object for an unfollowed or canceled redirect, + // since the real one won't be accurate if (redirectStatus) { - let channel = xmlhttp.channel; xmlhttp = { status, getResponseHeader: function (header) { if (header.toLowerCase() == 'location') { return redirectLocation; } - Zotero.debug("Warning: Attempt to get response header other than Location " - + "for redirect", 2); + if (redirectChannel) { + return redirectChannel.getResponseHeader(header); + } return null; } }; @@ -575,11 +490,6 @@ 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 }); @@ -619,14 +529,6 @@ Zotero.HTTP = new function () { 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; @@ -663,35 +565,190 @@ Zotero.HTTP = new function () { /** - * Download a file + * Download a file, streaming the response body directly to disk * - * @param {nsIURI|String} url - URL to request + * Uses fetch() + ReadableStream instead of XMLHttpRequest so that the response body is + * streamed to disk chunk by chunk, avoiding the 2 GB IOUtils.write() limit and reducing + * memory pressure for large files. + * + * @param {nsIURI|String} uri - URL to request * @param {String} path - Path to save file to - * @param {Object} [options] - See `Zotero.HTTP.request()` + * @param {Object} [options] + * @param {Object|Headers} [options.headers] - HTTP headers to send with the request + * @param {Boolean} [options.noCache] - Bypass the cache + * @param {Number[]|false} [options.successCodes] - HTTP status codes that are considered + * successful, or FALSE to allow all + * @param {Function} [options.onProgress] - Progress callback (totalBytes, contentLength) + * @param {Function} [options.cancellerReceiver] - Callback to receive a cancel function + * @param {Number} [options.timeout = 30000] - Timeout in milliseconds (connect and + * inactivity); 0 to disable + * @param {Number[]} [options.errorDelayIntervals] - Retry delay intervals for 5xx errors + * @param {Number} [options.errorDelayMax] - Max time to spend retrying 5xx errors + * @return {Promise} - A promise for a fetch Response object */ this.download = async function (uri, path, options = {}) { - // TODO: Convert request() to fetch() and use ReadableStream - var req = await this.request( - 'GET', - uri, - { - ...options, - isDownload: true, - responseType: 'blob', - // Downloads can have channel notification callbacks, etc., so always do them for real - noMock: true - } + let { url, username, password } = _parseURI(uri); + + if (!/^https?:/.test(url)) { + throw new Error("HTTP.download() only supports HTTP(S) URLs -- got " + url); + } + + let dispURL = options.displayURL || url; + if (username) { + dispURL = dispURL.replace(/^(https?:\/\/)/, `$1${username}:********@`); + } + dispURL = dispURL.replace(/key=[^&]+&?/, "").replace(/\?$/, ""); + + Zotero.debug("HTTP GET " + dispURL); + + if (this.browserIsOffline()) { + Zotero.debug(`HTTP GET ${dispURL} failed: ${Zotero.appName} is offline`); + throw new this.BrowserOfflineException(); + } + + return _retryOnServerError( + () => _downloadInternal(url, path, options, { + username, password, dispURL + }), + url, + options ); - if (req.status >= 200 && req.status < 300) { - var bytes = await IOUtils.write(path, await req.response.bytes()); - Zotero.debug(`Saved file to ${path} (${bytes} byte${bytes != 1 ? 's' : ''})`); - } - // Only relevant if status is in successCodes - else { - Zotero.debug("Not saving file for non-2xx response code"); - } - return req; }; + + + /** + * Internal implementation for download() -- streams a file to disk using fetch() + */ + async function _downloadInternal(url, path, options, ctx) { + let defaultTimeout = 30000; + let timeout = options.timeout !== 0 ? (options.timeout || defaultTimeout) : 0; + + // AbortController handles both cancellation and timeouts + let controller = new AbortController(); + let inactivityTimerID; + + function resetInactivityTimer() { + clearTimeout(inactivityTimerID); + if (timeout) { + inactivityTimerID = setTimeout(() => { + Zotero.warn(`Inactivity timeout for GET ${ctx.dispURL}` + + " -- aborting request"); + controller.abort(); + }, timeout); + } + } + + if (options.cancellerReceiver) { + options.cancellerReceiver(() => { + controller.abort(); + }); + } + + // Build fetch options + let fetchOptions = { + signal: controller.signal, + }; + if (options.noCache) { + fetchOptions.cache = 'no-store'; + } + + // Build headers + let headers = new Zotero.HTTP.CasePreservingHeaders(options.headers || {}); + if (ctx.username) { + let encoded = btoa(ctx.username + ':' + (ctx.password || '')); + headers.set('Authorization', `Basic ${encoded}`); + } + fetchOptions.headers = headers; + + // Start the request with a connect timeout + let connectTimerID; + if (timeout) { + connectTimerID = setTimeout(() => { + Zotero.warn(`Connect timeout for GET ${ctx.dispURL} -- aborting request`); + controller.abort(); + }, timeout); + } + + let response; + try { + response = await fetch(url, fetchOptions); + } + catch (e) { + clearTimeout(connectTimerID); + clearTimeout(inactivityTimerID); + if (controller.signal.aborted) { + throw new Zotero.HTTP.TimeoutException(timeout); + } + throw e; + } + clearTimeout(connectTimerID); + + let status = response.status; + + // Check success + let success; + if (options.successCodes) { + success = options.successCodes.includes(status); + } + else if (options.successCodes === false) { + success = true; + } + else { + success = status >= 200 && status < 300; + } + + if (!success) { + let msg = "HTTP GET " + ctx.dispURL + " failed with status code " + status; + Zotero.debug(msg, 1); + throw new Zotero.HTTP.UnexpectedStatusException(response, url, msg); + } + + // For non-2xx success (e.g., 404 in successCodes), don't write a file + if (status < 200 || status >= 300) { + Zotero.debug("HTTP GET " + ctx.dispURL + " finished with " + status); + return response; + } + + // Stream response body to disk + let reader = response.body.getReader(); + let totalBytes = 0; + let contentLength = parseInt(response.headers.get('Content-Length')) || null; + let onProgress = options.onProgress; + + try { + await IOUtils.write(path, new Uint8Array()); + while (true) { + resetInactivityTimer(); + let { done, value } = await reader.read(); + if (done) break; + await IOUtils.write(path, value, { mode: 'append' }); + totalBytes += value.byteLength; + if (onProgress) { + try { + onProgress(totalBytes, contentLength); + } + catch (e) { + Zotero.logError(e); + } + } + } + } + catch (e) { + clearTimeout(inactivityTimerID); + IOUtils.remove(path, { ignoreAbsent: true }).catch(e2 => Zotero.logError(e2)); + if (controller.signal.aborted) { + throw new Zotero.HTTP.TimeoutException(timeout); + } + throw e; + } + clearTimeout(inactivityTimerID); + + Zotero.debug("HTTP GET " + ctx.dispURL + " succeeded with " + status); + Zotero.debug("Saved " + path + " (" + totalBytes + " byte" + + (totalBytes != 1 ? 's' : '') + ")"); + + return response; + } /** @@ -1439,7 +1496,11 @@ Zotero.HTTP = new function () { }; async function _checkRetry(req) { - var retryAfter = req.getResponseHeader("Retry-After"); + // req may be an XMLHttpRequest (from request()) or a fetch Response + // (from download()) + var retryAfter = req.headers?.get + ? req.headers.get("Retry-After") + : req.getResponseHeader("Retry-After"); if (!retryAfter) { return false; } @@ -1451,8 +1512,95 @@ Zotero.HTTP = new function () { await Zotero.Promise.delay(retryAfter * 1000); return true; } - - + + + /** + * Call `fn` and automatically retry on 5xx errors with exponential backoff. + * + * @param {Function} fn - Async function to call (should throw + * UnexpectedStatusException on failure) + * @param {String} url - URL being requested (for _checkConnection) + * @param {Object} options - Must contain errorDelayIntervals, errorDelayMax, + * and cancellerReceiver if applicable + * @return {Promise} - Result of fn() + */ + async function _retryOnServerError(fn, url, options) { + var errorDelayGenerator; + + while (true) { + try { + return await fn(); + } + catch (e) { + if (e instanceof Zotero.HTTP.UnexpectedStatusException) { + _checkConnection(e.xmlhttp, url); + + if (e.is5xx()) { + Zotero.logError(e); + // Check for Retry-After header on 503 + if (e.xmlhttp.status == 503 && (await _checkRetry(e.xmlhttp))) { + continue; + } + // Don't retry if errorDelayMax is 0 + if (options.errorDelayMax === 0 + || Zotero.HTTP.disableErrorRetry) { + throw e; + } + // Automatically retry other 5xx errors by default + if (!errorDelayGenerator) { + // Keep trying for up to an hour + errorDelayGenerator + = Zotero.Utilities.Internal.delayGenerator( + options.errorDelayIntervals + || _errorDelayIntervals, + options.errorDelayMax !== undefined + ? options.errorDelayMax + : _errorDelayMax + ); + } + let delayPromise = errorDelayGenerator.next().value; + let keepGoing; + // Provide caller with a callback to cancel + // while waiting to retry + if (options.cancellerReceiver) { + let resolve; + let reject; + let cancelPromise = new Zotero.Promise( + (res, rej) => { + resolve = res; + reject = function () { + rej(new Zotero.HTTP.CancelledException); + }; + } + ); + options.cancellerReceiver(reject); + try { + keepGoing = await Promise.race( + [delayPromise, cancelPromise] + ); + } + catch (e) { + Zotero.debug("Request cancelled"); + throw e; + } + resolve(); + } + else { + keepGoing = await delayPromise; + } + if (!keepGoing) { + Zotero.logError("Failed too many times"); + throw e; + } + continue; + } + } + throw e; + } + } + } + + /** * Mimics the window.location/document.location interface, given an nsIURL * @param {nsIURL} url diff --git a/chrome/content/zotero/xpcom/storage/webdav.js b/chrome/content/zotero/xpcom/storage/webdav.js index 9db16e4276..263ac3bd52 100644 --- a/chrome/content/zotero/xpcom/storage/webdav.js +++ b/chrome/content/zotero/xpcom/storage/webdav.js @@ -521,10 +521,8 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { { successCodes: [200, 404], noCache: true, - notificationCallbacks: { - onProgress: function (req, progress, progressMax) { - request.onProgress(progress, progressMax) - }, + onProgress(progress, progressMax) { + request.onProgress(progress, progressMax); }, errorDelayIntervals: this.ERROR_DELAY_INTERVALS, errorDelayMax: this.ERROR_DELAY_MAX, diff --git a/chrome/content/zotero/xpcom/storage/zfs.js b/chrome/content/zotero/xpcom/storage/zfs.js index 8b40dc1bc1..970d3c579f 100644 --- a/chrome/content/zotero/xpcom/storage/zfs.js +++ b/chrome/content/zotero/xpcom/storage/zfs.js @@ -79,118 +79,124 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { return new Promise(async (resolve, reject) => { var resultOptions = {}; try { - let req = await Zotero.HTTP.download( + // Step 1: Request file metadata from the API + // + // The API responds with 302 + custom headers + Location pointing to S3, + // or 404 if the file doesn't exist remotely. + let apiReq = await Zotero.HTTP.request( + 'GET', uri, - destPath, { successCodes: [200, 302, 404], headers: this.apiClient.getHeaders(), noCache: true, - notificationCallbacks: { - asyncOnChannelRedirect: async function (oldChannel, newChannel, flags, callback) { - // These will be used in processDownload() if the download succeeds - oldChannel.QueryInterface(Components.interfaces.nsIHttpChannel); - - Zotero.debug(`Handling ${oldChannel.responseStatus} redirect for ${item.libraryKey}`); - Zotero.debug(oldChannel.URI.spec); - Zotero.debug(newChannel.URI.spec); - - var header; - try { - header = "Zotero-File-Modification-Time"; - requestData.mtime = parseInt(oldChannel.getResponseHeader(header)); - header = "Zotero-File-MD5"; - requestData.md5 = oldChannel.getResponseHeader(header); - header = "Zotero-File-Compressed"; - requestData.compressed = oldChannel.getResponseHeader(header) == 'Yes'; - } - catch (_e) { - reject(new Error(`${header} header not set in file request for ${item.libraryKey}`)); - callback.onRedirectVerifyCallback(Cr.NS_ERROR_ABORT); - return; - } - - if (!(await IOUtils.exists(path))) { - callback.onRedirectVerifyCallback(Cr.NS_OK); - return; - } - - var updateHash = false; - var fileModTime = await item.attachmentModificationTime; - if (requestData.mtime == fileModTime) { - Zotero.debug("File mod time matches remote file -- skipping download of " - + item.libraryKey); - } - // If not compressed, check hash, in case only timestamp changed - else if (!requestData.compressed && (await item.attachmentHash) == requestData.md5) { - Zotero.debug("File hash matches remote file -- skipping download of " - + item.libraryKey); - updateHash = true; - } - else { - callback.onRedirectVerifyCallback(Cr.NS_OK); - return; - } - - // Update local metadata and stop request, skipping file download - await OS.File.setDates(path, null, new Date(requestData.mtime)); - item.attachmentSyncedModificationTime = requestData.mtime; - if (updateHash) { - item.attachmentSyncedHash = requestData.md5; - } - item.attachmentSyncState = "in_sync"; - await item.saveTx({ skipAll: true }); - resultOptions.localChanges = true; - - callback.onRedirectVerifyCallback(Cr.NS_ERROR_ABORT); - }, - - onProgress: function (req, progress, progressMax) { - request.onProgress(progress, progressMax); - }, - }, + followRedirects: false, } ); - - if (req.status == 302) { - resolve(new Zotero.Sync.Storage.Result(resultOptions)); - return; - } - - if (req.status == 404) { + + if (apiReq.status == 404) { Zotero.debug("Remote file not found for item " + item.libraryKey); item.attachmentSyncState = "in_sync"; await item.saveTx({ skipAll: true }); - // Don't refresh item pane rows when nothing happened request.skipProgressBarUpdate = true; resolve(new Zotero.Sync.Storage.Result); return; } - + + // Read file metadata from the 302 response headers + var header; + try { + header = "Zotero-File-Modification-Time"; + requestData.mtime = parseInt(apiReq.getResponseHeader(header)); + header = "Zotero-File-MD5"; + requestData.md5 = apiReq.getResponseHeader(header); + header = "Zotero-File-Compressed"; + requestData.compressed = apiReq.getResponseHeader(header) == 'Yes'; + } + catch (_e) { + reject(new Error( + `${header} header not set in file request for ${item.libraryKey}` + )); + return; + } + + let fileURL = apiReq.getResponseHeader('Location'); + + // Check if we can skip the download + if (await IOUtils.exists(path)) { + let skipDownload = false; + let updateHash = false; + let fileModTime = await item.attachmentModificationTime; + if (requestData.mtime == fileModTime) { + Zotero.debug("File mod time matches remote file" + + " -- skipping download of " + item.libraryKey); + skipDownload = true; + } + // If not compressed, check hash, in case only timestamp changed + else if (!requestData.compressed + && (await item.attachmentHash) == requestData.md5) { + Zotero.debug("File hash matches remote file" + + " -- skipping download of " + item.libraryKey); + skipDownload = true; + updateHash = true; + } + + // Update local metadata and stop request, skipping file download + if (skipDownload) { + await OS.File.setDates(path, null, new Date(requestData.mtime)); + item.attachmentSyncedModificationTime = requestData.mtime; + if (updateHash) { + item.attachmentSyncedHash = requestData.md5; + } + item.attachmentSyncState = "in_sync"; + await item.saveTx({ skipAll: true }); + resultOptions.localChanges = true; + resolve(new Zotero.Sync.Storage.Result(resultOptions)); + return; + } + } + + // Step 2: Download the file from storage + let displayURL = fileURL.replace(/(\w:\/\/[^/]+\/).*/, '$1[...]'); + await Zotero.HTTP.download( + fileURL, + destPath, + { + displayURL, + noCache: true, + onProgress(progress, progressMax) { + request.onProgress(progress, progressMax); + }, + } + ); + // Don't try to process if the request has been cancelled if (request.isFinished()) { - Zotero.debug(`Download request ${request.name} is no longer running after file download`, 2); + Zotero.debug(`Download request ${request.name}` + + " is no longer running after file download", 2); resolve(new Zotero.Sync.Storage.Result); return; } - + Zotero.debug("Finished download of " + destPath); - + resolve(await Zotero.Sync.Storage.Local.processDownload(requestData)); } catch (e) { if (e instanceof Zotero.HTTP.UnexpectedStatusException) { - // If S3 connection is interrupted, delay and retry, or bail if too many - // consecutive failures - if (e.xmlhttp.status == 0) { - if (++this._s3ConsecutiveFailures < this._maxS3ConsecutiveFailures) { + // If S3 connection is interrupted, delay and retry + if (e.xmlhttp?.status == 0 || e.status == 0) { + if (++this._s3ConsecutiveFailures + < this._maxS3ConsecutiveFailures) { let libraryKey = item.libraryKey; - let msg = "S3 returned 0 for " + libraryKey + " -- retrying download"; + let msg = "S3 returned 0 for " + libraryKey + + " -- retrying download"; Zotero.logError(msg); if (this._s3Backoff < this._maxS3Backoff) { this._s3Backoff *= 2; } - Zotero.debug("Delaying " + libraryKey + " download for " + Zotero.debug("Delaying " + libraryKey + + " download for " + this._s3Backoff + " seconds", 2); Zotero.Promise.delay(this._s3Backoff * 1000) .then(function () { @@ -198,7 +204,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { }.bind(this)); return; } - + Zotero.debug(this._s3ConsecutiveFailures + " consecutive S3 failures -- aborting", 1); this._s3ConsecutiveFailures = 0; diff --git a/test/tests/httpTest.js b/test/tests/httpTest.js index 872138b66b..e30c8c4c53 100644 --- a/test/tests/httpTest.js +++ b/test/tests/httpTest.js @@ -30,6 +30,7 @@ describe("Zotero.HTTP", function () { { handle: function (request, response) { response.setHeader('Location', redirectLocation); + response.setHeader('X-Custom', 'redirect-value', false); response.setStatusLine(null, 301, "Moved Permanently"); response.write(`\n\n301 Moved Permanently\n\n

Moved Permanently

\n

The document has moved here.

\n`); } @@ -94,6 +95,7 @@ describe("Zotero.HTTP", function () { ); assert.equal(req.status, 301); assert.equal(req.getResponseHeader('Location'), redirectLocation); + assert.equal(req.getResponseHeader('X-Custom'), 'redirect-value'); }); it("should catch an interrupted connection", async function () { @@ -155,7 +157,13 @@ describe("Zotero.HTTP", function () { describe("Retries", function () { var spy; var delayStub; - + + before(async function () { + // Wait for proxy auth probing to finish so its + // Zotero.Promise.delay() calls don't pollute the stub + await Zotero.proxyAuthComplete; + }); + beforeEach(function () { delayStub = sinon.stub(Zotero.Promise, "delay").returns(Promise.resolve()); }); @@ -315,6 +323,213 @@ describe("Zotero.HTTP", function () { }); + describe("#download()", function () { + var tmpDir; + + before(function () { + httpd.registerPathHandler( + '/download/small.bin', + { + handle: function (request, response) { + response.setStatusLine(null, 200, "OK"); + response.setHeader("Content-Type", "application/octet-stream", false); + let data = "abc".repeat(1024); + response.setHeader("Content-Length", String(data.length), false); + response.write(data); + } + } + ); + httpd.registerPathHandler( + '/download/large.bin', + { + handle: function (request, response) { + response.setStatusLine(null, 200, "OK"); + response.setHeader("Content-Type", "application/octet-stream", false); + // 256 KB -- enough to exercise multiple onDataAvailable calls + let chunk = "x".repeat(1024); + for (let i = 0; i < 256; i++) { + response.write(chunk); + } + } + } + ); + httpd.registerPathHandler( + '/download/404', + { + handle: function (request, response) { + response.setStatusLine(null, 404, "Not Found"); + response.write("Not found"); + } + } + ); + httpd.registerPathHandler( + '/download/500', + { + handle: function (request, response) { + response.setStatusLine(null, 500, "Internal Server Error"); + response.write("Server error"); + } + } + ); + httpd.registerPathHandler( + '/download/redirect-to-file', + { + handle: function (request, response) { + response.setStatusLine(null, 302, "Found"); + response.setHeader("Location", baseURL + "download/small.bin", false); + } + } + ); + httpd.registerPathHandler( + '/download/custom-header', + { + handle: function (request, response) { + let val; + try { + val = request.getHeader("X-Custom"); + } + catch (e) { + val = ""; + } + response.setStatusLine(null, 200, "OK"); + response.setHeader("X-Echo", val, false); + response.write("ok"); + } + } + ); + }); + + beforeEach(async function () { + Zotero.HTTP.mock = null; + tmpDir = await getTempDirectory(); + }); + + afterEach(async function () { + await IOUtils.remove(tmpDir, { recursive: true, ignoreAbsent: true }); + }); + + + it("should download a file to disk", async function () { + let dest = PathUtils.join(tmpDir, "small.bin"); + let req = await Zotero.HTTP.download( + baseURL + "download/small.bin", + dest + ); + assert.equal(req.status, 200); + let stat = await IOUtils.stat(dest); + assert.equal(stat.size, 3 * 1024); + }); + + it("should download a larger file", async function () { + let dest = PathUtils.join(tmpDir, "large.bin"); + let req = await Zotero.HTTP.download( + baseURL + "download/large.bin", + dest + ); + assert.equal(req.status, 200); + let stat = await IOUtils.stat(dest); + assert.equal(stat.size, 256 * 1024); + }); + + it("should send request headers", async function () { + let dest = PathUtils.join(tmpDir, "custom.bin"); + let req = await Zotero.HTTP.download( + baseURL + "download/custom-header", + dest, + { + headers: { "X-Custom": "test-value" } + } + ); + assert.equal(req.status, 200); + assert.equal(req.headers.get("X-Echo"), "test-value"); + }); + + it("should throw UnexpectedStatusException for non-success status", async function () { + let dest = PathUtils.join(tmpDir, "404.bin"); + let e = await getPromiseError( + Zotero.HTTP.download(baseURL + "download/404", dest) + ); + assert.instanceOf(e, Zotero.HTTP.UnexpectedStatusException); + assert.equal(e.status, 404); + // File should not exist + assert.isFalse(await IOUtils.exists(dest)); + }); + + it("should allow non-success status with successCodes", async function () { + let dest = PathUtils.join(tmpDir, "404.bin"); + let req = await Zotero.HTTP.download( + baseURL + "download/404", + dest, + { + successCodes: [200, 404] + } + ); + assert.equal(req.status, 404); + }); + + it("should follow redirects", async function () { + let dest = PathUtils.join(tmpDir, "redirected.bin"); + let req = await Zotero.HTTP.download( + baseURL + "download/redirect-to-file", + dest + ); + assert.equal(req.status, 200); + let stat = await IOUtils.stat(dest); + assert.equal(stat.size, 3 * 1024); + }); + + it("should call onProgress during download", async function () { + let dest = PathUtils.join(tmpDir, "progress.bin"); + let progressCalls = []; + await Zotero.HTTP.download( + baseURL + "download/large.bin", + dest, + { + onProgress(progress, progressMax) { + progressCalls.push({ progress, progressMax }); + } + } + ); + assert.isAbove(progressCalls.length, 0); + // Last call should have accumulated all bytes + let last = progressCalls[progressCalls.length - 1]; + assert.equal(last.progress, 256 * 1024); + }); + + it("should retry on 5xx errors", async function () { + let delayStub = sinon.stub(Zotero.Promise, "delay") + .returns(Promise.resolve()); + try { + let dest = PathUtils.join(tmpDir, "500.bin"); + let e = await getPromiseError( + Zotero.HTTP.download( + baseURL + "download/500", + dest, + { + errorDelayIntervals: [10, 20], + errorDelayMax: 35 + } + ) + ); + assert.instanceOf(e, Zotero.HTTP.UnexpectedStatusException); + assert.equal(e.status, 500); + } + finally { + delayStub.restore(); + } + }); + + it("should accept nsIURI as first argument", async function () { + let dest = PathUtils.join(tmpDir, "nsuri.bin"); + let nsUri = Services.io.newURI(baseURL + "download/small.bin"); + let req = await Zotero.HTTP.download(nsUri, dest); + assert.equal(req.status, 200); + let stat = await IOUtils.stat(dest); + assert.equal(stat.size, 3 * 1024); + }); + }); + + describe("#processDocuments()", function () { beforeEach(function () { Zotero.HTTP.mock = null; diff --git a/test/tests/zfsTest.js b/test/tests/zfsTest.js index 3302e0ffdc..9cc17ade6f 100644 --- a/test/tests/zfsTest.js +++ b/test/tests/zfsTest.js @@ -143,20 +143,17 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { item.attachmentSyncState = "to_download"; await item.saveTx(); - httpd.registerPathHandler( - `/users/1/items/${item.key}/file`, - { - handle: function (request, response) { - response.setStatusLine(null, 404, null); - } - } + server.respondWith( + 'GET', + baseURL + `users/1/items/${item.key}/file`, + [404, {}, ""] ); var result = await engine.start(); - + assert.isFalse(result.localChanges); assert.isFalse(result.remoteChanges); assert.isFalse(result.syncRequired); - + assert.isFalse(library.storageDownloadNeeded); assert.equal(library.storageVersion, library.libraryVersion); assert.equal( @@ -214,13 +211,10 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { await item.saveTx(); Zotero.HTTP.disableErrorRetry = true; - httpd.registerPathHandler( - `/users/1/items/${item.key}/file`, - { - handle: function (request, response) { - response.setStatusLine(null, 500, null); - } - } + server.respondWith( + 'GET', + baseURL + `users/1/items/${item.key}/file`, + [500, {}, ""] ); // TODO: In stopOnError mode, this the promise is rejected. // This should probably test with stopOnError mode turned off instead. @@ -251,27 +245,17 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { var md5 = Zotero.Utilities.Internal.md5(text) var s3Path = `pretend-s3/${item.key}`; - httpd.registerPathHandler( - `/users/1/items/${item.key}/file`, - { - handle: function (request, response) { - if (!request.hasHeader('Zotero-API-Key')) { - response.setStatusLine(null, 403, "Forbidden"); - return; - } - var key = request.getHeader('Zotero-API-Key'); - if (key != apiKey) { - response.setStatusLine(null, 403, "Invalid key"); - return; - } - response.setStatusLine(null, 302, "Found"); - response.setHeader("Zotero-File-Modification-Time", mtime, false); - response.setHeader("Zotero-File-MD5", md5, false); - response.setHeader("Zotero-File-Compressed", "No", false); - response.setHeader("Location", baseURL + s3Path, false); - } + server.respondWith(function (req) { + if (req.method == "GET" + && req.url == baseURL + `users/1/items/${item.key}/file`) { + req.respond(302, { + "Zotero-File-Modification-Time": mtime, + "Zotero-File-MD5": md5, + "Zotero-File-Compressed": "No", + "Location": baseURL + s3Path, + }, ""); } - ); + }); httpd.registerPathHandler( "/" + s3Path, { @@ -313,18 +297,17 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { var md5 = Zotero.Utilities.Internal.md5(text); var s3Path = `pretend-s3/${item.key}`; - httpd.registerPathHandler( - `/users/1/items/${item.key}/file`, - { - handle: function (request, response) { - response.setStatusLine(null, 302, "Found"); - response.setHeader("Zotero-File-Modification-Time", mtime, false); - response.setHeader("Zotero-File-MD5", md5, false); - response.setHeader("Zotero-File-Compressed", "No", false); - response.setHeader("Location", baseURL + s3Path, false); - } + server.respondWith(function (req) { + if (req.method == "GET" + && req.url == baseURL + `users/1/items/${item.key}/file`) { + req.respond(302, { + "Zotero-File-Modification-Time": mtime, + "Zotero-File-MD5": md5, + "Zotero-File-Compressed": "No", + "Location": baseURL + s3Path, + }, ""); } - ); + }); httpd.registerPathHandler( "/" + s3Path, { @@ -683,27 +666,17 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { var processDownloadSpy = sinon.spy(Zotero.Sync.Storage.Local, "processDownload"); var s3Path = `pretend-s3/${item.key}`; - httpd.registerPathHandler( - `/users/1/items/${item.key}/file`, - { - handle: function (request, response) { - if (!request.hasHeader('Zotero-API-Key')) { - response.setStatusLine(null, 403, "Forbidden"); - return; - } - var key = request.getHeader('Zotero-API-Key'); - if (key != apiKey) { - response.setStatusLine(null, 403, "Invalid key"); - return; - } - response.setStatusLine(null, 302, "Found"); - response.setHeader("Zotero-File-Modification-Time", mtime, false); - response.setHeader("Zotero-File-MD5", md5, false); - response.setHeader("Zotero-File-Compressed", "No", false); - response.setHeader("Location", baseURL + s3Path, false); - } + server.respondWith(function (req) { + if (req.method == "GET" + && req.url == baseURL + `users/1/items/${item.key}/file`) { + req.respond(302, { + "Zotero-File-Modification-Time": mtime, + "Zotero-File-MD5": md5, + "Zotero-File-Compressed": "No", + "Location": baseURL + s3Path, + }, ""); } - ); + }); var result = await engine.start(); assert.equal(item.attachmentSyncedModificationTime, mtime);