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: redirect to native URI in Brave #960

Merged
merged 1 commit into from
Jan 17, 2021
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
12 changes: 10 additions & 2 deletions add-on/src/lib/ipfs-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ function ipfsContentPath (urlOrPath, opts) {
}
exports.ipfsContentPath = ipfsContentPath

// Turns URL or URIencoded path into a ipfs:// or ipns:// URI
function ipfsUri (urlOrPath) {
const contentPath = ipfsContentPath(urlOrPath, { keepURIParams: true })
if (!contentPath) return null
return contentPath.replace(/^\/(ip[f|n]s)\//, '$1://')
}
exports.ipfsUri = ipfsUri

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you verbify the function name in a way that makes what it does more explicit?


function subdomainPatternMatch (url) {
if (typeof url === 'string') {
url = new URL(url)
Expand Down Expand Up @@ -245,8 +253,8 @@ function createIpfsPathValidator (getState, getIpfs, dnslinkResolver) {
},

// Version of resolveToPublicUrl that always resolves to URL representing
// path gateway at local machine (This is ok, as subdomain will redirect
// to corre
// path gateway at local machine (This is ok, as localhost gw will redirect
// to correct subdomain)
resolveToLocalUrl (urlOrPath) {
const { gwURLString } = getState()
const ipfsPath = ipfsContentPath(urlOrPath, { keepURIParams: true })
Expand Down
38 changes: 26 additions & 12 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ log.error = debug('ipfs-companion:request:error')
const LRU = require('lru-cache')
const isIPFS = require('is-ipfs')
const isFQDN = require('is-fqdn')
const { pathAtHttpGateway, sameGateway } = require('./ipfs-path')
const { pathAtHttpGateway, sameGateway, ipfsUri } = require('./ipfs-path')
const { safeURL } = require('./options')
const { braveNodeType } = require('./ipfs-client/brave')

const redirectOptOutHint = 'x-ipfs-companion-no-redirect'
const recoverableNetworkErrors = new Set([
Expand Down Expand Up @@ -184,10 +185,9 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
// Detect dnslink using heuristics enabled in Preferences
if (state.dnslinkPolicy && dnslinkResolver.canLookupURL(request.url)) {
if (state.dnslinkRedirect) {
const redirectUrl = dnslinkResolver.dnslinkAtGateway(request.url)
if (redirectUrl && isSafeToRedirect(request, runtime)) {
// console.log('onBeforeRequest.dnslinkRedirect', dnslinkRedirect)
return { redirectUrl }
const dnslinkAtGw = dnslinkResolver.dnslinkAtGateway(request.url)
if (dnslinkAtGw && isSafeToRedirect(request, runtime)) {
return redirectToGateway(request, dnslinkAtGw, state, ipfsPathValidator, runtime)
}
} else if (state.dnslinkDataPreload) {
dnslinkResolver.preloadData(request.url)
Expand Down Expand Up @@ -299,9 +299,9 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
if (runtime.requiresXHRCORSfix && onHeadersReceivedRedirect.has(request.requestId)) {
onHeadersReceivedRedirect.delete(request.requestId)
if (state.dnslinkPolicy) {
const redirectUrl = dnslinkResolver.dnslinkAtGateway(request.url)
if (redirectUrl) {
return { redirectUrl }
const dnslinkAtGw = dnslinkResolver.dnslinkAtGateway(request.url)
if (dnslinkAtGw) {
return redirectToGateway(request, dnslinkAtGw, state, ipfsPathValidator, runtime)
}
}
return redirectToGateway(request, request.url, state, ipfsPathValidator, runtime)
Expand All @@ -326,12 +326,12 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
// in a way that works even when state.dnslinkPolicy !== 'enabled'
// All the following requests will be upgraded to IPNS
const cachedDnslink = dnslinkResolver.readAndCacheDnslink(new URL(request.url).hostname)
const redirectUrl = dnslinkResolver.dnslinkAtGateway(request.url, cachedDnslink)
const dnslinkAtGw = dnslinkResolver.dnslinkAtGateway(request.url, cachedDnslink)
// redirect only if local node is around, as we can't guarantee DNSLink support
// at a public subdomain gateway (requires more than 1 level of wildcard TLS certs)
if (redirectUrl && state.localGwAvailable) {
log(`onHeadersReceived: dnslinkRedirect from ${request.url} to ${redirectUrl}`)
return { redirectUrl }
if (dnslinkAtGw && state.localGwAvailable) {
log(`onHeadersReceived: dnslinkRedirect from ${request.url} to ${dnslinkAtGw}`)
return redirectToGateway(request, dnslinkAtGw, state, ipfsPathValidator, runtime)
}
}
// Additional validation of X-Ipfs-Path
Expand Down Expand Up @@ -398,6 +398,7 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
if (dnslink) {
const redirectUrl = dnslinkResolver.dnslinkAtGateway(request.url, dnslink)
log(`onErrorOccurred: attempting to recover from network error (${request.error}) using dnslink for ${request.url} → ${redirectUrl}`, request)
// We are unable to redirect in onErrorOccurred, but we can update the tab
return updateTabWithURL(request, redirectUrl, browser)
}
}
Expand All @@ -412,6 +413,7 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
if (isRecoverable(request, state, ipfsPathValidator)) {
const redirectUrl = ipfsPathValidator.resolveToPublicUrl(request.url)
log(`onErrorOccurred: attempting to recover from network error (${request.error}) for ${request.url} → ${redirectUrl}`, request)
// We are unable to redirect in onErrorOccurred, but we can update the tab
return updateTabWithURL(request, redirectUrl, browser)
}
},
Expand Down Expand Up @@ -484,6 +486,18 @@ function redirectToGateway (request, url, state, ipfsPathValidator, runtime) {
const useLocalhostName = false
redirectUrl = safeURL(redirectUrl, { useLocalhostName }).toString()
}
// Leverage native URI support in Brave for nice address bar.
if (type === 'main_frame' && state.ipfsNodeType === braveNodeType && !sameGateway(request.url, state.gwURL)) {
redirectUrl = ipfsUri(redirectUrl)
// In Brave 1.20.54 a webRequest redirect pointing at ipfs:// URI
// is not reflected in address bar - a http://*.localhost URL is displayed instead.
// but tabs.update works, so we do that for the main request.
if (redirectUrl !== url) { // futureproofing in case url from request becomes native
log('redirectToGateway: upgrading address bar to native URI', redirectUrl)
// manually set tab to native URI
return runtime.browser.tabs.update(request.tabId, { url: redirectUrl })
}
}
}

// return a redirect only if URL changed
Expand Down
2 changes: 1 addition & 1 deletion test/functional/lib/dnslink.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function spoofDnsTxtRecord (fqdn, dnslinkResolver, value) {
module.exports.spoofDnsTxtRecord = spoofDnsTxtRecord

function spoofCachedDnslink (fqdn, dnslinkResolver, value) {
// spoofs existence of valid DNS TXT record (used on cache miss)
// spoofs existence of valid DNS TXT record (used on cache hit)
dnslinkResolver.setDnslink(fqdn, value)
}
module.exports.spoofCachedDnslink = spoofCachedDnslink
Expand Down
51 changes: 50 additions & 1 deletion test/functional/lib/ipfs-path.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const { stub } = require('sinon')
const { describe, it, beforeEach, afterEach } = require('mocha')
const { expect } = require('chai')
const { URL } = require('url')
const { ipfsContentPath, createIpfsPathValidator, sameGateway } = require('../../../add-on/src/lib/ipfs-path')
const { ipfsUri, ipfsContentPath, createIpfsPathValidator, sameGateway } = require('../../../add-on/src/lib/ipfs-path')
const { initState } = require('../../../add-on/src/lib/state')
const createDnslinkResolver = require('../../../add-on/src/lib/dnslink')
const { optionDefaults } = require('../../../add-on/src/lib/options')
Expand Down Expand Up @@ -41,6 +41,7 @@ describe('ipfs-path.js', function () {
if (ipfs.resolve.reset) ipfs.resolve.reset()
})

// TODO: move to some lib?
describe('ipfsContentPath', function () {
it('should detect /ipfs/ path in URL from a public gateway', function () {
const url = 'https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar'
Expand Down Expand Up @@ -96,6 +97,54 @@ describe('ipfs-path.js', function () {
})
})

// TODO: move to some lib?
describe('ipfsUri', function () {
it('should detect /ipfs/ path in URL from a public gateway', function () {
const url = 'https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar'
expect(ipfsUri(url)).to.equal('ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar')
})
it('should detect /ipfs/ path in detached IPFS path', function () {
const path = '/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar'
expect(ipfsUri(path)).to.equal('ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar')
})
it('should detect /ipns/ path in URL from a public gateway', function () {
const url = 'https://ipfs.io/ipns/libp2p.io/bundles/'
expect(ipfsUri(url)).to.equal('ipns://libp2p.io/bundles/')
})
it('should detect /ipns/ path in detached IPFS path', function () {
const path = '/ipns/libp2p.io/bundles/'
expect(ipfsUri(path)).to.equal('ipns://libp2p.io/bundles/')
})
it('should keep search and hash from original URL', function () {
const url = 'https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest'
expect(ipfsUri(url)).to.equal('ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
})
it('should preserve search and hash in detached IPFS path', function () {
const path = '/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest'
expect(ipfsUri(path)).to.equal('ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest')
})
it('should decode special characters in URL', function () {
const url = 'https://ipfs.io/ipfs/Qmb8wsGZNXt5VXZh1pEmYynjB6Euqpq3HYyeAdw2vScTkQ/1%20-%20Barrel%20-%20Part%201'
expect(ipfsUri(url)).to.equal('ipfs://Qmb8wsGZNXt5VXZh1pEmYynjB6Euqpq3HYyeAdw2vScTkQ/1 - Barrel - Part 1')
})
it('should decode special characters in path', function () {
const path = '/ipfs/Qmb8wsGZNXt5VXZh1pEmYynjB6Euqpq3HYyeAdw2vScTkQ/1%20-%20Barrel%20-%20Part%201'
expect(ipfsUri(path)).to.equal('ipfs://Qmb8wsGZNXt5VXZh1pEmYynjB6Euqpq3HYyeAdw2vScTkQ/1 - Barrel - Part 1')
})
it('should resolve CID-in-subdomain URL to IPFS path', function () {
const url = 'https://bafybeicgmdpvw4duutrmdxl4a7gc52sxyuk7nz5gby77afwdteh3jc5bqa.ipfs.dweb.link/wiki/Mars.html?argTest#hashTest'
expect(ipfsUri(url)).to.equal('ipfs://bafybeicgmdpvw4duutrmdxl4a7gc52sxyuk7nz5gby77afwdteh3jc5bqa/wiki/Mars.html?argTest#hashTest')
})
it('should return null if there is no valid path for input URL', function () {
const url = 'https://foo.io/invalid/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest'
expect(ipfsUri(url)).to.equal(null)
})
it('should return null if there is no valid path for input path', function () {
const path = '/invalid/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR'
expect(ipfsUri(path)).to.equal(null)
})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of positive tests, should add negative tests too

})

describe('sameGateway', function () {
it('should return true on direct host match', function () {
const url = 'https://127.0.0.1:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar'
Expand Down
30 changes: 29 additions & 1 deletion test/functional/lib/ipfs-request-workarounds.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const { createRequestModifier } = require('../../../add-on/src/lib/ipfs-request'
const createDNSLinkResolver = require('../../../add-on/src/lib/dnslink')
const { createIpfsPathValidator } = require('../../../add-on/src/lib/ipfs-path')
const { optionDefaults } = require('../../../add-on/src/lib/options')
const { braveNodeType } = require('../../../add-on/src/lib/ipfs-client/brave')
const { spoofDnsTxtRecord } = require('./dnslink.test.js')

// const nodeTypes = ['external', 'embedded']

Expand Down Expand Up @@ -282,18 +284,44 @@ describe('modifyRequest processing', function () {
modifyRequest = createRequestModifier(getState, dnslinkResolver, ipfsPathValidator, runtime)
// test
const request = {
tabId: 42404,
statusCode: 404,
type: 'main_frame',
url: brokenDNSLinkUrl
}
browser.tabs.update.flush()
assert.ok(browser.tabs.update.notCalled)
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: fixedDNSLinkUrl }).notCalled)
modifyRequest.onCompleted(request)
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: fixedDNSLinkUrl }).calledOnce)
browser.tabs.update.flush()
})
})

// Brave seems to ignore redirect to ipfs:// and ipns://, but if we force tab update via tabs API,
// then address bar is correct
describe('redirect of main_frame request to local gateway when Brave node is used', function () {
it('should force native URI in address bar via tabs.update API', async function () {
const httpDNSLinkUrl = 'https://example.com/ipns/docs.ipfs.io/some/path?query=val'
const nativeDNSLinkUri = 'ipns://docs.ipfs.io/some/path?query=val'
spoofDnsTxtRecord('docs.ipfs.io', dnslinkResolver, '/ipfs/bafkqaaa')
state.ipfsNodeType = braveNodeType
// ensure clean modifyRequest
runtime = Object.assign({}, await createRuntimeChecks(browser)) // make it mutable for tests
modifyRequest = createRequestModifier(getState, dnslinkResolver, ipfsPathValidator, runtime)
// test
const request = {
tabId: 42,
type: 'main_frame',
url: httpDNSLinkUrl
}
browser.tabs.update.flush()
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: nativeDNSLinkUri }).notCalled)
await modifyRequest.onBeforeRequest(request)
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: nativeDNSLinkUri }).calledOnce)
browser.tabs.update.flush()
})
})

after(function () {
delete global.URL
delete global.browser
Expand Down