From db1a5f4e18bcf0ae4d0e503f1489cca344e651a6 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 3 Jul 2018 00:05:46 +0200 Subject: [PATCH 1/3] fix: restore gateway redirect for CORS XHR in Firefox Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/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. --- add-on/src/lib/ipfs-companion.js | 5 +++ add-on/src/lib/ipfs-request.js | 32 ++++++++++++++++-- test/functional/lib/ipfs-request.test.js | 43 ++++++++++++++---------- 3 files changed, 60 insertions(+), 20 deletions(-) diff --git a/add-on/src/lib/ipfs-companion.js b/add-on/src/lib/ipfs-companion.js index 2458c22a2..42fb5d1f1 100644 --- a/add-on/src/lib/ipfs-companion.js +++ b/add-on/src/lib/ipfs-companion.js @@ -90,6 +90,7 @@ module.exports = async function init () { function registerListeners () { browser.webRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, {urls: ['']}, ['blocking', 'requestHeaders']) browser.webRequest.onBeforeRequest.addListener(onBeforeRequest, {urls: ['']}, ['blocking']) + browser.webRequest.onHeadersReceived.addListener(onHeadersReceived, {urls: ['']}, ['blocking']) browser.storage.onChanged.addListener(onStorageChange) browser.webNavigation.onCommitted.addListener(onNavigationCommitted) browser.tabs.onUpdated.addListener(onUpdatedTab) @@ -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 diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index 0975a7dc4..9b3e951b2 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/add-on/src/lib/ipfs-request.js @@ -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 @@ -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 } @@ -85,6 +88,29 @@ 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) + } } } @@ -92,6 +118,7 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) { exports.redirectOptOutHint = redirectOptOutHint exports.createRequestModifier = createRequestModifier +exports.onHeadersReceivedRedirect = onHeadersReceivedRedirect // types of requests to be skipped before any normalization happens function preNormalizationSkip (state, request) { @@ -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 } } diff --git a/test/functional/lib/ipfs-request.test.js b/test/functional/lib/ipfs-request.test.js index 6be3e535c..0763da5d4 100644 --- a/test/functional/lib/ipfs-request.test.js +++ b/test/functional/lib/ipfs-request.test.js @@ -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 () { @@ -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 () { @@ -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') }) }) }) From 0f11332513a2415225ab8c304518b463cc5570cd Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 3 Jul 2018 00:25:15 +0200 Subject: [PATCH 2/3] fix: cleanup on webRequest.onErrorOccurred --- add-on/src/lib/ipfs-companion.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/add-on/src/lib/ipfs-companion.js b/add-on/src/lib/ipfs-companion.js index 42fb5d1f1..05e983b12 100644 --- a/add-on/src/lib/ipfs-companion.js +++ b/add-on/src/lib/ipfs-companion.js @@ -91,6 +91,7 @@ module.exports = async function init () { browser.webRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, {urls: ['']}, ['blocking', 'requestHeaders']) browser.webRequest.onBeforeRequest.addListener(onBeforeRequest, {urls: ['']}, ['blocking']) browser.webRequest.onHeadersReceived.addListener(onHeadersReceived, {urls: ['']}, ['blocking']) + browser.webRequest.onErrorOccurred.addListener(onErrorOccurred, {urls: ['']}) browser.storage.onChanged.addListener(onStorageChange) browser.webNavigation.onCommitted.addListener(onNavigationCommitted) browser.tabs.onUpdated.addListener(onUpdatedTab) @@ -148,6 +149,10 @@ module.exports = async function init () { 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 From 407b11f8e1c12a4a52a1d8b3e66ee65911ff8479 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 3 Jul 2018 14:59:41 +0200 Subject: [PATCH 3/3] style: clickable issue URLs in Browser Console https://github.com/ipfs-shipyard/ipfs-companion/pull/511#issuecomment-402063552 --- add-on/src/lib/ipfs-request.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index 9b3e951b2..8b1312cab 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/add-on/src/lib/ipfs-request.js @@ -68,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.warn('[ipfs-companion] Executing "Connection: close" workaround for 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 } @@ -176,7 +176,7 @@ 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] Delaying redirect of CORS XHR until onHeadersReceived due to ipfs-companion/issues/436:', request.url) + 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 }