diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index e8ab13b50d..91173a03b0 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -2064,7 +2064,15 @@ Zotero.Attachments = new function(){ let contentType; let skip = false; let domains = new Set(); + let redirectLimit = 10; + let redirectURLTries = new Map(); while (true) { + if (redirectLimit == 0) { + Zotero.debug("Too many redirects -- stopping"); + skip = true; + break; + } + let domain = urlToDomain(nextURL); let noDelay = domains.has(domain); domains.add(domain); @@ -2104,12 +2112,27 @@ Zotero.Attachments = new function(){ if (!location) { throw new Error("Location header not provided"); } + + let currentURL = nextURL; + nextURL = Services.io.newURI(nextURL, null, null).resolve(location); if (isTriedURL(nextURL)) { Zotero.debug("Redirect URL has already been tried -- skipping"); skip = true; break; } + + // Keep track of tries for each redirect URL, and stop if too many + let maxTriesPerRedirectURL = 2; + let tries = (redirectURLTries.get(currentURL) || 0) + 1; + if (tries > maxTriesPerRedirectURL) { + Zotero.debug(`Too many redirects to ${currentURL} -- stopping`); + skip = true; + break; + } + redirectURLTries.set(currentURL, tries); + // And keep track of total redirects for this chain + redirectLimit--; continue; } diff --git a/test/tests/attachmentsTest.js b/test/tests/attachmentsTest.js index 5e1b6802b5..5c7ae8bdf9 100644 --- a/test/tests/attachmentsTest.js +++ b/test/tests/attachmentsTest.js @@ -679,6 +679,24 @@ describe("Zotero.Attachments", function() { // DOI 6 redirects to page 8, which is on a different domain and has a PDF [doiPrefix + doi6, pageURL8, true], [pageURL8, pageURL8, true], + + // Redirect loop + ['http://website/redirect_loop1', 'http://website/redirect_loop2', false], + ['http://website/redirect_loop2', 'http://website/redirect_loop3', false], + ['http://website/redirect_loop3', 'http://website/redirect_loop1', false], + + // Too many total redirects + ['http://website/too_many_redirects1', 'http://website/too_many_redirects2', false], + ['http://website/too_many_redirects2', 'http://website/too_many_redirects3', false], + ['http://website/too_many_redirects3', 'http://website/too_many_redirects4', false], + ['http://website/too_many_redirects4', 'http://website/too_many_redirects5', false], + ['http://website/too_many_redirects5', 'http://website/too_many_redirects6', false], + ['http://website/too_many_redirects6', 'http://website/too_many_redirects7', false], + ['http://website/too_many_redirects7', 'http://website/too_many_redirects8', false], + ['http://website/too_many_redirects8', 'http://website/too_many_redirects9', false], + ['http://website/too_many_redirects9', 'http://website/too_many_redirects10', false], + ['http://website/too_many_redirects10', 'http://website/too_many_redirects11', false], + ['http://website/too_many_redirects11', pageURL1, true], ]; for (let route of routes) { let [expectedURL, responseURL, includePDF] = route; @@ -1092,6 +1110,24 @@ describe("Zotero.Attachments", function() { assert.equal(await OS.File.stat(attachment.getFilePath()).size, pdfSize); }); + it("should stop after too many redirects to the same URL", async function () { + var item = createUnsavedDataObject('item', { itemType: 'journalArticle' }); + item.setField('url', 'http://website/redirect_loop1'); + await item.saveTx(); + var attachment = await Zotero.Attachments.addAvailablePDF(item); + assert.isFalse(attachment); + assert.equal(requestStub.callCount, 7); + }); + + it("should stop after too many total redirects for a given page URL", async function () { + var item = createUnsavedDataObject('item', { itemType: 'journalArticle' }); + item.setField('url', 'http://website/too_many_redirects1'); + await item.saveTx(); + var attachment = await Zotero.Attachments.addAvailablePDF(item); + assert.isFalse(attachment); + assert.equal(requestStub.callCount, 10); + }); + it("should handle a custom resolver in HTML mode", async function () { var doi = doi4; var item = createUnsavedDataObject('item', { itemType: 'journalArticle' });