From 1acf89954a2ff08b4205ae35309353c520ea9575 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Wed, 1 Apr 2020 00:47:55 +0200 Subject: [PATCH] fix: decode content paths with decodeURI This avoids ibreaking IPFS content paths by converting them to URI params --- add-on/src/lib/http-proxy.js | 7 ++++++ add-on/src/lib/ipfs-path.js | 2 +- .../lib/ipfs-request-gateway-redirect.test.js | 24 ++++++++++++++----- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/add-on/src/lib/http-proxy.js b/add-on/src/lib/http-proxy.js index 59af54735..6ce25bad3 100644 --- a/add-on/src/lib/http-proxy.js +++ b/add-on/src/lib/http-proxy.js @@ -23,6 +23,13 @@ log.error = debug('ipfs-companion:http-proxy:error') // registerSubdomainProxy is necessary wourkaround for supporting subdomains // under 'localhost' (*.ipfs.localhost) because some operating systems do not // resolve them to local IP and return NX error not found instead +// +// State in Q2 2020: +// - Chromium hardcodes `localhost` name to point at local IP and proxy is not +// really necessary, but we do it just to be safe. +// - Firefox requires proxy to avoid DNS lookup, but there is an open issue +// that will remove that need at some point: +// https://bugzilla.mozilla.org/show_bug.cgi?id=1220810 async function registerSubdomainProxy (getState, runtime, notify) { try { const { useSubdomainProxy: enable, gwURLString } = getState() diff --git a/add-on/src/lib/ipfs-path.js b/add-on/src/lib/ipfs-path.js index ec15dc971..608ec429f 100644 --- a/add-on/src/lib/ipfs-path.js +++ b/add-on/src/lib/ipfs-path.js @@ -25,7 +25,7 @@ function ipfsContentPath (urlOrPath, opts) { // To get IPFS content path we need to reverse URI encoding of special // characters (https://github.com/ipfs/ipfs-companion/issues/303) - const contentPath = decodeURIComponent(url.pathname) + const contentPath = decodeURI(url.pathname) // End if not a content path if (!isIPFS.path(contentPath)) return null diff --git a/test/functional/lib/ipfs-request-gateway-redirect.test.js b/test/functional/lib/ipfs-request-gateway-redirect.test.js index 0f31f47ad..d833f4689 100644 --- a/test/functional/lib/ipfs-request-gateway-redirect.test.js +++ b/test/functional/lib/ipfs-request-gateway-redirect.test.js @@ -259,6 +259,10 @@ describe('modifyRequest.onBeforeRequest:', function () { const cid = 'bafybeigxjv2o4jse2lajbd5c7xxl5rluhyqg5yupln42252e5tcao7hbge' const peerid = 'bafzbeigxjv2o4jse2lajbd5c7xxl5rluhyqg5yupln42252e5tcao7hbge' + // Tests use different CID in X-Ipfs-Path header just to ensure it does not + // override the one from path + const fakeXIpfsPathHdrVal = '/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ' + describe('with external node', function () { beforeEach(function () { state.ipfsNodeType = 'external' @@ -269,21 +273,29 @@ describe('modifyRequest.onBeforeRequest:', function () { const request = url2request(`https://${cid}.ipfs.dweb.link/`) // X-Ipfs-Path to ensure value from URL takes a priority - request.responseHeaders = [{ name: 'X-Ipfs-Path', value: '/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ' }] + request.responseHeaders = [{ name: 'X-Ipfs-Path', value: fakeXIpfsPathHdrVal }] /// We expect redirect to path-based gateway because go-ipfs >=0.5 will - // return redirect to subdomain, and we don't want to break older - // versions + // return redirect to a subdomain, and we don't want to break users + // running older versions of go-ipfs by loading subdomain first and + // failing. expect(modifyRequest.onBeforeRequest(request).redirectUrl) .to.equal(`http://localhost:8080/ipfs/${cid}/`) }) it('should be redirected to localhost gateway (*.ipfs on 3rd party gw)', function () { state.redirect = true const request = url2request(`https://${cid}.ipfs.cf-ipfs.com/`) - request.responseHeaders = [{ name: 'X-Ipfs-Path', value: '/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ' }] + request.responseHeaders = [{ name: 'X-Ipfs-Path', value: fakeXIpfsPathHdrVal }] expect(modifyRequest.onBeforeRequest(request).redirectUrl) .to.equal(`http://localhost:8080/ipfs/${cid}/`) }) + it('should be redirected to localhost gateway and keep URL encoding of original path', function () { + state.redirect = true + const request = url2request('https://bafybeigfejjsuq5im5c3w3t3krsiytszhfdc4v5myltcg4myv2n2w6jumy.ipfs.dweb.link/%3Ffilename=test.jpg?arg=val') + request.responseHeaders = [{ name: 'X-Ipfs-Path', value: fakeXIpfsPathHdrVal }] + expect(modifyRequest.onBeforeRequest(request).redirectUrl) + .to.equal('http://localhost:8080/ipfs/bafybeigfejjsuq5im5c3w3t3krsiytszhfdc4v5myltcg4myv2n2w6jumy/%3Ffilename=test.jpg?arg=val') + }) it('should be redirected to localhost gateway (*.ipns on default gw)', function () { state.redirect = true const request = url2request(`https://${peerid}.ipns.dweb.link/`) @@ -301,13 +313,13 @@ describe('modifyRequest.onBeforeRequest:', function () { it('should be left untouched for *.ipfs at default public subdomain gw', function () { state.redirect = true const request = url2request(`https://${cid}.ipfs.dweb.link/`) - request.responseHeaders = [{ name: 'X-Ipfs-Path', value: '/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ' }] + request.responseHeaders = [{ name: 'X-Ipfs-Path', value: fakeXIpfsPathHdrVal }] expectNoRedirect(modifyRequest, request) }) it('should be redirected to user-prefered public gateway if 3rd party subdomain gw', function () { state.redirect = true const request = url2request(`https://${cid}.ipfs.cf-ipfs.com/`) - request.responseHeaders = [{ name: 'X-Ipfs-Path', value: '/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ' }] + request.responseHeaders = [{ name: 'X-Ipfs-Path', value: fakeXIpfsPathHdrVal }] expect(modifyRequest.onBeforeRequest(request).redirectUrl) .to.equal(`https://${cid}.ipfs.dweb.link/`) })