Merge pull request #518 from ipfs-shipyard/fix/ipfs-paths-on-dnslink-…

…host

Exclude gateway paths from dnslink redirects

This PR: 

- ensures dnslink redirect to `/ipns/{fqdn}/{path}` does not happen for `/ipfs/`, `/ipns/` and `/api/` paths
- adds proper dnslink tests

### Rationale 

Public gateways are often exposed under the same domain name  (https://ipfs.io) and even
tho right now CID-based redirect takes precedence things could break if we ever do some refactoring and move things around. 

Rare edge cases which are fixed by this PR:
- https://ipfs.io/api/v0/dns/ipfs.io  broken due to dnslink triggering redirect to `/ipns/`.
- https://ipfs.io/ipfs/invalid_cid in which no valid CID is detected, so path is marked as non-ipfs and dnslink redirect skews original error.

add-on/src/lib/dns-link.js

@@ -25,9 +25,7 @@ module.exports = function createDnsLink (getState) {
 
     dnslinkLookupAndOptionalRedirect (requestUrl) {
       const url = new URL(requestUrl)
-      const fqdn = url.hostname
-      const dnslink = dnsLink.cachedDnslinkLookup(fqdn)
-      if (dnslink) {
+      if (dnsLink.canRedirectToIpns(url)) {
         // redirect to IPNS and leave it up to the gateway
         // to load the correct path from IPFS
         // - https://github.com/ipfs/ipfs-companion/issues/298
@@ -86,7 +84,16 @@ module.exports = function createDnsLink (getState) {
     },
 
     canRedirectToIpns (url) {
-      if (!url.pathname.startsWith('/ipfs/') && !url.pathname.startsWith('/ipns/')) {
+      // Safety check: detect and skip gateway paths
+      // Public gateways such as ipfs.io are often exposed under the same domain name.
+      // We don't want dnslink to interfere with content-addressing redirects,
+      // or things like /api/v0 paths exposed by the writable gateway
+      // so we ignore known namespaces exposed by HTTP2IPFS gateways
+      // and ignore them even if things like CID are invalid
+      // -- we don't want to skew errors from gateway
+      const path = url.pathname
+      const httpGatewayPath = path.startsWith('/ipfs/') || path.startsWith('/ipns/') || path.startsWith('/api/v')
+      if (!httpGatewayPath) {
         const fqdn = url.hostname
         const dnslink = dnsLink.cachedDnslinkLookup(fqdn)
         if (dnslink) {

test/functional/lib/dns-link.test.js

@@ -2,39 +2,109 @@
 const { describe, it, before, after } = require('mocha')
 const { expect } = require('chai')
 const { URL } = require('url')
+const sinon = require('sinon')
 const createDnsLink = require('../../../add-on/src/lib/dns-link')
 
+function setFakeDnslink (fqdn, dnsLink) {
+  // stub the existence of valid dnslink
+  dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR')
+}
+
 // https://github.com/ipfs/ipfs-companion/issues/303
-describe('DNSLINK', function () {
+    // dnslinkLookupAndOptionalRedirect (requestUrl) {
+describe('dnsLink', function () {
   before(() => {
     global.URL = URL
   })
 
-  describe('redirectToIpnsPath(url) with external gateway', function () {
-    it('should return IPNS path at a custom gateway', async function () {
-      const url = new URL('http://ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
-      const getState = () => ({
-        gwURL: new URL('http://127.0.0.1:8080'),
-        pubGwURL: new URL('https://ipfs.io'),
-        ipfsNodeType: 'external'
-      })
+  const getState = () => ({
+    gwURL: new URL('http://127.0.0.1:8080'),
+    pubGwURL: new URL('https://gateway.foobar.io'),
+    ipfsNodeType: 'external',
+    peerCount: 1
+  })
+  const getExternalNodeState = () => Object.assign({}, getState(), {ipfsNodeType: 'external'})
+  const getEmbeddedNodeState = () => Object.assign({}, getState(), {ipfsNodeType: 'embedded'})
+
+  describe('dnslinkLookupAndOptionalRedirect(url)', function () {
+    it('should return nothing if dnslink is present but path starts with /api/v0/', function () {
+      const url = new URL('https://dnslinksite1.io/api/v0/dns/ipfs.io')
+      const dnsLink = createDnsLink(getState)
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.dnslinkLookupAndOptionalRedirect(url.toString())).to.equal(undefined)
+    })
+    it('should return nothing if dnslink is present but path starts with /ipfs/', function () {
+      const url = new URL('https://dnslinksite2.io/ipfs/foo/bar')
       const dnsLink = createDnsLink(getState)
-      expect(dnsLink.redirectToIpnsPath(url).redirectUrl)
-        .to.equal('http://127.0.0.1:8080/ipns/ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.dnslinkLookupAndOptionalRedirect(url.toString())).to.equal(undefined)
+    })
+    it('should return nothing if dnslink is present but path starts with /ipfs/', function () {
+      const url = new URL('https://dnslinksite3.io/ipns/foo/bar')
+      const dnsLink = createDnsLink(getState)
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.dnslinkLookupAndOptionalRedirect(url.toString())).to.equal(undefined)
+    })
+    it('with external node should return redirect to custom gateway if dnslink is present and path does not belong to a gateway', function () {
+      const url = new URL('https://dnslinksite4.io/foo/barl?a=b#c=d')
+      const dnsLink = createDnsLink(getExternalNodeState)
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.dnslinkLookupAndOptionalRedirect(url.toString()).redirectUrl)
+          .to.equal('http://127.0.0.1:8080/ipns/dnslinksite4.io/foo/barl?a=b#c=d')
+    })
+    it('with embedded node should return redirect to public gateway if dnslink is present and path does not belong to a gateway', function () {
+      const url = new URL('https://dnslinksite4.io/foo/barl?a=b#c=d')
+      const dnsLink = createDnsLink(getEmbeddedNodeState)
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.dnslinkLookupAndOptionalRedirect(url.toString()).redirectUrl)
+          .to.equal('https://gateway.foobar.io/ipns/dnslinksite4.io/foo/barl?a=b#c=d')
     })
   })
 
-  describe('redirectToIpnsPath(url) with embedded gateway', function () {
-    it('should return IPNS path at a public gateway', async function () {
-      const url = new URL('http://ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
-      const getState = () => ({
-        gwURL: new URL('http://127.0.0.1:8080'),
-        pubGwURL: new URL('https://ipfs.io'),
-        ipfsNodeType: 'embedded'
+  describe('redirectToIpnsPath(url)', function () {
+    describe('with external gateway', function () {
+      it('should return IPNS path at a custom gateway', function () {
+        const url = new URL('http://ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
+        const dnsLink = createDnsLink(getExternalNodeState)
+        expect(dnsLink.redirectToIpnsPath(url).redirectUrl)
+          .to.equal('http://127.0.0.1:8080/ipns/ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
+      })
+    })
+
+    describe('with embedded gateway', function () {
+      it('should return IPNS path at a public gateway', function () {
+        const url = new URL('http://ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
+        const dnsLink = createDnsLink(getEmbeddedNodeState)
+        expect(dnsLink.redirectToIpnsPath(url).redirectUrl)
+          .to.equal('https://gateway.foobar.io/ipns/ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
       })
+    })
+  })
+
+  describe('canRedirectToIpns(url)', function () {
+    it('should return false if dnslink is present but path starts with /api/v0/', function () {
+      const url = new URL('https://dnslinksite1.io/api/v0/dns/ipfs.io')
+      const dnsLink = createDnsLink(getState)
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.canRedirectToIpns(url)).to.equal(false)
+    })
+    it('should return false if dnslink is present but path starts with /ipfs/', function () {
+      const url = new URL('https://dnslinksite2.io/ipfs/foo/bar')
+      const dnsLink = createDnsLink(getState)
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.canRedirectToIpns(url)).to.equal(false)
+    })
+    it('should return false if dnslink is present but path starts with /ipfs/', function () {
+      const url = new URL('https://dnslinksite3.io/ipns/foo/bar')
+      const dnsLink = createDnsLink(getState)
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.canRedirectToIpns(url)).to.equal(false)
+    })
+    it('should return true if dnslink is present and path does not belong to a gateway', function () {
+      const url = new URL('https://dnslinksite4.io/foo/bar')
       const dnsLink = createDnsLink(getState)
-      expect(dnsLink.redirectToIpnsPath(url).redirectUrl)
-        .to.equal('https://ipfs.io/ipns/ipfs.git.sexy/sketches/ipld_intro.html?a=b#c=d')
+      setFakeDnslink(url.hostname, dnsLink)
+      expect(dnsLink.canRedirectToIpns(url)).to.equal(true)
     })
   })