Skip to content

Commit

Permalink
fix: restore gateway redirect for CORS XHR in Firefox
Browse files Browse the repository at this point in the history
Context for CORS XHR problems in Firefox:
#436

In short, onBeforeRequest should not change anything, as it will trigger
false-positive CORS error.  onHeadersReceived is after CORS validation
happens, so its ok to cancel and redirect late.

This is not ideal, as there is an outgoing request to the public
gateway, and we need to read response headers before connection is
aborted, but we can't do better than that until
https://bugzilla.mozilla.org/show_bug.cgi?id=1450965 is resolved.
  • Loading branch information
lidel committed Jul 2, 2018
1 parent c9b5ee3 commit db1a5f4
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 20 deletions.
5 changes: 5 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,7 @@ 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.storage.onChanged.addListener(onStorageChange)
browser.webNavigation.onCommitted.addListener(onNavigationCommitted)
browser.tabs.onUpdated.addListener(onUpdatedTab)
Expand Down Expand Up @@ -143,6 +144,10 @@ module.exports = async function init () {
return modifyRequest.onBeforeRequest(request)
}

function onHeadersReceived (request) {
return modifyRequest.onHeadersReceived(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 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 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 db1a5f4

Please sign in to comment.