Skip to content

Commit

Permalink
fix: decode content paths with decodeURI
Browse files Browse the repository at this point in the history
This avoids ibreaking IPFS content paths by converting them to URI params
  • Loading branch information
lidel committed Apr 3, 2020
1 parent 4ce869e commit 1acf899
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
7 changes: 7 additions & 0 deletions add-on/src/lib/http-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion add-on/src/lib/ipfs-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 18 additions & 6 deletions test/functional/lib/ipfs-request-gateway-redirect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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/`)
Expand All @@ -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/`)
})
Expand Down

0 comments on commit 1acf899

Please sign in to comment.