Skip to content

Commit

Permalink
fix: limit the CORS workaround to Firefox
Browse files Browse the repository at this point in the history 
This implements suggestions from:
#494 (comment)

Wider context:
#494
  • Loading branch information
@lidel
lidel committed Jun 5, 2018
1 parent 1492d8d commit 8a713cd
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 29 deletions.
2 changes: 1 addition & 1 deletion add-on/src/lib/ipfs-companion.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
37 changes: 20 additions & 17 deletions add-on/src/lib/ipfs-request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
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()

Expand Down Expand Up @@ -52,23 +52,8 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator) {
if (request.method === 'HEAD') {
return
}
// Ignore XHR requests for which redirect would fail due to CORS bug in Firefox
// - We want the same behaviour on all browsers, so we conform to the Firefox limitation
// - More context: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
if (request.type === 'xmlhttprequest') {
// XHR Origin is fuzzy right now: Firefox 60 uses request.originUrl, Chrome 63 uses request.initiator
const originUrl = request.originUrl || request.initiator
if (originUrl) {
const sourceOrigin = new URL(originUrl).origin
const targetOrigin = new URL(request.url).origin
if (sourceOrigin !== targetOrigin) {
console.warn('[ipfs-companion] skipping XHR redirect due to https://github.com/ipfs-shipyard/ipfs-companion/issues/436', request)
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
Expand All @@ -91,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
Expand Down
35 changes: 24 additions & 11 deletions test/functional/lib/ipfs-request.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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,
Expand All @@ -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 () {
Expand Down Expand Up @@ -88,26 +90,40 @@ describe('modifyRequest', 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 Chrome', function () {
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 Chrome', function () {
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
Expand All @@ -116,15 +132,12 @@ describe('modifyRequest', function () {
state.ipfsNodeType = nodeType
})
it(`should be left untouched if request is a cross-origin XHR (Firefox, ${nodeType} node)`, function () {
// Context: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
// ==> 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)
})
it(`should be left untouched if request is a cross-origin XHR (Chrome, ${nodeType} node)`, function () {
// Context: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
const xhrRequest = {url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html'}
expect(modifyRequest(xhrRequest)).to.equal(undefined)
})
})
})
})
Expand Down

0 comments on commit 8a713cd

Please sign in to comment.