Avoid infinite/excessive loops in Find Available PDF

https://forums.zotero.org/discussion/100634/potential-infinite-loop-when-trying-to-find-available-pdf

Closes #2883
This commit is contained in:
Dan Stillman 2022-10-30 04:44:31 -04:00
parent 4722366b42
commit 8e59e49d29
2 changed files with 59 additions and 0 deletions

View file

@ -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;
}

View file

@ -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' });