Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove XHR CORS workaround for Firefox #771

Merged
merged 1 commit into from
Oct 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"@babel/preset-env",
{
"targets": {
"firefox": 67,
"firefox": 69,
"chrome": 70
}
}
Expand Down
7 changes: 0 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,6 @@ We are also available at the [#ipfs](https://webchat.freenode.net/?channels=ipfs

Questions specific to this browser companion can be asked directly at [`#ipfs-in-web-browsers`](https://webchat.freenode.net/?channels=ipfs-in-web-browsers)

#### Cross-Origin XHR are executed "twice" in Firefox

Due to CORS bug XHRs in Firefox are handled via late redirects in `onHeadersReceived`.
Original request is cancelled after response headers are received, so there is no overhead of reading response payload twice.

For more details on this see [PR #511](https://github.com/ipfs-shipyard/ipfs-companion/pull/511).

#### Upload via Right-Click Does Not Work in Firefox

See [this workaround](https://github.com/ipfs/ipfs-companion/issues/227).
Expand Down
2 changes: 1 addition & 1 deletion add-on/manifest.firefox.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"applications": {
"gecko": {
"id": "ipfs-firefox-addon@lidel.org",
"strict_min_version": "67.0"
"strict_min_version": "69.0"
}
},
"page_action": {
Expand Down
37 changes: 0 additions & 37 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ const recoverableErrors = new Set([
'net::ERR_INTERNET_DISCONNECTED' // no network
])

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

// 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
function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, runtime) {
Expand Down Expand Up @@ -316,19 +313,6 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
}

if (state.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)) {
onHeadersReceivedRedirect.delete(request.requestId)
if (state.dnslinkPolicy) {
const dnslinkRedirect = dnslinkResolver.dnslinkRedirect(request.url)
if (dnslinkRedirect) {
return dnslinkRedirect
}
}
return redirectToGateway(request.url, state, ipfsPathValidator)
}

// Detect X-Ipfs-Path Header and upgrade transport to IPFS:
// 1. Check if DNSLink exists and redirect to it.
// 2. If there is no DNSLink, validate path from the header and redirect
Expand Down Expand Up @@ -417,19 +401,13 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
})
}
}

// 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

function redirectToGateway (requestUrl, state, ipfsPathValidator) {
// TODO: redirect to `ipfs://` if hasNativeProtocolHandler === true
Expand All @@ -450,21 +428,6 @@ function isSafeToRedirect (request, runtime) {
return false
}

// 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' && !request.responseHeaders) {
// 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) {
log('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
}
}
}
return true
}

Expand Down
17 changes: 5 additions & 12 deletions test/functional/lib/ipfs-request-dnslink.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ describe('modifyRequest processing', function () {
const request = url2request('http://explore.ipld.io/index.html?argTest#hashTest')
expect(modifyRequest.onBeforeRequest(request).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest')
})
it('should redirect in onBeforeRequest if DNS TXT record exists, XHR is cross-origin and runtime is not Firefox', function () {
it('should redirect in onBeforeRequest if DNS TXT record exists, XHR is cross-origin and runtime is Chromium', function () {
// stub existence of a valid DNS record
const fqdn = 'explore.ipld.io'
dnslinkResolver.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd')
Expand All @@ -173,19 +173,15 @@ describe('modifyRequest processing', function () {
const xhrRequest = { url: 'http://explore.ipld.io/index.html?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() }
expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest')
})
it('should redirect later in onHeadersReceived if dnslink exists, XHR is cross-origin and runtime is Firefox', function () {
it('should redirect in onBeforeRequest if dnslink exists, XHR is cross-origin and runtime is Firefox', function () {
// stub existence of a valid DNS record
const fqdn = 'explore.ipld.io'
dnslinkResolver.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd')
//
// Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
runtime.isFirefox = true
// Firefox uses 'originUrl' for origin
const xhrRequest = { url: 'http://explore.ipld.io/index.html?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(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest')
expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest')
})
it('should do nothing if dnslink does not exist and XHR is cross-origin in Firefox', function () {
// stub no dnslink
Expand Down Expand Up @@ -325,18 +321,15 @@ describe('modifyRequest processing', function () {
const xhrRequest = { url: 'http://explore.ipld.io/index.html?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId() }
expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest')
})
it('should redirect later in onHeadersReceived if XHR is cross-origin and runtime is Firefox', function () {
it('should redirect in onBeforeRequest if XHR is cross-origin and runtime is Firefox', function () {
// stub existence of a valid DNS record
const fqdn = 'explore.ipld.io'
dnslinkResolver.setDnslink(fqdn, '/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd')
//
// Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
runtime.isFirefox = true
const xhrRequest = { url: 'http://explore.ipld.io/index.html?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(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest')
expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/explore.ipld.io/index.html?argTest#hashTest')
})
it('should do nothing if DNS TXT record is missing and XHR is cross-origin in Firefox', function () {
// stub cached info about lack of dnslink
Expand Down
24 changes: 8 additions & 16 deletions test/functional/lib/ipfs-request-gateway-redirect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,24 +119,20 @@ describe('modifyRequest.onBeforeRequest:', function () {
const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', originUrl: '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 fetched from the same origin and redirect is enabled in non-Firefox', function () {
it('should be served from custom gateway if fetched from the same origin and redirect is enabled in Chromium', function () {
runtime.isFirefox = false
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 XHR 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 Chromium', function () {
runtime.isFirefox = false
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 in onHeadersReceived 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
it('should be served from custom gateway if XHR is cross-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://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')
expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('http://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
})
})
describe('with embedded node', function () {
Expand All @@ -148,24 +144,20 @@ describe('modifyRequest.onBeforeRequest:', function () {
const xhrRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest', type: 'xmlhttprequest', originUrl: '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 fetched from the same origin and redirect is enabled in non-Firefox', function () {
it('should be served from public gateway if fetched from the same origin and redirect is enabled in Chromium', function () {
runtime.isFirefox = false
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 XHR 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 Chromium', function () {
runtime.isFirefox = false
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')
})
it('should be served from public gateway via late redirect in onHeadersReceived 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
it('should be served from public gateway if XHR is cross-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://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')
expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
})
})
})
Expand Down