From 613ef27d8d22635074b2499d382357ed5404935b Mon Sep 17 00:00:00 2001 From: Emanuele D'Agostini Date: Sun, 9 Aug 2020 15:48:54 +0200 Subject: [PATCH 1/3] fix: Regression in HTTP recovery of background tabs #897 --- add-on/src/lib/ipfs-request.js | 6 +++--- .../lib/ipfs-request-gateway-recover.test.js | 12 ++++++------ test/functional/lib/ipfs-request-workarounds.test.js | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index 38095b734..8106ebb09 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/add-on/src/lib/ipfs-request.js @@ -414,9 +414,9 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru // Chromium bug: sometimes tabs.update does not work from onCompleted, // we run additional update after 1s just to be sure - setTimeout(() => browser.tabs.update({ url: fixedUrl }), 1000) + setTimeout(() => browser.tabs.update(request.tabId,{ url: fixedUrl }), 1000) - return browser.tabs.update({ url: fixedUrl }) + return browser.tabs.update(request.tabId,{ url: fixedUrl }) } if (isRecoverable(request, state, ipfsPathValidator)) { @@ -616,7 +616,7 @@ async function updateTabWithURL (request, redirectUrl, browser) { // Do nothing if the URL remains the same if (request.url === redirectUrl) return - return browser.tabs.update({ + return browser.tabs.update(request.tabId,{ active: true, url: redirectUrl }) diff --git a/test/functional/lib/ipfs-request-gateway-recover.test.js b/test/functional/lib/ipfs-request-gateway-recover.test.js index 83faaa090..6a9fe315d 100644 --- a/test/functional/lib/ipfs-request-gateway-recover.test.js +++ b/test/functional/lib/ipfs-request-gateway-recover.test.js @@ -54,7 +54,7 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors it('should redirect to default subdomain gateway on broken subdomain gateway request', async function () { const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.brokenexample.com/wiki/', 500) await requestHandler.onCompleted(request) - assert.ok(browser.tabs.update.withArgs({ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL') }) it('should do nothing if broken request is a non-IPFS request', async function () { const request = urlRequestWithStatus('https://wikipedia.org', 500) @@ -89,7 +89,7 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors it('should recover from unreachable third party public gateway by reopening on the public gateway', async function () { const request = urlRequestWithStatus('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500) await requestHandler.onCompleted(request) - assert.ok(browser.tabs.update.withArgs({ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL') }) }) @@ -153,7 +153,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors it('should redirect to default subdomain gateway on failed subdomain gateway request', async function () { const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.brokenexample.com/wiki/', 500) await requestHandler.onErrorOccurred(request) - assert.ok(browser.tabs.update.withArgs({ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL') }) it('should do nothing if failed request is a non-IPFS request', async function () { const request = urlRequestWithNetworkError('https://wikipedia.org') @@ -179,7 +179,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors it('should recover from unreachable third party public gateway by reopening on the public gateway', async function () { const request = urlRequestWithNetworkError('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h') await requestHandler.onErrorOccurred(request) - assert.ok(browser.tabs.update.withArgs({ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL') }) it('should recover from unreachable HTTP server by reopening DNSLink on the active gateway', async function () { state.dnslinkPolicy = 'best-effort' @@ -188,7 +188,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors const expectedUrl = 'http://localhost:8080/ipns/en.wikipedia-on-ipfs.org/' const request = urlRequestWithNetworkError('https://en.wikipedia-on-ipfs.org/') await requestHandler.onErrorOccurred(request) - assert.ok(browser.tabs.update.withArgs({ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with DNSLink on local gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with DNSLink on local gateway URL') dnslinkResolver.clearCache() }) it('should recover from failed DNS for .eth opening it on EthDNS gateway at .eth.link', async function () { @@ -199,7 +199,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors const expectedUrl = 'https://almonit.eth.link/' const request = urlRequestWithNetworkError('https://almonit.eth', dnsFailure) await requestHandler.onErrorOccurred(request) - assert.ok(browser.tabs.update.withArgs({ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with ENS resource on local gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with ENS resource on local gateway URL') dnslinkResolver.clearCache() }) }) diff --git a/test/functional/lib/ipfs-request-workarounds.test.js b/test/functional/lib/ipfs-request-workarounds.test.js index 4083aa5af..20eabddab 100644 --- a/test/functional/lib/ipfs-request-workarounds.test.js +++ b/test/functional/lib/ipfs-request-workarounds.test.js @@ -193,7 +193,7 @@ describe('modifyRequest processing', function () { browser.tabs.update.flush() assert.ok(browser.tabs.update.notCalled) modifyRequest.onCompleted(request) - assert.ok(browser.tabs.update.withArgs({ url: fixedDNSLinkUrl }).calledOnce) + assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: fixedDNSLinkUrl }).calledOnce) browser.tabs.update.flush() }) }) From 06f5790ed613f9113e6ffdf9e485bdbf1f5e72a5 Mon Sep 17 00:00:00 2001 From: Emanuele D'Agostini Date: Sun, 9 Aug 2020 16:25:29 +0200 Subject: [PATCH 2/3] fix: Add space after comma on previous changes --- add-on/src/lib/ipfs-request.js | 6 +++--- .../lib/ipfs-request-gateway-recover.test.js | 12 ++++++------ test/functional/lib/ipfs-request-workarounds.test.js | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index 8106ebb09..f6403e55a 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/add-on/src/lib/ipfs-request.js @@ -414,9 +414,9 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru // Chromium bug: sometimes tabs.update does not work from onCompleted, // we run additional update after 1s just to be sure - setTimeout(() => browser.tabs.update(request.tabId,{ url: fixedUrl }), 1000) + setTimeout(() => browser.tabs.update(request.tabId, { url: fixedUrl }), 1000) - return browser.tabs.update(request.tabId,{ url: fixedUrl }) + return browser.tabs.update(request.tabId, { url: fixedUrl }) } if (isRecoverable(request, state, ipfsPathValidator)) { @@ -616,7 +616,7 @@ async function updateTabWithURL (request, redirectUrl, browser) { // Do nothing if the URL remains the same if (request.url === redirectUrl) return - return browser.tabs.update(request.tabId,{ + return browser.tabs.update(request.tabId, { active: true, url: redirectUrl }) diff --git a/test/functional/lib/ipfs-request-gateway-recover.test.js b/test/functional/lib/ipfs-request-gateway-recover.test.js index 6a9fe315d..48342a131 100644 --- a/test/functional/lib/ipfs-request-gateway-recover.test.js +++ b/test/functional/lib/ipfs-request-gateway-recover.test.js @@ -54,7 +54,7 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors it('should redirect to default subdomain gateway on broken subdomain gateway request', async function () { const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.brokenexample.com/wiki/', 500) await requestHandler.onCompleted(request) - assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId, { url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL') }) it('should do nothing if broken request is a non-IPFS request', async function () { const request = urlRequestWithStatus('https://wikipedia.org', 500) @@ -89,7 +89,7 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors it('should recover from unreachable third party public gateway by reopening on the public gateway', async function () { const request = urlRequestWithStatus('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500) await requestHandler.onCompleted(request) - assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId, { url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL') }) }) @@ -153,7 +153,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors it('should redirect to default subdomain gateway on failed subdomain gateway request', async function () { const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.brokenexample.com/wiki/', 500) await requestHandler.onErrorOccurred(request) - assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId, { url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL') }) it('should do nothing if failed request is a non-IPFS request', async function () { const request = urlRequestWithNetworkError('https://wikipedia.org') @@ -179,7 +179,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors it('should recover from unreachable third party public gateway by reopening on the public gateway', async function () { const request = urlRequestWithNetworkError('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h') await requestHandler.onErrorOccurred(request) - assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId, { url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL') }) it('should recover from unreachable HTTP server by reopening DNSLink on the active gateway', async function () { state.dnslinkPolicy = 'best-effort' @@ -188,7 +188,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors const expectedUrl = 'http://localhost:8080/ipns/en.wikipedia-on-ipfs.org/' const request = urlRequestWithNetworkError('https://en.wikipedia-on-ipfs.org/') await requestHandler.onErrorOccurred(request) - assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with DNSLink on local gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId, { url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with DNSLink on local gateway URL') dnslinkResolver.clearCache() }) it('should recover from failed DNS for .eth opening it on EthDNS gateway at .eth.link', async function () { @@ -199,7 +199,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors const expectedUrl = 'https://almonit.eth.link/' const request = urlRequestWithNetworkError('https://almonit.eth', dnsFailure) await requestHandler.onErrorOccurred(request) - assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with ENS resource on local gateway URL') + assert.ok(browser.tabs.update.withArgs(request.tabId, { url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with ENS resource on local gateway URL') dnslinkResolver.clearCache() }) }) diff --git a/test/functional/lib/ipfs-request-workarounds.test.js b/test/functional/lib/ipfs-request-workarounds.test.js index 20eabddab..3686bdd36 100644 --- a/test/functional/lib/ipfs-request-workarounds.test.js +++ b/test/functional/lib/ipfs-request-workarounds.test.js @@ -193,7 +193,7 @@ describe('modifyRequest processing', function () { browser.tabs.update.flush() assert.ok(browser.tabs.update.notCalled) modifyRequest.onCompleted(request) - assert.ok(browser.tabs.update.withArgs(request.tabId,{ url: fixedDNSLinkUrl }).calledOnce) + assert.ok(browser.tabs.update.withArgs(request.tabId, { url: fixedDNSLinkUrl }).calledOnce) browser.tabs.update.flush() }) }) From 1689d9edc9efc392c9f3940c1015a9d8fe52cf4a Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Mon, 10 Aug 2020 15:22:30 +0200 Subject: [PATCH 3/3] test: ensure recovery tests use real tabId --- test/functional/lib/ipfs-request-gateway-recover.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/lib/ipfs-request-gateway-recover.test.js b/test/functional/lib/ipfs-request-gateway-recover.test.js index 48342a131..3646b351d 100644 --- a/test/functional/lib/ipfs-request-gateway-recover.test.js +++ b/test/functional/lib/ipfs-request-gateway-recover.test.js @@ -11,8 +11,8 @@ const createDnslinkResolver = require('../../../add-on/src/lib/dnslink') const { createIpfsPathValidator } = require('../../../add-on/src/lib/ipfs-path') const { optionDefaults } = require('../../../add-on/src/lib/options') -const url2request = (url, type = 'main_frame') => { - return { url, type } +const url2request = (url, type = 'main_frame', tabId = new Date().valueOf()) => { + return { url, type, tabId } } const urlRequestWithStatus = (url, statusCode = 200, type = 'main_frame') => { return { ...url2request(url, type), statusCode }