Skip to content

Commit

Permalink
Merge pull request #516 from ipfs-shipyard/fix/dnslink-breaking-cors-xhr
Browse files Browse the repository at this point in the history 
Dnslink lookup is disabled by default, but if user opted-in in latest version websites were broken due to false-positive late gateway redirects of XHR requests.

This PR  fixes dnslink regression introduced by #511
and adds tests to guard against the problem in future.

Some deeper refactoring is due, but that is something for separate PR.
I want to land this PR as soon as possible, so that dnslink works again.
  • Loading branch information
@lidel
lidel authored Jul 5, 2018
2 parents 999682b + a967a62 commit 99d9f12
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 12 deletions.
22 changes: 17 additions & 5 deletions add-on/src/lib/dns-link.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,22 @@ module.exports = function createDnsLink (getState) {
let dnslink = cache.get(fqdn)
if (typeof dnslink === 'undefined') {
try {
console.info('dnslink cache miss for: ' + fqdn)
console.info(`[ipfs-companion] dnslink cache miss for '${fqdn}', running DNS TXT lookup`)
dnslink = dnsLink.readDnslinkFromTxtRecord(fqdn)
if (dnslink) {
cache.set(fqdn, dnslink)
console.info(`Resolved dnslink: '${fqdn}' -> '${dnslink}'`)
console.info(`[ipfs-companion] found dnslink: '${fqdn}' -> '${dnslink}'`)
} else {
cache.set(fqdn, false)
console.info(`Resolved NO dnslink for '${fqdn}'`)
console.info(`[ipfs-companion] found NO dnslink for '${fqdn}'`)
}
} catch (error) {
console.error(`Error in dnslinkLookupAndOptionalRedirect for '${fqdn}'`)
console.error(`[ipfs-companion] Error in dnslinkLookupAndOptionalRedirect for '${fqdn}'`)
console.error(error)
}
} else {
console.info(`Resolved via cached dnslink: '${fqdn}' -> '${dnslink}'`)
// Most of the time we will hit cache, which makes below line is too noisy
// console.info(`[ipfs-companion] using cached dnslink: '${fqdn}' -> '${dnslink}'`)
}
return dnslink
},
Expand Down Expand Up @@ -84,6 +85,17 @@ module.exports = function createDnsLink (getState) {
}
},

canRedirectToIpns (url) {
if (!url.pathname.startsWith('/ipfs/') && !url.pathname.startsWith('/ipns/')) {
const fqdn = url.hostname
const dnslink = dnsLink.cachedDnslinkLookup(fqdn)
if (dnslink) {
return true
}
}
return false
},

redirectToIpnsPath (originalUrl) {
// TODO: redirect to `ipns://` if hasNativeProtocolHandler === true
const fqdn = originalUrl.hostname
Expand Down
21 changes: 14 additions & 7 deletions add-on/src/lib/ipfs-request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,14 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) {
}
// Detect valid /ipfs/ and /ipns/ on any site
if (ipfsPathValidator.publicIpfsOrIpnsResource(request.url) && isSafeToRedirect(request, runtime)) {
return redirectToGateway(request.url, state)
return redirectToGateway(request.url, state, dnsLink)
}
// Look for dnslink in TXT records of visited sites
if (state.dnslink && dnsLink.isDnslookupSafeForURL(request.url) && isSafeToRedirect(request, runtime)) {
return dnsLink.dnslinkLookupAndOptionalRedirect(request.url)
if (state.dnslink && dnsLink.isDnslookupSafeForURL(request.url)) {
const dnslinkRedirect = dnsLink.dnslinkLookupAndOptionalRedirect(request.url)
if (dnslinkRedirect && isSafeToRedirect(request, runtime)) {
return dnslinkRedirect
}
}
}
},
Expand Down Expand Up @@ -99,7 +102,7 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) {
if (onHeadersReceivedRedirect.has(request.requestId)) {
const state = getState()
onHeadersReceivedRedirect.delete(request.requestId)
return redirectToGateway(request.url, state)
return redirectToGateway(request.url, state, dnsLink)
}
},

Expand Down Expand Up @@ -151,10 +154,14 @@ function postNormalizationSkip (state, request) {
return false
}

function redirectToGateway (requestUrl, state) {
function redirectToGateway (requestUrl, state, dnsLink) {
// TODO: redirect to `ipfs://` if hasNativeProtocolHandler === true
const gwUrl = state.ipfsNodeType === 'embedded' ? state.pubGwURL : state.gwURL
const url = new URL(requestUrl)
if (state.dnslink && dnsLink.canRedirectToIpns(url)) {
// late dnslink in onHeadersReceived
return dnsLink.redirectToIpnsPath(url)
}
const gwUrl = state.ipfsNodeType === 'embedded' ? state.pubGwURL : state.gwURL
url.protocol = gwUrl.protocol
url.host = gwUrl.host
url.port = gwUrl.port
Expand Down Expand Up @@ -210,7 +217,7 @@ function normalizedRedirectingProtocolRequest (request, pubGwUrl) {
path = path.replace(/^#dweb:\//i, '/') // dweb:/ipfs/Qm → /ipfs/Qm
path = path.replace(/^#ipfs:\/\//i, '/ipfs/') // ipfs://Qm → /ipfs/Qm
path = path.replace(/^#ipns:\/\//i, '/ipns/') // ipns://Qm → /ipns/Qm
console.log(`oldPath: '${oldPath}' new: '${path}'`)
// console.log(`oldPath: '${oldPath}' new: '${path}'`)
if (oldPath !== path && IsIpfs.path(path)) {
return { redirectUrl: urlAtPublicGw(path, pubGwUrl) }
}
Expand Down
53 changes: 53 additions & 0 deletions test/functional/lib/ipfs-request.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,59 @@ describe('modifyRequest.onBeforeRequest', function () {
expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined)
})
})

describe('request to FQDN with dnslink experiment enabled', function () {
let activeGateway
beforeEach(function () {
// pretend API is online and we can do dns lookups with it
state.dnslink = true
state.peerCount = 1
// embedded node (js-ipfs) defaults to public gw
activeGateway = (state.ipfsNodeType === 'external' ? state.gwURLString : state.pubGwURLString)
})
it('should be redirected to active gateway if dnslink exists', function () {
// stub the existence of valid dnslink
const fqdn = 'ipfs.git.sexy'
dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss')
//
const request = url2request('http://ipfs.git.sexy/index.html?argTest#hashTest')
expect(modifyRequest.onBeforeRequest(request).redirectUrl).to.equal(activeGateway + '/ipns/ipfs.git.sexy/index.html?argTest#hashTest')
})
it('should be redirected to active gateway if fetched from the same origin and redirect is enabled in non-Firefox', function () {
// stub the existence of valid dnslink
const fqdn = 'ipfs.git.sexy'
dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss')
//
runtime.isFirefox = false
const xhrRequest = {url: 'http://ipfs.git.sexy/index.html?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId()}
expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/ipfs.git.sexy/index.html?argTest#hashTest')
})
it('should be redirected to active gateway via late redirect if dnslink exists and XHR is cross-origin in Firefox', function () {
// stub the existence of valid dnslink
const fqdn = 'ipfs.git.sexy'
dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss')
//
// Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
runtime.isFirefox = true
const xhrRequest = {url: 'http://ipfs.git.sexy/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/ipfs.git.sexy/index.html?argTest#hashTest')
})
it('should be left unouched if dnslink does not exist and XHR is cross-origin in Firefox', function () {
// stub no dnslink
const fqdn = 'youtube.com'
dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns(undefined)
// Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436
runtime.isFirefox = true
const xhrRequest = {url: 'https://youtube.com/index.html?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId()}
// onBeforeRequest should not change anything
expect(modifyRequest.onBeforeRequest(xhrRequest)).to.equal(undefined)
// onHeadersReceived should not change anything
expect(modifyRequest.onHeadersReceived(xhrRequest)).to.equal(undefined)
})
})
})
})

Expand Down

0 comments on commit 99d9f12

Please sign in to comment.