Skip to content

Commit

Permalink
Merge pull request #511 from ipfs-shipyard/fix/cross-origin-xhr-firefox
Browse files Browse the repository at this point in the history
This PR restores gateway redirect for CORS XHR in Firefox via late redirect in `onHeadersReceived` and closes #436.

Context for why CORS XHR were not redirected until now can be found in #436 (comment) and upstream [Bug #1450965](https://bugzilla.mozilla.org/show_bug.cgi?id=1450965). 

In short, `onBeforeRequest` can't redirect anything when CORS XHR is processed, otherwise it will trigger false-positive CORS error.  Good news is that `onHeadersReceived` is executed  long after CORS validation happens in Firefox, and allows us to cancel original connection and do a late redirect right after response headers arrive. 

This is the best we can do until upstream [Bug #1450965](https://bugzilla.mozilla.org/show_bug.cgi?id=1450965) is addressed.

Additional Notes: 
- The original outgoing request to the public gateway has to happen :( 
  I [raised privacy concerns](https://bugzilla.mozilla.org/show_bug.cgi?id=1450965#c4) in the upstream bug.
-  Good news is that we only need to read response headers before connection is  aborted. The overhead is minimal and I believe it is worth it: enables us to reach feature-parity with Chrome and removes dapp developer confusion that was caused by different redirect behaviors in Firefox and Chrome.
  • Loading branch information
lidel authored Jul 3, 2018
2 parents c9b5ee3 + 407b11f commit e51cf4c
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 20 deletions.
10 changes: 10 additions & 0 deletions add-on/src/lib/ipfs-companion.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ module.exports = async function init () {
function registerListeners () {
browser.webRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, {urls: ['<all_urls>']}, ['blocking', 'requestHeaders'])
browser.webRequest.onBeforeRequest.addListener(onBeforeRequest, {urls: ['<all_urls>']}, ['blocking'])
browser.webRequest.onHeadersReceived.addListener(onHeadersReceived, {urls: ['<all_urls>']}, ['blocking'])
browser.webRequest.onErrorOccurred.addListener(onErrorOccurred, {urls: ['<all_urls>']})
browser.storage.onChanged.addListener(onStorageChange)
browser.webNavigation.onCommitted.addListener(onNavigationCommitted)
browser.tabs.onUpdated.addListener(onUpdatedTab)
Expand Down Expand Up @@ -143,6 +145,14 @@ module.exports = async function init () {
return modifyRequest.onBeforeRequest(request)
}

function onHeadersReceived (request) {
return modifyRequest.onHeadersReceived(request)
}

function onErrorOccurred (request) {
return modifyRequest.onErrorOccurred(request)
}

// RUNTIME MESSAGES (one-off messaging)
// ===================================================================
// https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/sendMessage
Expand Down
32 changes: 30 additions & 2 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ const IsIpfs = require('is-ipfs')
const { urlAtPublicGw } = require('./ipfs-path')
const redirectOptOutHint = 'x-ipfs-companion-no-redirect'

// Tracking late redirects for edge cases such as https://github.com/ipfs-shipyard/ipfs-companion/issues/436
const onHeadersReceivedRedirect = new Set()

function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) {
// 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
Expand Down Expand Up @@ -65,7 +68,7 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) {
if (request.url.includes('/api/v0/add')) {
for (let header of request.requestHeaders) {
if (header.name === 'Connection') {
console.log('[ipfs-companion] Executing "Connection: close" workaround for https://github.com/ipfs/go-ipfs/issues/5168')
console.warn('[ipfs-companion] Executing "Connection: close" workaround for ipfs.files.add due to https://github.com/ipfs/go-ipfs/issues/5168')
header.value = 'close'
break
}
Expand All @@ -85,13 +88,37 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) {
return {
requestHeaders: request.requestHeaders
}
},

onHeadersReceived (request) {
// Fired when the HTTP response headers associated with a request have been received.
// You can use this event to modify HTTP response headers or do a very late 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)) {
const state = getState()
onHeadersReceivedRedirect.delete(request.requestId)
return redirectToGateway(request.url, state)
}
},

onErrorOccurred (request) {
// Fired when a request could not be processed due to an error:
// for example, a lack of Internet connectivity.

// Cleanup after https://github.com/ipfs-shipyard/ipfs-companion/issues/436
if (onHeadersReceivedRedirect.has(request.requestId)) {
onHeadersReceivedRedirect.delete(request.requestId)
}
}

}
}

exports.redirectOptOutHint = redirectOptOutHint
exports.createRequestModifier = createRequestModifier
exports.onHeadersReceivedRedirect = onHeadersReceivedRedirect

// types of requests to be skipped before any normalization happens
function preNormalizationSkip (state, request) {
Expand Down Expand Up @@ -149,7 +176,8 @@ function isSafeToRedirect (request, runtime) {
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)
console.warn('[ipfs-companion] 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
}
}
Expand Down
43 changes: 25 additions & 18 deletions test/functional/lib/ipfs-request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ const url2request = (string) => {
return {url: string, type: 'main_frame'}
}

const fakeRequestId = () => {
return Math.floor(Math.random() * 100000).toString()
}

const nodeTypes = ['external', 'embedded']

describe('modifyRequest.onBeforeRequest', function () {
Expand Down Expand Up @@ -111,11 +115,20 @@ describe('modifyRequest.onBeforeRequest', function () {
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 fetch 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 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'}
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 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('with embedded node', function () {
beforeEach(function () {
Expand All @@ -131,25 +144,19 @@ describe('modifyRequest.onBeforeRequest', function () {
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 fetch 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 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'}
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')
})
})
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.onBeforeRequest(xhrRequest)).to.equal(undefined)
})
it('should be served from public gateway via late redirect 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('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
})
})
})
Expand Down

0 comments on commit e51cf4c

Please sign in to comment.