From b7606ae3a05bbef8f52e3566f89c0060cadca7df Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Fri, 26 Oct 2018 16:17:58 +0200 Subject: [PATCH] fix: edge cases around localhost redirects (#607) - ignored requests are identified early and cached across all browser.webRequest.* hooks - global toggle now correctly disables all hooks and workarounds - Closes #604 --- add-on/src/lib/ipfs-request.js | 158 ++++++++++-------- .../lib/ipfs-request-gateway-redirect.test.js | 39 +++-- 2 files changed, 110 insertions(+), 87 deletions(-) diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index 879beff3d..fbfd66438 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/add-on/src/lib/ipfs-request.js @@ -1,6 +1,7 @@ 'use strict' /* eslint-env browser, webextensions */ +const LRU = require('lru-cache') const IsIpfs = require('is-ipfs') const { safeIpfsPath, pathAtHttpGateway } = require('./ipfs-path') const redirectOptOutHint = 'x-ipfs-companion-no-redirect' @@ -17,14 +18,45 @@ const recoverableErrors = new Set([ // 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) { - // 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 const browser = runtime.browser + + // Ignored requests are identified once and cached across all browser.webRequest hooks + const ignoredRequests = new LRU({ max: 128, maxAge: 1000 * 30 }) + const ignore = (id) => ignoredRequests.set(id, true) + const isIgnored = (id) => ignoredRequests.get(id) !== undefined + const preNormalizationSkip = (state, request) => { + // skip requests to the custom gateway or API (otherwise we have too much recursion) + if (request.url.startsWith(state.gwURLString) || request.url.startsWith(state.apiURLString)) { + ignore(request.requestId) + } + // skip websocket handshake (not supported by HTTP2IPFS gateways) + if (request.type === 'websocket') { + ignore(request.requestId) + } + // skip all local requests + if (request.url.startsWith('http://127.0.0.1') || request.url.startsWith('http://localhost') || request.url.startsWith('http://[::1]')) { + ignore(request.requestId) + } + return isIgnored(request.requestId) + } + const postNormalizationSkip = (state, request) => { + // skip requests to the public gateway if embedded node is running (otherwise we have too much recursion) + if (state.ipfsNodeType === 'embedded' && request.url.startsWith(state.pubGwURLString)) { + ignore(request.requestId) + // TODO: do not skip and redirect to `ipfs://` and `ipns://` if hasNativeProtocolHandler === true + } + return isIgnored(request.requestId) + } + + // Build RequestModifier return { + // browser.webRequest.onBeforeRequest + // This event is triggered when a request is about to be made, and before headers are available. + // This is a good place to listen if you want to cancel or redirect the request. onBeforeRequest (request) { - // This event is triggered when a request is about to be made, and before headers are available. - // This is a good place to listen if you want to cancel or redirect the request. const state = getState() // early sanity checks if (preNormalizationSkip(state, request)) { @@ -70,14 +102,17 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru } }, + // browser.webRequest.onBeforeSendHeaders + // This event is triggered before sending any HTTP data, but after all HTTP headers are available. + // This is a good place to listen if you want to modify HTTP request headers. onBeforeSendHeaders (request) { - // This event is triggered before sending any HTTP data, but after all HTTP headers are available. - // This is a good place to listen if you want to modify HTTP request headers. const state = getState() - // ignore websocket handshake (not supported by HTTP2IPFS gateways) - if (request.type === 'websocket') { + + // Skip if IPFS integrations are inactive or request is marked as ignored + if (!state.active || isIgnored(request.requestId)) { return } + if (request.url.startsWith(state.apiURLString)) { // There is a bug in go-ipfs related to keep-alive connections // that results in partial response for ipfs.files.add @@ -132,12 +167,18 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru } }, + // browser.webRequest.onHeadersReceived + // 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. 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. const state = getState() - if (state.active && state.redirect) { + // Skip if IPFS integrations are inactive or request is marked as ignored + if (!state.active || isIgnored(request.requestId)) { + return + } + + 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)) { @@ -204,42 +245,46 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru } }, + // browser.webRequest.onErrorOccurred + // Fired when a request could not be processed due to an error: + // for example, a lack of Internet connectivity. async onErrorOccurred (request) { - // Fired when a request could not be processed due to an error: - // for example, a lack of Internet connectivity. const state = getState() - if (state.active) { - // console.log('onErrorOccurred:' + request.error) - // console.log('onErrorOccurred', request) - // Check if error is final and can be recovered via DNSLink - const recoverableViaDnslink = - state.dnslinkPolicy && - request.type === 'main_frame' && - recoverableErrors.has(request.error) - if (recoverableViaDnslink && dnslinkResolver.canLookupURL(request.url)) { - // Explicit call to ignore global DNSLink policy and force DNS TXT lookup - const cachedDnslink = dnslinkResolver.readAndCacheDnslink(new URL(request.url).hostname) - const dnslinkRedirect = dnslinkResolver.dnslinkRedirect(request.url, cachedDnslink) - // We can't redirect in onErrorOccurred, so if DNSLink is present - // recover by opening IPNS version in a new tab - // TODO: add tests and demo - if (dnslinkRedirect) { - console.log(`[ipfs-companion] onErrorOccurred: recovering using dnslink for ${request.url}`, dnslinkRedirect) - const currentTabId = await browser.tabs.query({ active: true, currentWindow: true }).then(tabs => tabs[0].id) - await browser.tabs.create({ - active: true, - openerTabId: currentTabId, - url: dnslinkRedirect.redirectUrl - }) - } - } + // Skip if IPFS integrations are inactive or request is marked as ignored + if (!state.active || isIgnored(request.requestId)) { + return + } - // Cleanup after https://github.com/ipfs-shipyard/ipfs-companion/issues/436 - if (onHeadersReceivedRedirect.has(request.requestId)) { - onHeadersReceivedRedirect.delete(request.requestId) + // console.log('onErrorOccurred:' + request.error) + // console.log('onErrorOccurred', request) + // Check if error is final and can be recovered via DNSLink + const recoverableViaDnslink = + state.dnslinkPolicy && + request.type === 'main_frame' && + recoverableErrors.has(request.error) + if (recoverableViaDnslink && dnslinkResolver.canLookupURL(request.url)) { + // Explicit call to ignore global DNSLink policy and force DNS TXT lookup + const cachedDnslink = dnslinkResolver.readAndCacheDnslink(new URL(request.url).hostname) + const dnslinkRedirect = dnslinkResolver.dnslinkRedirect(request.url, cachedDnslink) + // We can't redirect in onErrorOccurred, so if DNSLink is present + // recover by opening IPNS version in a new tab + // TODO: add tests and demo + if (dnslinkRedirect) { + console.log(`[ipfs-companion] onErrorOccurred: recovering using dnslink for ${request.url}`, dnslinkRedirect) + const currentTabId = await browser.tabs.query({ active: true, currentWindow: true }).then(tabs => tabs[0].id) + await browser.tabs.create({ + active: true, + openerTabId: currentTabId, + url: dnslinkRedirect.redirectUrl + }) } } + + // Cleanup after https://github.com/ipfs-shipyard/ipfs-companion/issues/436 + if (onHeadersReceivedRedirect.has(request.requestId)) { + onHeadersReceivedRedirect.delete(request.requestId) + } } } @@ -249,37 +294,6 @@ exports.redirectOptOutHint = redirectOptOutHint exports.createRequestModifier = createRequestModifier exports.onHeadersReceivedRedirect = onHeadersReceivedRedirect -// types of requests to be skipped before any normalization happens -function preNormalizationSkip (state, request) { - // skip requests to the custom gateway or API (otherwise we have too much recursion) - if (request.url.startsWith(state.gwURLString) || request.url.startsWith(state.apiURLString)) { - return true - } - - // skip websocket handshake (not supported by HTTP2IPFS gateways) - if (request.type === 'websocket') { - return true - } - - // skip all local requests - if (request.url.startsWith('http://127.0.0.1:') || request.url.startsWith('http://localhost:') || request.url.startsWith('http://[::1]:')) { - return true - } - - return false -} - -// types of requests to be skipped after expensive normalization happens -function postNormalizationSkip (state, request) { - // skip requests to the public gateway if embedded node is running (otherwise we have too much recursion) - if (state.ipfsNodeType === 'embedded' && request.url.startsWith(state.pubGwURLString)) { - return true - // TODO: do not skip and redirect to `ipfs://` and `ipns://` if hasNativeProtocolHandler === true - } - - return false -} - function redirectToGateway (requestUrl, state, dnslinkResolver) { // TODO: redirect to `ipfs://` if hasNativeProtocolHandler === true const gateway = state.ipfsNodeType === 'embedded' ? state.pubGwURLString : state.gwURLString diff --git a/test/functional/lib/ipfs-request-gateway-redirect.test.js b/test/functional/lib/ipfs-request-gateway-redirect.test.js index eeb539784..c1e2c1325 100644 --- a/test/functional/lib/ipfs-request-gateway-redirect.test.js +++ b/test/functional/lib/ipfs-request-gateway-redirect.test.js @@ -19,6 +19,11 @@ const fakeRequestId = () => { return Math.floor(Math.random() * 100000).toString() } +const expectNoRedirect = (modifyRequest, request) => { + expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expect(modifyRequest.onHeadersReceived(request)).to.equal(undefined) +} + const nodeTypes = ['external', 'embedded'] describe('modifyRequest.onBeforeRequest:', function () { @@ -76,28 +81,28 @@ describe('modifyRequest.onBeforeRequest:', function () { it(`should be left untouched if redirect is disabled (${nodeType} node)`, function () { state.redirect = false const request = url2request('https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) it(`should be left untouched if redirect is enabled but global active flag is OFF (${nodeType} node)`, function () { state.active = false state.redirect = true const request = url2request('https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) it(`should be left untouched if URL includes opt-out hint (${nodeType} node)`, function () { // A safe way for preloading data at arbitrary gateways - it should arrive at original destination const request = url2request('https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?x-ipfs-companion-no-redirect#hashTest') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) expect(redirectOptOutHint).to.equal('x-ipfs-companion-no-redirect') }) it(`should be left untouched if CID is invalid (${nodeType} node)`, function () { const request = url2request('https://google.com/ipfs/notacid?argTest#hashTest') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) it(`should be left untouched if its is a HEAD preload with explicit opt-out in URL hash (${nodeType} node)`, function () { // HTTP HEAD is a popular way for preloading data at arbitrary gateways, so we have a dedicated test to make sure it works as expected const headRequest = { url: 'https://google.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#x-ipfs-companion-no-redirect', method: 'HEAD' } - expect(modifyRequest.onBeforeRequest(headRequest)).to.equal(undefined) + expectNoRedirect(modifyRequest, headRequest) }) }) }) @@ -214,12 +219,12 @@ describe('modifyRequest.onBeforeRequest:', function () { it(`should be left untouched if redirect is disabled' (${nodeType} node)`, function () { state.redirect = false const request = url2request('https://google.com/ipns/ipfs.io?argTest#hashTest') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) it(`should be left untouched if FQDN is not a real domain nor a valid CID (${nodeType} node)`, function () { const request = url2request('https://google.com/ipns/notafqdnorcid?argTest#hashTest') dnslinkResolver.readDnslinkFromTxtRecord = sinon.stub().returns(false) - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) }) }) @@ -229,27 +234,31 @@ describe('modifyRequest.onBeforeRequest:', function () { nodeTypes.forEach(function (nodeType) { beforeEach(function () { state.ipfsNodeType = nodeType + state.redirect = true }) describe(`with ${nodeType} node:`, function () { describe('request for IPFS path at a localhost', function () { // we do not touch local requests, as it may interfere with other nodes running at the same machine // or could produce false-positives such as redirection from 127.0.0.1:5001/ipfs/path to 127.0.0.1:8080/ipfs/path it('should be left untouched if 127.0.0.1 is used', function () { - state.redirect = true const request = url2request('http://127.0.0.1:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) it('should be left untouched if localhost is used', function () { // https://github.com/ipfs/ipfs-companion/issues/291 - state.redirect = true const request = url2request('http://localhost:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) + }) + it('should be left untouched if localhost is used, even when x-ipfs-path is present', function () { + // https://github.com/ipfs-shipyard/ipfs-companion/issues/604 + const request = url2request('http://localhost:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/') + request.responseHeaders = [{ name: 'X-Ipfs-Path', value: '/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ' }] + expectNoRedirect(modifyRequest, request) }) it('should be left untouched if [::1] is used', function () { // https://github.com/ipfs/ipfs-companion/issues/291 - state.redirect = true const request = url2request('http://[::1]:5001/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ/') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) }) @@ -259,12 +268,12 @@ describe('modifyRequest.onBeforeRequest:', function () { it('should be left untouched for IPFS', function () { state.redirect = true const request = url2request('http://bafybeigxjv2o4jse2lajbd5c7xxl5rluhyqg5yupln42252e5tcao7hbge.ipfs.dweb.link/') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) it('should be left untouched for IPNS', function () { state.redirect = true const request = url2request('http://bafybeigxjv2o4jse2lajbd5c7xxl5rluhyqg5yupln42252e5tcao7hbge.ipns.dweb.link/') - expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) + expectNoRedirect(modifyRequest, request) }) }) })