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: restore ESR compatibility #812

Merged
merged 1 commit into from
Nov 11, 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": 69,
"firefox": 68,
"chrome": 72
}
}
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": "69.0"
"strict_min_version": "68.0"
}
},
"page_action": {
Expand Down
35 changes: 35 additions & 0 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ const recoverableNetworkErrors = new Set([
])
const recoverableHttpError = (code) => code && code >= 400

// 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 @@ -311,6 +314,19 @@ 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 (runtime.requiresXHRCORSfix && 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 @@ -404,6 +420,11 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
}
}

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

// Check if error can be recovered by opening same content-addresed path
// using active gateway (public or local, depending on redirect state)
if (isRecoverable(request, state, ipfsPathValidator)) {
Expand Down Expand Up @@ -463,6 +484,20 @@ 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
if (runtime.requiresXHRCORSfix && 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
23 changes: 11 additions & 12 deletions add-on/src/lib/runtime-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ function getBrowserInfo (browser) {
if (browser && browser.runtime && browser.runtime.getBrowserInfo) {
return browser.runtime.getBrowserInfo()
}
return Promise.resolve()
return Promise.resolve({})
}

function getPlatformInfo (browser) {
Expand All @@ -28,21 +28,20 @@ function hasChromeSocketsForTcp () {

async function createRuntimeChecks (browser) {
// browser
const browserInfo = await getBrowserInfo(browser)
const runtimeBrowserName = browserInfo ? browserInfo.name : 'unknown'
const runtimeIsFirefox = !!(runtimeBrowserName.includes('Firefox') || runtimeBrowserName.includes('Fennec'))
const runtimeHasNativeProtocol = !!(browser && browser.protocol && browser.protocol.registerStringProtocol)
const { name, version } = await getBrowserInfo(browser)
const isFirefox = name && (name.includes('Firefox') || name.includes('Fennec'))
const hasNativeProtocolHandler = !!(browser && browser.protocol && browser.protocol.registerStringProtocol)
// platform
const platformInfo = await getPlatformInfo(browser)
const runtimeIsAndroid = platformInfo ? platformInfo.os === 'android' : false
const runtimeHasSocketsForTcp = hasChromeSocketsForTcp()
const isAndroid = platformInfo ? platformInfo.os === 'android' : false
return Object.freeze({
browser,
isFirefox: runtimeIsFirefox,
isAndroid: runtimeIsAndroid,
isBrave: runtimeHasSocketsForTcp, // TODO: make it more robust
hasChromeSocketsForTcp: runtimeHasSocketsForTcp,
hasNativeProtocolHandler: runtimeHasNativeProtocol
isFirefox,
isAndroid,
isBrave: hasChromeSocketsForTcp(), // TODO: make it more robust
requiresXHRCORSfix: !!(isFirefox && version && version.startsWith('68')),
hasChromeSocketsForTcp: hasChromeSocketsForTcp(),
hasNativeProtocolHandler
})
}

Expand Down
27 changes: 27 additions & 0 deletions test/functional/lib/ipfs-request-dnslink.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,20 @@ describe('modifyRequest processing', function () {
const xhrRequest = { url: 'http://explore.ipld.io/index.html?argTest#hashTest', type: 'xmlhttprequest', originUrl: '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 <69', 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 <69: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
runtime.requiresXHRCORSfix = 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')
})
it('should do nothing if dnslink does not exist and XHR is cross-origin in Firefox', function () {
// stub no dnslink
const fqdn = 'youtube.com'
Expand Down Expand Up @@ -286,6 +300,19 @@ describe('modifyRequest processing', function () {
xhrRequest.responseHeaders = [{ name: 'X-Ipfs-Path', value: '/ipfs/QmbfimSwTuCvGL8XBr3yk1iCjqgk2co2n21cWmcQohymDd' }]
expect(modifyRequest.onHeadersReceived(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 <69', 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 <69: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
runtime.requiresXHRCORSfix = 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')
})
// Test makes more sense for dnslinkPolicy=enabled, but we keep it here for completeness
it('should do nothing if DNS TXT record is missing and XHR is cross-origin in Firefox', function () {
// stub no dnslink
Expand Down
30 changes: 30 additions & 0 deletions test/functional/lib/ipfs-request-gateway-redirect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,36 @@ describe('modifyRequest.onBeforeRequest:', function () {
expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
})
})
describe('with external node when runtime.requiresXHRCORSfix', function () {
beforeEach(function () {
state.ipfsNodeType = 'external'
browser.runtime.getBrowserInfo = () => Promise.resolve({ name: 'Firefox', version: '68.0.0' })
})
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.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 () {
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 () {
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
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('request for a path matching /ipns/{path}', function () {
Expand Down
33 changes: 25 additions & 8 deletions test/functional/lib/runtime-checks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { describe, it, before, beforeEach, after } = require('mocha')
const { expect } = require('chai')
const browser = require('sinon-chrome')
const { createRuntimeChecks, hasChromeSocketsForTcp } = require('../../../add-on/src/lib/runtime-checks')
const promiseStub = (result) => () => Promise.resolve(result)

describe('runtime-checks.js', function () {
before(() => {
Expand All @@ -21,14 +22,6 @@ describe('runtime-checks.js', function () {
browser.flush()
})

// current version of sinon-chrome is missing stubs for some APIs,
// this is a workaround until fix is provided upstream
function promiseStub (result) {
return () => {
return Promise.resolve(result)
}
}

it('should return true when in Firefox runtime', async function () {
browser.runtime.getBrowserInfo = promiseStub({ name: 'Firefox' })
const runtime = await createRuntimeChecks(browser)
Expand All @@ -42,6 +35,30 @@ describe('runtime-checks.js', function () {
})
})

describe('requiresXHRCORSfix', function () {
beforeEach(function () {
browser.flush()
})

it('should return true when in Firefox runtime < 69', async function () {
browser.runtime.getBrowserInfo = promiseStub({ name: 'Firefox', version: '68.0.0' })
const runtime = await createRuntimeChecks(browser)
expect(runtime.requiresXHRCORSfix).to.equal(true)
})

it('should return false when in Firefox runtime >= 69', async function () {
browser.runtime.getBrowserInfo = promiseStub({ name: 'Firefox', version: '69.0.0' })
const runtime = await createRuntimeChecks(browser)
expect(runtime.requiresXHRCORSfix).to.equal(false)
})

it('should return false when if getBrowserInfo is not present', async function () {
browser.runtime.getBrowserInfo = undefined
const runtime = await createRuntimeChecks(browser)
expect(runtime.requiresXHRCORSfix).to.equal(false)
})
})

describe('isAndroid', function () {
beforeEach(function () {
browser.flush()
Expand Down