Merge pull request #494 from ipfs-shipyard/fix/firefox-cors-bug

This PR disables gateway redirect for cross-origin XHRs. - It fixes all the websites broken by [CORS false-positive bug in Firefox](#436 (comment)) - It is a "lesser evil". There will be no gateway redirect for such requests, but at least "IPFS-enabled" websites that do cross-origin requests to different gateways will work. - The full context is in #436 (comment) including a link to the upstream bug.
ipfs · Jun 5, 2018 · d1d715e · d1d715e
2 parents 62d5438 + 8a713cd
commit d1d715e
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 10 deletions.
diff --git a/add-on/src/lib/ipfs-companion.js b/add-on/src/lib/ipfs-companion.js
@@ -58,7 +58,7 @@ module.exports = async function init () {
       onCopyCanonicalAddress: () => copier.copyCanonicalAddress(),
       onCopyAddressAtPublicGw: () => copier.copyAddressAtPublicGw()
     })
-    modifyRequest = createRequestModifier(getState, dnsLink, ipfsPathValidator)
+    modifyRequest = createRequestModifier(getState, dnsLink, ipfsPathValidator, runtime)
     ipfsProxy = createIpfsProxy(() => ipfs, getState)
     registerListeners()
     await setApiStatusUpdateInterval(options.ipfsApiPollMs)

diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js
@@ -1,10 +1,10 @@
 'use strict'
-/* eslint-env browser */
+/* eslint-env browser, webextensions */
 
 const IsIpfs = require('is-ipfs')
 const { urlAtPublicGw } = require('./ipfs-path')
 
-function createRequestModifier (getState, dnsLink, ipfsPathValidator) {
+function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) {
   return function modifyRequest (request) {
     const state = getState()
 
@@ -49,11 +49,11 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator) {
     // handle redirects to custom gateway
     if (state.redirect) {
       // Ignore preload requests
-      if (request.method === 'HEAD' && state.preloadAtPublicGateway && request.url.startsWith(state.pubGwURLString)) {
+      if (request.method === 'HEAD') {
         return
       }
       // Detect valid /ipfs/ and /ipns/ on any site
-      if (ipfsPathValidator.publicIpfsOrIpnsResource(request.url)) {
+      if (ipfsPathValidator.publicIpfsOrIpnsResource(request.url) && isSafeToRedirect(request, runtime)) {
         return redirectToGateway(request.url, state)
       }
       // Look for dnslink in TXT records of visited sites
@@ -76,6 +76,24 @@ function redirectToGateway (requestUrl, state) {
   return { redirectUrl: url.toString() }
 }
 
+function isSafeToRedirect (request, runtime) {
+  // 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') {
+    // 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) {
+        console.warn('[ipfs-companion] skipping redirect of cross-origin XHR due to https://github.com/ipfs-shipyard/ipfs-companion/issues/436', request)
+        return false
+      }
+    }
+  }
+  return true
+}
+
 // REDIRECT-BASED PROTOCOL HANDLERS
 // This API is available only Firefox (protocol_handlers from manifest.json)
 // Background: https://github.com/ipfs-shipyard/ipfs-companion/issues/164#issuecomment-282513891

diff --git a/test/functional/lib/ipfs-request.test.js b/test/functional/lib/ipfs-request.test.js
@@ -5,6 +5,7 @@ const { expect } = require('chai')
 const { URL } = require('url')
 const browser = require('sinon-chrome')
 const { initState } = require('../../../add-on/src/lib/state')
+const createRuntimeChecks = require('../../../add-on/src/lib/runtime-checks')
 const { createRequestModifier } = require('../../../add-on/src/lib/ipfs-request')
 const createDnsLink = require('../../../add-on/src/lib/dns-link')
 const { createIpfsPathValidator } = require('../../../add-on/src/lib/ipfs-path')
@@ -17,14 +18,14 @@ const url2request = (string) => {
 const nodeTypes = ['external', 'embedded']
 
 describe('modifyRequest', function () {
-  let state, dnsLink, ipfsPathValidator, modifyRequest
+  let state, dnsLink, ipfsPathValidator, modifyRequest, runtime
 
   before(function () {
     global.URL = URL
     global.browser = browser
   })
 
-  beforeEach(function () {
+  beforeEach(async function () {
     state = Object.assign(initState(optionDefaults), {
       ipfsNodeType: 'external',
       peerCount: 1,
@@ -35,8 +36,9 @@ describe('modifyRequest', function () {
     })
     const getState = () => state
     dnsLink = createDnsLink(getState)
+    runtime = Object.assign({}, await createRuntimeChecks(browser)) // make it mutable for tests
     ipfsPathValidator = createIpfsPathValidator(getState, dnsLink)
-    modifyRequest = createRequestModifier(getState, dnsLink, ipfsPathValidator)
+    modifyRequest = createRequestModifier(getState, dnsLink, ipfsPathValidator, runtime)
   })
 
   describe('request for a path matching /ipfs/{CIDv0}', function () {
@@ -73,6 +75,69 @@ describe('modifyRequest', function () {
           const request = url2request('https://google.com/ipfs/notacid?argTest#hashTest')
           expect(modifyRequest(request)).to.equal(undefined)
         })
+        it(`should be left untouched if its is a preload request (${nodeType} node)`, function () {
+          // HTTP HEAD is often used for preloading data at arbitrary gateways - it should arrive at original destination
+          const headRequest = {url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', method: 'HEAD'}
+          expect(modifyRequest(headRequest)).to.equal(undefined)
+        })
+      })
+    })
+  })
+
+  describe('XHR request for a path matching /ipfs/{CIDv0}', function () {
+    describe('with external node', function () {
+      beforeEach(function () {
+        state.ipfsNodeType = 'external'
+      })
+      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(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(xhrRequest).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
+      })
+      it('should be served from custom gateway if fetch 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'}
+        expect(modifyRequest(xhrRequest).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
+      })
+    })
+    describe('with embedded node', function () {
+      beforeEach(function () {
+        state.ipfsNodeType = 'embedded'
+      })
+      it('should be served from public 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(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 () {
+        runtime.isFirefox = false
+        const xhrRequest = {url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://google.com/'}
+        expect(modifyRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
+      })
+      it('should be served from public gateway if fetch 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'}
+        expect(modifyRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
+      })
+    })
+    describe('with every node type', function () {
+      // tests in which results should be the same for all node types
+      nodeTypes.forEach(function (nodeType) {
+        beforeEach(function () {
+          state.ipfsNodeType = nodeType
+        })
+        it(`should be left untouched if request is a cross-origin XHR (Firefox, ${nodeType} node)`, function () {
+          // ==> This behavior is intentional
+          // Context for XHR & bogus CORS 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'}
+          expect(modifyRequest(xhrRequest)).to.equal(undefined)
+        })
       })
     })
   })
@@ -317,8 +382,8 @@ describe('modifyRequest', function () {
           expect(modifyRequest(request)).to.equal(undefined)
         })
         it('should not be normalized if request.type != main_frame', function () {
-          const xhrRequest = {url: 'https://duckduckgo.com/?q=ipfs%3A%2FQmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR%3FargTest%23hashTest&foo=bar', type: 'xmlhttprequest'}
-          expect(modifyRequest(xhrRequest)).to.equal(undefined)
+          const mediaRequest = {url: 'https://duckduckgo.com/?q=ipfs%3A%2FQmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR%3FargTest%23hashTest&foo=bar', type: 'media'}
+          expect(modifyRequest(mediaRequest)).to.equal(undefined)
         })
       })