From fb5958a30f98fc1848c8d2dfc53a6581bb3cc3aa Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Mon, 11 Nov 2019 18:05:28 +0100 Subject: [PATCH] fix: restore ESR compatibility (#812) - changes minimal Firefox version to 68 - restores CORS workaround when runtime is Firefox <69. This makes Companion once again compatible with Firefox ESR and Fennec-based Firefox for Android. Closes #784 Closes #779 --- .babelrc | 2 +- add-on/manifest.firefox.json | 2 +- add-on/src/lib/ipfs-request.js | 35 +++++++++++++++++++ add-on/src/lib/runtime-checks.js | 23 ++++++------ .../lib/ipfs-request-dnslink.test.js | 27 ++++++++++++++ .../lib/ipfs-request-gateway-redirect.test.js | 30 ++++++++++++++++ test/functional/lib/runtime-checks.test.js | 33 ++++++++++++----- 7 files changed, 130 insertions(+), 22 deletions(-) diff --git a/.babelrc b/.babelrc index ba9f6c7a8..c3e6b26ed 100644 --- a/.babelrc +++ b/.babelrc @@ -7,7 +7,7 @@ "@babel/preset-env", { "targets": { - "firefox": 69, + "firefox": 68, "chrome": 72 } } diff --git a/add-on/manifest.firefox.json b/add-on/manifest.firefox.json index 3452553f1..4f06f5dc3 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": "69.0" + "strict_min_version": "68.0" } }, "page_action": { diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index 2573a6abf..af684f2e6 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/add-on/src/lib/ipfs-request.js @@ -24,6 +24,9 @@ const recoverableNetworkErrors = new Set([ ]) const recoverableHttpError = (code) => code && code >= 400 +// 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) { @@ -311,6 +314,19 @@ 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 (runtime.requiresXHRCORSfix && 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 @@ -404,6 +420,11 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru } } + // Cleanup after https://github.com/ipfs-shipyard/ipfs-companion/issues/436 + if (runtime.requiresXHRCORSfix && onHeadersReceivedRedirect.has(request.requestId)) { + onHeadersReceivedRedirect.delete(request.requestId) + } + // Check if error can be recovered by opening same content-addresed path // using active gateway (public or local, depending on redirect state) if (isRecoverable(request, state, ipfsPathValidator)) { @@ -463,6 +484,20 @@ 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 + if (runtime.requiresXHRCORSfix && 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/add-on/src/lib/runtime-checks.js b/add-on/src/lib/runtime-checks.js index a60ef81bd..7f0092ea0 100644 --- a/add-on/src/lib/runtime-checks.js +++ b/add-on/src/lib/runtime-checks.js @@ -6,7 +6,7 @@ function getBrowserInfo (browser) { if (browser && browser.runtime && browser.runtime.getBrowserInfo) { return browser.runtime.getBrowserInfo() } - return Promise.resolve() + return Promise.resolve({}) } function getPlatformInfo (browser) { @@ -28,21 +28,20 @@ function hasChromeSocketsForTcp () { async function createRuntimeChecks (browser) { // browser - const browserInfo = await getBrowserInfo(browser) - const runtimeBrowserName = browserInfo ? browserInfo.name : 'unknown' - const runtimeIsFirefox = !!(runtimeBrowserName.includes('Firefox') || runtimeBrowserName.includes('Fennec')) - const runtimeHasNativeProtocol = !!(browser && browser.protocol && browser.protocol.registerStringProtocol) + const { name, version } = await getBrowserInfo(browser) + const isFirefox = name && (name.includes('Firefox') || name.includes('Fennec')) + const hasNativeProtocolHandler = !!(browser && browser.protocol && browser.protocol.registerStringProtocol) // platform const platformInfo = await getPlatformInfo(browser) - const runtimeIsAndroid = platformInfo ? platformInfo.os === 'android' : false - const runtimeHasSocketsForTcp = hasChromeSocketsForTcp() + const isAndroid = platformInfo ? platformInfo.os === 'android' : false return Object.freeze({ browser, - isFirefox: runtimeIsFirefox, - isAndroid: runtimeIsAndroid, - isBrave: runtimeHasSocketsForTcp, // TODO: make it more robust - hasChromeSocketsForTcp: runtimeHasSocketsForTcp, - hasNativeProtocolHandler: runtimeHasNativeProtocol + isFirefox, + isAndroid, + isBrave: hasChromeSocketsForTcp(), // TODO: make it more robust + requiresXHRCORSfix: !!(isFirefox && version && version.startsWith('68')), + hasChromeSocketsForTcp: hasChromeSocketsForTcp(), + hasNativeProtocolHandler }) } diff --git a/test/functional/lib/ipfs-request-dnslink.test.js b/test/functional/lib/ipfs-request-dnslink.test.js index 43a4909d9..13bf3cedd 100644 --- a/test/functional/lib/ipfs-request-dnslink.test.js +++ b/test/functional/lib/ipfs-request-dnslink.test.js @@ -183,6 +183,20 @@ describe('modifyRequest processing', function () { const xhrRequest = { url: 'http://explore.ipld.io/index.html?argTest#hashTest', type: 'xmlhttprequest', originUrl: '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 <69', 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 <69: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 + runtime.requiresXHRCORSfix = 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') + }) it('should do nothing if dnslink does not exist and XHR is cross-origin in Firefox', function () { // stub no dnslink const fqdn = 'youtube.com' @@ -286,6 +300,19 @@ describe('modifyRequest processing', function () { xhrRequest.responseHeaders = [{ name: 'X-Ipfs-Path', value: '/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd' }] expect(modifyRequest.onHeadersReceived(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 <69', function () { + // stub existence of a valid DNS record + const fqdn = 'explore.ipld.io' + dnslinkResolver.setDnslink(fqdn, '/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd') + // + // Context for CORS XHR problems in Firefox <69: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 + runtime.requiresXHRCORSfix = 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') + }) // Test makes more sense for dnslinkPolicy=enabled, but we keep it here for completeness it('should do nothing if DNS TXT record is missing and XHR is cross-origin in Firefox', function () { // stub no dnslink diff --git a/test/functional/lib/ipfs-request-gateway-redirect.test.js b/test/functional/lib/ipfs-request-gateway-redirect.test.js index 1279d9bc1..91916e1a7 100644 --- a/test/functional/lib/ipfs-request-gateway-redirect.test.js +++ b/test/functional/lib/ipfs-request-gateway-redirect.test.js @@ -160,6 +160,36 @@ describe('modifyRequest.onBeforeRequest:', function () { expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') }) }) + describe('with external node when runtime.requiresXHRCORSfix', function () { + beforeEach(function () { + state.ipfsNodeType = 'external' + browser.runtime.getBrowserInfo = () => Promise.resolve({ name: 'Firefox', version: '68.0.0' }) + }) + it('should be served from custom gateway if fetched from the same 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://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 () { + 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 () { + 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 + 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') + }) + }) }) describe('request for a path matching /ipns/{path}', function () { diff --git a/test/functional/lib/runtime-checks.test.js b/test/functional/lib/runtime-checks.test.js index 343ab7ada..fffd577a5 100644 --- a/test/functional/lib/runtime-checks.test.js +++ b/test/functional/lib/runtime-checks.test.js @@ -5,6 +5,7 @@ const { describe, it, before, beforeEach, after } = require('mocha') const { expect } = require('chai') const browser = require('sinon-chrome') const { createRuntimeChecks, hasChromeSocketsForTcp } = require('../../../add-on/src/lib/runtime-checks') +const promiseStub = (result) => () => Promise.resolve(result) describe('runtime-checks.js', function () { before(() => { @@ -21,14 +22,6 @@ describe('runtime-checks.js', function () { browser.flush() }) - // current version of sinon-chrome is missing stubs for some APIs, - // this is a workaround until fix is provided upstream - function promiseStub (result) { - return () => { - return Promise.resolve(result) - } - } - it('should return true when in Firefox runtime', async function () { browser.runtime.getBrowserInfo = promiseStub({ name: 'Firefox' }) const runtime = await createRuntimeChecks(browser) @@ -42,6 +35,30 @@ describe('runtime-checks.js', function () { }) }) + describe('requiresXHRCORSfix', function () { + beforeEach(function () { + browser.flush() + }) + + it('should return true when in Firefox runtime < 69', async function () { + browser.runtime.getBrowserInfo = promiseStub({ name: 'Firefox', version: '68.0.0' }) + const runtime = await createRuntimeChecks(browser) + expect(runtime.requiresXHRCORSfix).to.equal(true) + }) + + it('should return false when in Firefox runtime >= 69', async function () { + browser.runtime.getBrowserInfo = promiseStub({ name: 'Firefox', version: '69.0.0' }) + const runtime = await createRuntimeChecks(browser) + expect(runtime.requiresXHRCORSfix).to.equal(false) + }) + + it('should return false when if getBrowserInfo is not present', async function () { + browser.runtime.getBrowserInfo = undefined + const runtime = await createRuntimeChecks(browser) + expect(runtime.requiresXHRCORSfix).to.equal(false) + }) + }) + describe('isAndroid', function () { beforeEach(function () { browser.flush()