From 40b7ab75353dd5140c82d6884d23ceabbcd48220 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 1 Oct 2019 11:11:52 +0200 Subject: [PATCH] fix: remove XHR CORS workaround for Firefox (#771) This change removes workaround introduced in PR #494 and restores redirect in onBeforeRequest. The original bug was fixed in Firefox 69, that is why we also bump minimal version. More info at: https://github.com/ipfs-shipyard/ipfs-companion/issues/436#issuecomment-507764433 --- .babelrc | 2 +- README.md | 7 ---- add-on/manifest.firefox.json | 2 +- add-on/src/lib/ipfs-request.js | 37 ------------------- .../lib/ipfs-request-dnslink.test.js | 17 +++------ .../lib/ipfs-request-gateway-redirect.test.js | 24 ++++-------- 6 files changed, 15 insertions(+), 74 deletions(-) diff --git a/.babelrc b/.babelrc index 64bd61c74..f54d84692 100644 --- a/.babelrc +++ b/.babelrc @@ -7,7 +7,7 @@ "@babel/preset-env", { "targets": { - "firefox": 67, + "firefox": 69, "chrome": 70 } } diff --git a/README.md b/README.md index 990513e86..0cc0e4e0a 100644 --- a/README.md +++ b/README.md @@ -226,13 +226,6 @@ We are also available at the [#ipfs](https://webchat.freenode.net/?channels=ipfs Questions specific to this browser companion can be asked directly at [`#ipfs-in-web-browsers`](https://webchat.freenode.net/?channels=ipfs-in-web-browsers) -#### Cross-Origin XHR are executed "twice" in Firefox - -Due to CORS bug XHRs in Firefox are handled via late redirects in `onHeadersReceived`. -Original request is cancelled after response headers are received, so there is no overhead of reading response payload twice. - -For more details on this see [PR #511](https://github.com/ipfs-shipyard/ipfs-companion/pull/511). - #### Upload via Right-Click Does Not Work in Firefox See [this workaround](https://github.com/ipfs/ipfs-companion/issues/227). diff --git a/add-on/manifest.firefox.json b/add-on/manifest.firefox.json index 85c2dc67a..3452553f1 100644 --- a/add-on/manifest.firefox.json +++ b/add-on/manifest.firefox.json @@ -8,7 +8,7 @@ "applications": { "gecko": { "id": "ipfs-firefox-addon@lidel.org", - "strict_min_version": "67.0" + "strict_min_version": "69.0" } }, "page_action": { diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index e68117f4f..03d179e60 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/add-on/src/lib/ipfs-request.js @@ -20,9 +20,6 @@ const recoverableErrors = new Set([ 'net::ERR_INTERNET_DISCONNECTED' // no network ]) -// Tracking late redirects for edge cases such as https://github.com/ipfs-shipyard/ipfs-companion/issues/436 -const onHeadersReceivedRedirect = new Set() - // Request modifier provides event listeners for the various stages of making an HTTP request // API Details: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, runtime) { @@ -316,19 +313,6 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru } if (state.redirect) { - // Late redirect as a workaround for edge cases such as: - // - CORS XHR in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 - if (onHeadersReceivedRedirect.has(request.requestId)) { - onHeadersReceivedRedirect.delete(request.requestId) - if (state.dnslinkPolicy) { - const dnslinkRedirect = dnslinkResolver.dnslinkRedirect(request.url) - if (dnslinkRedirect) { - return dnslinkRedirect - } - } - return redirectToGateway(request.url, state, ipfsPathValidator) - } - // Detect X-Ipfs-Path Header and upgrade transport to IPFS: // 1. Check if DNSLink exists and redirect to it. // 2. If there is no DNSLink, validate path from the header and redirect @@ -417,11 +401,6 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru }) } } - - // Cleanup after https://github.com/ipfs-shipyard/ipfs-companion/issues/436 - if (onHeadersReceivedRedirect.has(request.requestId)) { - onHeadersReceivedRedirect.delete(request.requestId) - } } } @@ -429,7 +408,6 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru exports.redirectOptOutHint = redirectOptOutHint exports.createRequestModifier = createRequestModifier -exports.onHeadersReceivedRedirect = onHeadersReceivedRedirect function redirectToGateway (requestUrl, state, ipfsPathValidator) { // TODO: redirect to `ipfs://` if hasNativeProtocolHandler === true @@ -450,21 +428,6 @@ function isSafeToRedirect (request, runtime) { return false } - // Ignore XHR requests for which redirect would fail due to CORS bug in Firefox - // See: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 - // TODO: revisit when upstream bug is addressed - if (runtime.isFirefox && request.type === 'xmlhttprequest' && !request.responseHeaders) { - // Sidenote on XHR Origin: Firefox 60 uses request.originUrl, Chrome 63 uses request.initiator - if (request.originUrl) { - const sourceOrigin = new URL(request.originUrl).origin - const targetOrigin = new URL(request.url).origin - if (sourceOrigin !== targetOrigin) { - log('Delaying redirect of CORS XHR until onHeadersReceived due to https://github.com/ipfs-shipyard/ipfs-companion/issues/436 :', request.url) - onHeadersReceivedRedirect.add(request.requestId) - return false - } - } - } return true } diff --git a/test/functional/lib/ipfs-request-dnslink.test.js b/test/functional/lib/ipfs-request-dnslink.test.js index 164d6ef23..43a4909d9 100644 --- a/test/functional/lib/ipfs-request-dnslink.test.js +++ b/test/functional/lib/ipfs-request-dnslink.test.js @@ -163,7 +163,7 @@ describe('modifyRequest processing', function () { const request = url2request('http://explore.ipld.io/index.html?argTest#hashTest') expect(modifyRequest.onBeforeRequest(request).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest') }) - it('should redirect in onBeforeRequest if DNS TXT record exists, XHR is cross-origin and runtime is not Firefox', function () { + it('should redirect in onBeforeRequest if DNS TXT record exists, XHR is cross-origin and runtime is Chromium', function () { // stub existence of a valid DNS record const fqdn = 'explore.ipld.io' dnslinkResolver.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd') @@ -173,19 +173,15 @@ describe('modifyRequest processing', function () { const xhrRequest = { url: 'http://explore.ipld.io/index.html?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() } expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest') }) - it('should redirect later in onHeadersReceived if dnslink exists, XHR is cross-origin and runtime is Firefox', function () { + it('should redirect in onBeforeRequest if dnslink exists, XHR is cross-origin and runtime is Firefox', function () { // stub existence of a valid DNS record const fqdn = 'explore.ipld.io' dnslinkResolver.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd') // - // Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 runtime.isFirefox = true // Firefox uses 'originUrl' for origin const xhrRequest = { url: 'http://explore.ipld.io/index.html?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() } - // onBeforeRequest should not change anything, as it will trigger false-positive CORS error - expect(modifyRequest.onBeforeRequest(xhrRequest)).to.equal(undefined) - // onHeadersReceived is after CORS validation happens, so its ok to cancel and redirect late - expect(modifyRequest.onHeadersReceived(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest') + expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest') }) it('should do nothing if dnslink does not exist and XHR is cross-origin in Firefox', function () { // stub no dnslink @@ -325,7 +321,7 @@ describe('modifyRequest processing', function () { const xhrRequest = { url: 'http://explore.ipld.io/index.html?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() } expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest') }) - it('should redirect later in onHeadersReceived if XHR is cross-origin and runtime is Firefox', function () { + it('should redirect in onBeforeRequest if XHR is cross-origin and runtime is Firefox', function () { // stub existence of a valid DNS record const fqdn = 'explore.ipld.io' dnslinkResolver.setDnslink(fqdn, '/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd') @@ -333,10 +329,7 @@ describe('modifyRequest processing', function () { // Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 runtime.isFirefox = true const xhrRequest = { url: 'http://explore.ipld.io/index.html?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() } - // onBeforeRequest should not change anything, as it will trigger false-positive CORS error - expect(modifyRequest.onBeforeRequest(xhrRequest)).to.equal(undefined) - // onHeadersReceived is after CORS validation happens, so its ok to cancel and redirect late - expect(modifyRequest.onHeadersReceived(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest') + expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest') }) it('should do nothing if DNS TXT record is missing and XHR is cross-origin in Firefox', function () { // stub cached info about lack of dnslink diff --git a/test/functional/lib/ipfs-request-gateway-redirect.test.js b/test/functional/lib/ipfs-request-gateway-redirect.test.js index f2a735671..1279d9bc1 100644 --- a/test/functional/lib/ipfs-request-gateway-redirect.test.js +++ b/test/functional/lib/ipfs-request-gateway-redirect.test.js @@ -119,24 +119,20 @@ describe('modifyRequest.onBeforeRequest:', function () { const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://google.com/' } expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) - it('should be served from custom gateway if fetched from the same origin and redirect is enabled in non-Firefox', function () { + it('should be served from custom gateway if fetched from the same origin and redirect is enabled in Chromium', function () { runtime.isFirefox = false const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://google.com/' } expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) - it('should be served from custom gateway if XHR is cross-origin and redirect is enabled in non-Firefox', function () { + it('should be served from custom gateway if XHR is cross-origin and redirect is enabled in Chromium', function () { runtime.isFirefox = false const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() } expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) - it('should be served from custom gateway via late redirect in onHeadersReceived if XHR is cross-origin and redirect is enabled in Firefox', function () { - // Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 + it('should be served from custom gateway if XHR is cross-origin and redirect is enabled in Firefox', function () { runtime.isFirefox = true const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() } - // onBeforeRequest should not change anything, as it will trigger false-positive CORS error - expect(modifyRequest.onBeforeRequest(xhrRequest)).to.equal(undefined) - // onHeadersReceived is after CORS validation happens, so its ok to cancel and redirect late - expect(modifyRequest.onHeadersReceived(xhrRequest).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') + expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) }) describe('with embedded node', function () { @@ -148,24 +144,20 @@ describe('modifyRequest.onBeforeRequest:', function () { const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://google.com/' } expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) - it('should be served from public gateway if fetched from the same origin and redirect is enabled in non-Firefox', function () { + it('should be served from public gateway if fetched from the same origin and redirect is enabled in Chromium', function () { runtime.isFirefox = false const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://google.com/' } expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) - it('should be served from public gateway if XHR is cross-origin and redirect is enabled in non-Firefox', function () { + it('should be served from public gateway if XHR is cross-origin and redirect is enabled in Chromium', function () { runtime.isFirefox = false const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() } expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) - it('should be served from public gateway via late redirect in onHeadersReceived if XHR is cross-origin and redirect is enabled in Firefox', function () { - // Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 + it('should be served from public gateway if XHR is cross-origin and redirect is enabled in Firefox', function () { runtime.isFirefox = true const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() } - // onBeforeRequest should not change anything, as it will trigger false-positive CORS error - expect(modifyRequest.onBeforeRequest(xhrRequest)).to.equal(undefined) - // onHeadersReceived is after CORS validation happens, so its ok to cancel and redirect late - expect(modifyRequest.onHeadersReceived(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') + expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) }) })