From 4d0ffa629778dbdbc4e136d020a9eb9d7afb9c6f Mon Sep 17 00:00:00 2001 From: achingbrain Date: Thu, 29 Feb 2024 11:53:06 +0000 Subject: [PATCH 1/4] fix: append query path to path resolved from ipns name If an IPNS name resolves to a CID+path, and the requested URL also has a path, the resolved path should be the path from the IPNS record joined with the path from the requested URL, not the other way round. --- packages/verified-fetch/package.json | 1 + .../src/utils/parse-url-string.ts | 7 +- .../test/utils/parse-url-string.spec.ts | 185 ++++++++++++++++++ 3 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 packages/verified-fetch/test/utils/parse-url-string.spec.ts diff --git a/packages/verified-fetch/package.json b/packages/verified-fetch/package.json index 08b1affc..aa0ef5cf 100644 --- a/packages/verified-fetch/package.json +++ b/packages/verified-fetch/package.json @@ -90,6 +90,7 @@ "@helia/json": "^3.0.1", "@helia/utils": "^0.0.2", "@ipld/car": "^5.3.0", + "@libp2p/interface-compliance-tests": "^5.3.2", "@libp2p/logger": "^4.0.7", "@libp2p/peer-id-factory": "^4.0.7", "@sgtpooki/file-type": "^1.0.1", diff --git a/packages/verified-fetch/src/utils/parse-url-string.ts b/packages/verified-fetch/src/utils/parse-url-string.ts index d869f77c..ad76f8ba 100644 --- a/packages/verified-fetch/src/utils/parse-url-string.ts +++ b/packages/verified-fetch/src/utils/parse-url-string.ts @@ -136,13 +136,14 @@ export async function parseUrlString ({ urlString, ipns, logger }: ParseUrlStrin */ const pathParts = [] + if (resolvedPath != null && resolvedPath.length > 0) { + pathParts.push(resolvedPath) + } + if (urlPath.length > 0) { pathParts.push(urlPath) } - if (resolvedPath != null && resolvedPath.length > 0) { - pathParts.push(resolvedPath) - } const path = pathParts.join('/') return { diff --git a/packages/verified-fetch/test/utils/parse-url-string.spec.ts b/packages/verified-fetch/test/utils/parse-url-string.spec.ts new file mode 100644 index 00000000..0e8fbc50 --- /dev/null +++ b/packages/verified-fetch/test/utils/parse-url-string.spec.ts @@ -0,0 +1,185 @@ +import { matchPeerId } from '@libp2p/interface-compliance-tests/matchers' +import { defaultLogger } from '@libp2p/logger' +import { createEd25519PeerId } from '@libp2p/peer-id-factory' +import { expect } from 'aegir/chai' +import { CID } from 'multiformats/cid' +import { stubInterface } from 'sinon-ts' +import { parseUrlString } from '../../src/utils/parse-url-string.js' +import type { IPNS } from '@helia/ipns' +import type { ComponentLogger } from '@libp2p/interface' +import type { StubbedInstance } from 'sinon-ts' + +describe('parse-url-string', () => { + let logger: ComponentLogger + let ipns: StubbedInstance + + beforeEach(() => { + logger = defaultLogger() + ipns = stubInterface() + }) + + it('should parse an ipfs:// url', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + + await expect(parseUrlString({ + urlString: `ipfs://${cid}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipfs', + path: '', + cid, + query: {} + }) + }) + + it('should parse an ipfs:// url with a path', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const path = 'foo/bar/baz.txt' + + await expect(parseUrlString({ + urlString: `ipfs://${cid}/${path}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipfs', + path, + cid, + query: {} + }) + }) + + it('should parse an ipfs:// url with a path and a query', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const path = 'foo/bar/baz.txt' + + await expect(parseUrlString({ + urlString: `ipfs://${cid}/${path}?format=car&download=true&filename=qux.zip`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipfs', + path, + cid, + query: { + format: 'car', + download: true, + filename: 'qux.zip' + } + }) + }) + + it('should parse an ipns:// url', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const peerId = await createEd25519PeerId() + + ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ + cid, + path: '' + }) + + await expect(parseUrlString({ + urlString: `ipns://${peerId}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipns', + path: '', + cid, + query: {} + }) + }) + + it('should parse an ipns:// url with a path', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const peerId = await createEd25519PeerId() + const path = 'foo/bar/baz.txt' + + ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ + cid, + path: '' + }) + + await expect(parseUrlString({ + urlString: `ipns://${peerId}/${path}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipns', + path, + cid, + query: {} + }) + }) + + it('should parse an ipns:// url that resolves to a path', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const peerId = await createEd25519PeerId() + const path = 'foo/bar/baz.txt' + + ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ + cid, + path + }) + + await expect(parseUrlString({ + urlString: `ipns://${peerId}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipns', + path, + cid, + query: {} + }) + }) + + it('should parse an ipns:// url with a path that resolves to a path', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const peerId = await createEd25519PeerId() + const recordPath = 'foo' + const requestPath = 'bar/baz.txt' + + ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ + cid, + path: recordPath + }) + + await expect(parseUrlString({ + urlString: `ipns://${peerId}/${requestPath}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipns', + path: `${recordPath}/${requestPath}`, + cid, + query: {} + }) + }) + + it('should parse an ipns:// url with a path and a query', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const peerId = await createEd25519PeerId() + const path = 'foo/bar/baz.txt' + + ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ + cid, + path + }) + + await expect(parseUrlString({ + urlString: `ipns://${peerId}?format=car&download=true&filename=qux.zip`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipns', + path, + cid, + query: { + format: 'car', + download: true, + filename: 'qux.zip' + } + }) + }) +}) From cff1bd793cb3868012f8df8da16567aed6e3387e Mon Sep 17 00:00:00 2001 From: achingbrain Date: Thu, 29 Feb 2024 12:29:06 +0000 Subject: [PATCH 2/4] chore: move test to same dir as unit under test --- .../src/utils/parse-url-string.ts | 8 +- .../test/parse-url-string.spec.ts | 323 --------------- .../test/utils/parse-url-string.spec.ts | 392 ++++++++++++------ 3 files changed, 265 insertions(+), 458 deletions(-) delete mode 100644 packages/verified-fetch/test/parse-url-string.spec.ts diff --git a/packages/verified-fetch/src/utils/parse-url-string.ts b/packages/verified-fetch/src/utils/parse-url-string.ts index ad76f8ba..b19ba8ad 100644 --- a/packages/verified-fetch/src/utils/parse-url-string.ts +++ b/packages/verified-fetch/src/utils/parse-url-string.ts @@ -98,15 +98,19 @@ export async function parseUrlString ({ urlString, ipns, logger }: ParseUrlStrin resolvedPath = resolveResult?.path log.trace('resolved %s to %c', cidOrPeerIdOrDnsLink, cid) ipnsCache.set(cidOrPeerIdOrDnsLink, resolveResult, 60 * 1000 * 2) - } catch (err) { + } catch (err: any) { log.error('Could not resolve DnsLink for "%s"', cidOrPeerIdOrDnsLink, err) - errors.push(err as Error) + errors.push(err) } } } } if (cid == null) { + if (errors.length === 1) { + throw errors[0] + } + throw new AggregateError(errors, `Invalid resource. Cannot determine CID from URL "${urlString}"`) } diff --git a/packages/verified-fetch/test/parse-url-string.spec.ts b/packages/verified-fetch/test/parse-url-string.spec.ts deleted file mode 100644 index 6c56f112..00000000 --- a/packages/verified-fetch/test/parse-url-string.spec.ts +++ /dev/null @@ -1,323 +0,0 @@ -import { type PeerId } from '@libp2p/interface' -import { defaultLogger } from '@libp2p/logger' -import { createEd25519PeerId } from '@libp2p/peer-id-factory' -import { expect } from 'aegir/chai' -import { CID } from 'multiformats/cid' -import { stubInterface } from 'sinon-ts' -import { parseUrlString } from '../src/utils/parse-url-string.js' -import type { IPNS } from '@helia/ipns' - -describe('parseUrlString', () => { - describe('invalid URLs', () => { - it('throws for invalid URLs', async () => { - const ipns = stubInterface({}) - try { - await parseUrlString({ - urlString: 'invalid', - ipns, - logger: defaultLogger() - }) - throw new Error('Should have thrown') - } catch (err) { - expect((err as Error).message).to.equal('Invalid URL: invalid, please use ipfs:// or ipns:// URLs only.') - } - }) - - it('throws for invalid protocols', async () => { - const ipns = stubInterface({}) - try { - await parseUrlString({ - urlString: 'http://mydomain.com', - ipns, - logger: defaultLogger() - }) - throw new Error('Should have thrown') - } catch (err) { - expect((err as Error).message).to.equal('Invalid URL: http://mydomain.com, please use ipfs:// or ipns:// URLs only.') - } - }) - - it('throws an error if resulting CID is invalid', async () => { - const ipns = stubInterface({ - // @ts-expect-error - purposefully invalid response - resolveDns: async (_: string) => { - return null - } - }) - try { - await parseUrlString({ - urlString: 'ipns://mydomain.com', - ipns, - logger: defaultLogger() - }) - throw new Error('Should have thrown') - } catch (err) { - expect((err as Error).message).to.equal('Invalid resource. Cannot determine CID from URL "ipns://mydomain.com"') - } - }) - }) - - describe('ipfs:// URLs', () => { - it('handles invalid CIDs', async () => { - const ipns = stubInterface({}) - try { - await parseUrlString({ - urlString: 'ipfs://QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4i', - ipns, - logger: defaultLogger() - }) - throw new Error('Should have thrown') - } catch (aggErr) { - expect(aggErr).to.have.property('message', 'Invalid resource. Cannot determine CID from URL "ipfs://QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4i"') - expect(aggErr).to.have.property('errors').with.lengthOf(1).that.deep.equals([ - new TypeError('Invalid CID for ipfs:// URL') - ]) - } - }) - - it('can parse a URL with CID only', async () => { - const ipns = stubInterface({}) - const result = await parseUrlString({ - urlString: 'ipfs://QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr', - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipfs') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('') - }) - - it('can parse URL with CID+path', async () => { - const ipns = stubInterface({}) - const result = await parseUrlString({ - urlString: 'ipfs://QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm/1 - Barrel - Part 1/1 - Barrel - Part 1 - alt.txt', - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipfs') - expect(result.cid.toString()).to.equal('QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm') - expect(result.path).to.equal('1 - Barrel - Part 1/1 - Barrel - Part 1 - alt.txt') - }) - - it('can parse URL with CID+queryString', async () => { - const ipns = stubInterface({}) - const result = await parseUrlString({ - urlString: 'ipfs://QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm?format=car', - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipfs') - expect(result.cid.toString()).to.equal('QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm') - expect(result.path).to.equal('') - expect(result.query).to.deep.equal({ format: 'car' }) - }) - - it('can parse URL with CID+path+queryString', async () => { - const ipns = stubInterface({}) - const result = await parseUrlString({ - urlString: 'ipfs://QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm/1 - Barrel - Part 1/1 - Barrel - Part 1 - alt.txt?format=tar', - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipfs') - expect(result.cid.toString()).to.equal('QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm') - expect(result.path).to.equal('1 - Barrel - Part 1/1 - Barrel - Part 1 - alt.txt') - expect(result.query).to.deep.equal({ format: 'tar' }) - }) - }) - - describe('ipns:// URLs', () => { - let ipns: IPNS - - beforeEach(async () => { - ipns = stubInterface({ - resolveDns: async (dnsLink: string) => { - expect(dnsLink).to.equal('mydomain.com') - return { - cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), - path: '' - } - } - }) - }) - - it('handles invalid DNSLinkDomains', async () => { - ipns = stubInterface({ - resolve: async (peerId: PeerId) => { - throw new Error('Unexpected failure from ipns resolve method') - }, - resolveDns: async (_: string) => { - return Promise.reject(new Error('Unexpected failure from dns query')) - } - }) - - try { - await parseUrlString({ urlString: 'ipns://mydomain.com', ipns, logger: defaultLogger() }) - throw new Error('Should have thrown') - } catch (aggErr) { - expect(aggErr).to.have.property('message', 'Invalid resource. Cannot determine CID from URL "ipns://mydomain.com"') - expect(aggErr).to.have.property('errors').with.lengthOf(2).that.deep.equals([ - new TypeError('Could not parse PeerId in ipns url "mydomain.com", Non-base64 character'), - new Error('Unexpected failure from dns query') - ]) - } - }) - - it('can parse a URL with DNSLinkDomain only', async () => { - const result = await parseUrlString({ - urlString: 'ipns://mydomain.com', - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipns') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('') - }) - - it('can parse a URL with DNSLinkDomain+path', async () => { - const result = await parseUrlString({ - urlString: 'ipns://mydomain.com/some/path/to/file.txt', - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipns') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('some/path/to/file.txt') - }) - - it('can parse a URL with DNSLinkDomain+queryString', async () => { - const result = await parseUrlString({ - urlString: 'ipns://mydomain.com?format=json', - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipns') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('') - expect(result.query).to.deep.equal({ format: 'json' }) - }) - - it('can parse a URL with DNSLinkDomain+path+queryString', async () => { - const result = await parseUrlString({ - urlString: 'ipns://mydomain.com/some/path/to/file.txt?format=json', - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipns') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('some/path/to/file.txt') - expect(result.query).to.deep.equal({ format: 'json' }) - }) - }) - - describe('ipns:// URLs', () => { - let ipns: IPNS - let testPeerId: PeerId - - beforeEach(async () => { - testPeerId = await createEd25519PeerId() - ipns = stubInterface({ - resolve: async (peerId: PeerId) => { - expect(peerId.toString()).to.equal(testPeerId.toString()) - return { - cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), - path: '' - } - } - }) - }) - - it('handles invalid PeerIds', async () => { - ipns = stubInterface({ - resolve: async (peerId: PeerId) => { - throw new Error('Unexpected failure from ipns resolve method') - }, - resolveDns: async (_: string) => { - return Promise.reject(new Error('Unexpected failure from dns query')) - } - }) - - try { - await parseUrlString({ urlString: 'ipns://123PeerIdIsFake456', ipns, logger: defaultLogger() }) - throw new Error('Should have thrown') - } catch (aggErr) { - expect(aggErr).to.have.property('message', 'Invalid resource. Cannot determine CID from URL "ipns://123PeerIdIsFake456"') - expect(aggErr).to.have.property('errors').with.lengthOf(2).that.deep.equals([ - new TypeError('Could not parse PeerId in ipns url "123PeerIdIsFake456", Non-base58btc character'), - new Error('Unexpected failure from dns query') - ]) - } - }) - - it('handles valid PeerId resolve failures', async () => { - ipns = stubInterface({ - resolve: async (_: PeerId) => { - return Promise.reject(new Error('Unexpected failure from ipns resolve method')) - }, - resolveDns: async (_: string) => { - return Promise.reject(new Error('Unexpected failure from dns query')) - } - }) - - // await expect(parseUrlString({ urlString: `ipns://${testPeerId.toString()}`, ipns })).to.eventually.be.rejected() - // .with.property('message', `Could not resolve PeerId "${testPeerId.toString()}", Unexpected failure from ipns resolve method`) - - try { - await parseUrlString({ urlString: `ipns://${testPeerId.toString()}`, ipns, logger: defaultLogger() }) - throw new Error('Should have thrown') - } catch (aggErr) { - expect(aggErr).to.have.property('message', `Invalid resource. Cannot determine CID from URL "ipns://${testPeerId.toString()}"`) - expect(aggErr).to.have.property('errors').with.lengthOf(2).that.deep.equals([ - new TypeError(`Could not resolve PeerId "${testPeerId.toString()}", Unexpected failure from ipns resolve method`), - new Error('Unexpected failure from dns query') - ]) - } - }) - - it('can parse a URL with PeerId only', async () => { - const result = await parseUrlString({ - urlString: `ipns://${testPeerId.toString()}`, - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipns') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('') - }) - - it('can parse a URL with PeerId+path', async () => { - const result = await parseUrlString({ - urlString: `ipns://${testPeerId.toString()}/some/path/to/file.txt`, - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipns') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('some/path/to/file.txt') - }) - - it('can parse a URL with PeerId+queryString', async () => { - const result = await parseUrlString({ - urlString: `ipns://${testPeerId.toString()}?fomat=dag-cbor`, - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipns') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('') - expect(result.query).to.deep.equal({ fomat: 'dag-cbor' }) - }) - - it('can parse a URL with PeerId+path+queryString', async () => { - const result = await parseUrlString({ - urlString: `ipns://${testPeerId.toString()}/some/path/to/file.txt?fomat=dag-cbor`, - ipns, - logger: defaultLogger() - }) - expect(result.protocol).to.equal('ipns') - expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - expect(result.path).to.equal('some/path/to/file.txt') - expect(result.query).to.deep.equal({ fomat: 'dag-cbor' }) - }) - }) -}) diff --git a/packages/verified-fetch/test/utils/parse-url-string.spec.ts b/packages/verified-fetch/test/utils/parse-url-string.spec.ts index 0e8fbc50..7b4fe735 100644 --- a/packages/verified-fetch/test/utils/parse-url-string.spec.ts +++ b/packages/verified-fetch/test/utils/parse-url-string.spec.ts @@ -6,10 +6,10 @@ import { CID } from 'multiformats/cid' import { stubInterface } from 'sinon-ts' import { parseUrlString } from '../../src/utils/parse-url-string.js' import type { IPNS } from '@helia/ipns' -import type { ComponentLogger } from '@libp2p/interface' +import type { ComponentLogger, PeerId } from '@libp2p/interface' import type { StubbedInstance } from 'sinon-ts' -describe('parse-url-string', () => { +describe('parseUrlString', () => { let logger: ComponentLogger let ipns: StubbedInstance @@ -18,168 +18,294 @@ describe('parse-url-string', () => { ipns = stubInterface() }) - it('should parse an ipfs:// url', async () => { - const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + describe('invalid URLs', () => { + it('throws for invalid URLs', async () => { + await expect( + parseUrlString({ + urlString: 'invalid', + ipns, + logger + }) + ).to.eventually.be.rejected + .with.property('message', 'Invalid URL: invalid, please use ipfs:// or ipns:// URLs only.') + }) - await expect(parseUrlString({ - urlString: `ipfs://${cid}`, - ipns, - logger - })).to.eventually.deep.equal({ - protocol: 'ipfs', - path: '', - cid, - query: {} + it('throws for invalid protocols', async () => { + await expect( + parseUrlString({ + urlString: 'invalid', + ipns, + logger + }) + ).to.eventually.be.rejected + .with.property('message', 'Invalid URL: invalid, please use ipfs:// or ipns:// URLs only.') }) - }) - it('should parse an ipfs:// url with a path', async () => { - const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - const path = 'foo/bar/baz.txt' + it('throws an error if resulting CID is invalid', async () => { + // @ts-expect-error - purposefully invalid response + ipns.resolveDns.returns(null) - await expect(parseUrlString({ - urlString: `ipfs://${cid}/${path}`, - ipns, - logger - })).to.eventually.deep.equal({ - protocol: 'ipfs', - path, - cid, - query: {} + await expect( + parseUrlString({ + urlString: 'ipns://mydomain.com', + ipns, + logger + }) + ).to.eventually.be.rejected + .with.property('message', 'Could not parse PeerId in ipns url "mydomain.com", Non-base64 character') }) }) - it('should parse an ipfs:// url with a path and a query', async () => { - const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - const path = 'foo/bar/baz.txt' - - await expect(parseUrlString({ - urlString: `ipfs://${cid}/${path}?format=car&download=true&filename=qux.zip`, - ipns, - logger - })).to.eventually.deep.equal({ - protocol: 'ipfs', - path, - cid, - query: { - format: 'car', - download: true, - filename: 'qux.zip' - } + describe('ipfs:// URLs', () => { + it('handles invalid CIDs', async () => { + await expect( + parseUrlString({ + urlString: 'ipfs://QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4i', + ipns, + logger + }) + ).to.eventually.be.rejected + .with.property('message', 'Invalid CID for ipfs:// URL') }) - }) - it('should parse an ipns:// url', async () => { - const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - const peerId = await createEd25519PeerId() + it('can parse a URL with CID only', async () => { + const result = await parseUrlString({ + urlString: 'ipfs://QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr', + ipns, + logger + }) + expect(result.protocol).to.equal('ipfs') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('') + }) - ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ - cid, - path: '' + it('can parse URL with CID+path', async () => { + const result = await parseUrlString({ + urlString: 'ipfs://QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm/1 - Barrel - Part 1/1 - Barrel - Part 1 - alt.txt', + ipns, + logger + }) + expect(result.protocol).to.equal('ipfs') + expect(result.cid.toString()).to.equal('QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm') + expect(result.path).to.equal('1 - Barrel - Part 1/1 - Barrel - Part 1 - alt.txt') }) - await expect(parseUrlString({ - urlString: `ipns://${peerId}`, - ipns, - logger - })).to.eventually.deep.equal({ - protocol: 'ipns', - path: '', - cid, - query: {} + it('can parse URL with CID+queryString', async () => { + const result = await parseUrlString({ + urlString: 'ipfs://QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm?format=car', + ipns, + logger + }) + expect(result.protocol).to.equal('ipfs') + expect(result.cid.toString()).to.equal('QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm') + expect(result.path).to.equal('') + expect(result.query).to.deep.equal({ format: 'car' }) + }) + + it('can parse URL with CID+path+queryString', async () => { + const result = await parseUrlString({ + urlString: 'ipfs://QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm/1 - Barrel - Part 1/1 - Barrel - Part 1 - alt.txt?format=tar', + ipns, + logger + }) + expect(result.protocol).to.equal('ipfs') + expect(result.cid.toString()).to.equal('QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm') + expect(result.path).to.equal('1 - Barrel - Part 1/1 - Barrel - Part 1 - alt.txt') + expect(result.query).to.deep.equal({ format: 'tar' }) }) }) - it('should parse an ipns:// url with a path', async () => { - const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - const peerId = await createEd25519PeerId() - const path = 'foo/bar/baz.txt' + describe('ipns:// URLs', () => { + it('handles invalid DNSLinkDomains', async () => { + ipns.resolve.rejects(new Error('Unexpected failure from ipns resolve method')) + ipns.resolveDns.rejects(new Error('Unexpected failure from ipns dns query')) + + await expect(parseUrlString({ urlString: 'ipns://mydomain.com', ipns, logger })).to.eventually.be.rejected + .with.property('errors').that.deep.equals([ + new TypeError('Could not parse PeerId in ipns url "mydomain.com", Non-base64 character'), + new Error('Unexpected failure from ipns dns query') + ]) + }) + + it('can parse a URL with DNSLinkDomain only', async () => { + ipns.resolveDns.withArgs('mydomain.com').resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) - ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ - cid, - path: '' + const result = await parseUrlString({ + urlString: 'ipns://mydomain.com', + ipns, + logger + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('') }) - await expect(parseUrlString({ - urlString: `ipns://${peerId}/${path}`, - ipns, - logger - })).to.eventually.deep.equal({ - protocol: 'ipns', - path, - cid, - query: {} + it('can parse a URL with DNSLinkDomain+path', async () => { + ipns.resolveDns.withArgs('mydomain.com').resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) + + const result = await parseUrlString({ + urlString: 'ipns://mydomain.com/some/path/to/file.txt', + ipns, + logger + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('some/path/to/file.txt') }) - }) - it('should parse an ipns:// url that resolves to a path', async () => { - const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - const peerId = await createEd25519PeerId() - const path = 'foo/bar/baz.txt' + it('can parse a URL with DNSLinkDomain+queryString', async () => { + ipns.resolveDns.withArgs('mydomain.com').resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) - ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ - cid, - path + const result = await parseUrlString({ + urlString: 'ipns://mydomain.com?format=json', + ipns, + logger + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('') + expect(result.query).to.deep.equal({ format: 'json' }) }) - await expect(parseUrlString({ - urlString: `ipns://${peerId}`, - ipns, - logger - })).to.eventually.deep.equal({ - protocol: 'ipns', - path, - cid, - query: {} + it('can parse a URL with DNSLinkDomain+path+queryString', async () => { + ipns.resolveDns.withArgs('mydomain.com').resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) + + const result = await parseUrlString({ + urlString: 'ipns://mydomain.com/some/path/to/file.txt?format=json', + ipns, + logger: defaultLogger() + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('some/path/to/file.txt') + expect(result.query).to.deep.equal({ format: 'json' }) }) }) - it('should parse an ipns:// url with a path that resolves to a path', async () => { - const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - const peerId = await createEd25519PeerId() - const recordPath = 'foo' - const requestPath = 'bar/baz.txt' + describe('ipns:// URLs', () => { + let testPeerId: PeerId - ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ - cid, - path: recordPath + beforeEach(async () => { + testPeerId = await createEd25519PeerId() }) - await expect(parseUrlString({ - urlString: `ipns://${peerId}/${requestPath}`, - ipns, - logger - })).to.eventually.deep.equal({ - protocol: 'ipns', - path: `${recordPath}/${requestPath}`, - cid, - query: {} + it('handles invalid PeerIds', async () => { + ipns.resolve.rejects(new Error('Unexpected failure from ipns resolve method')) + ipns.resolveDns.rejects(new Error('Unexpected failure from ipns dns query')) + + await expect(parseUrlString({ urlString: 'ipns://123PeerIdIsFake456', ipns, logger })).to.eventually.be.rejected + .with.property('errors').that.deep.equals([ + new TypeError('Could not parse PeerId in ipns url "123PeerIdIsFake456", Non-base58btc character'), + new Error('Unexpected failure from ipns dns query') + ]) }) - }) - it('should parse an ipns:// url with a path and a query', async () => { - const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') - const peerId = await createEd25519PeerId() - const path = 'foo/bar/baz.txt' - - ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ - cid, - path - }) - - await expect(parseUrlString({ - urlString: `ipns://${peerId}?format=car&download=true&filename=qux.zip`, - ipns, - logger - })).to.eventually.deep.equal({ - protocol: 'ipns', - path, - cid, - query: { - format: 'car', - download: true, - filename: 'qux.zip' - } + it('handles valid PeerId resolve failures', async () => { + ipns.resolve.rejects(new Error('Unexpected failure from ipns resolve method')) + ipns.resolveDns.rejects(new Error('Unexpected failure from ipns dns query')) + + await expect(parseUrlString({ urlString: `ipns://${testPeerId}`, ipns, logger })).to.eventually.be.rejected + .with.property('errors').that.deep.equals([ + new TypeError(`Could not resolve PeerId "${testPeerId}", Unexpected failure from ipns resolve method`), + new Error('Unexpected failure from ipns dns query') + ]) + }) + + it('can parse a URL with PeerId only', async () => { + ipns.resolve.withArgs(matchPeerId(testPeerId)).resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) + const result = await parseUrlString({ + urlString: `ipns://${testPeerId}`, + ipns, + logger + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('') + }) + + it('can parse a URL with PeerId+path', async () => { + ipns.resolve.withArgs(matchPeerId(testPeerId)).resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) + const result = await parseUrlString({ + urlString: `ipns://${testPeerId}/some/path/to/file.txt`, + ipns, + logger + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('some/path/to/file.txt') + }) + + it('can parse a URL with PeerId+queryString', async () => { + ipns.resolve.withArgs(matchPeerId(testPeerId)).resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) + const result = await parseUrlString({ + urlString: `ipns://${testPeerId}?fomat=dag-cbor`, + ipns, + logger + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('') + expect(result.query).to.deep.equal({ fomat: 'dag-cbor' }) + }) + + it('can parse a URL with PeerId+path+queryString', async () => { + ipns.resolve.withArgs(matchPeerId(testPeerId)).resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) + const result = await parseUrlString({ + urlString: `ipns://${testPeerId}/some/path/to/file.txt?fomat=dag-cbor`, + ipns, + logger + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('some/path/to/file.txt') + expect(result.query).to.deep.equal({ fomat: 'dag-cbor' }) + }) + + it('should parse an ipns:// url with a path that resolves to a path', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const peerId = await createEd25519PeerId() + const recordPath = 'foo' + const requestPath = 'bar/baz.txt' + + ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ + cid, + path: recordPath + }) + + await expect(parseUrlString({ + urlString: `ipns://${peerId}/${requestPath}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipns', + path: `${recordPath}/${requestPath}`, + cid, + query: {} + }) }) }) }) From 96663fa85bac6e96e2759942e4db45ccafc71a54 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Thu, 29 Feb 2024 12:31:15 +0000 Subject: [PATCH 3/4] chore: update name --- packages/verified-fetch/test/utils/parse-url-string.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/verified-fetch/test/utils/parse-url-string.spec.ts b/packages/verified-fetch/test/utils/parse-url-string.spec.ts index 7b4fe735..ce86adad 100644 --- a/packages/verified-fetch/test/utils/parse-url-string.spec.ts +++ b/packages/verified-fetch/test/utils/parse-url-string.spec.ts @@ -285,7 +285,7 @@ describe('parseUrlString', () => { expect(result.query).to.deep.equal({ fomat: 'dag-cbor' }) }) - it('should parse an ipns:// url with a path that resolves to a path', async () => { + it('should parse an ipns:// url with a path that resolves to a CID with a path', async () => { const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') const peerId = await createEd25519PeerId() const recordPath = 'foo' From b02f16bde1c59916f5c1b2341ff4481da578aaed Mon Sep 17 00:00:00 2001 From: achingbrain Date: Thu, 29 Feb 2024 13:15:32 +0000 Subject: [PATCH 4/4] chore: add tests for slashes --- .../src/utils/parse-url-string.ts | 42 ++++++++----- .../test/utils/parse-url-string.spec.ts | 61 +++++++++++++++++++ 2 files changed, 87 insertions(+), 16 deletions(-) diff --git a/packages/verified-fetch/src/utils/parse-url-string.ts b/packages/verified-fetch/src/utils/parse-url-string.ts index b19ba8ad..1d20f218 100644 --- a/packages/verified-fetch/src/utils/parse-url-string.ts +++ b/packages/verified-fetch/src/utils/parse-url-string.ts @@ -133,27 +133,37 @@ export async function parseUrlString ({ urlString, ipns, logger }: ParseUrlStrin } } - /** - * join the path from resolve result & given path. - * e.g. /ipns// that is resolved to /ipfs//, when requested as /ipns//, should be - * resolved to /ipfs/// - */ - const pathParts = [] - - if (resolvedPath != null && resolvedPath.length > 0) { - pathParts.push(resolvedPath) + return { + protocol, + cid, + path: joinPaths(resolvedPath, urlPath), + query + } +} + +/** + * join the path from resolve result & given path. + * e.g. /ipns// that is resolved to /ipfs//, when requested as /ipns//, should be + * resolved to /ipfs/// + */ +function joinPaths (resolvedPath: string | undefined, urlPath: string): string { + let path = '' + + if (resolvedPath != null) { + path += resolvedPath } if (urlPath.length > 0) { - pathParts.push(urlPath) + path = `${path.length > 0 ? `${path}/` : path}${urlPath}` } - const path = pathParts.join('/') + // replace duplicate forward slashes + path = path.replace(/\/(\/)+/g, '/') - return { - protocol, - cid, - path, - query + // strip trailing forward slash if present + if (path.startsWith('/')) { + path = path.substring(1) } + + return path } diff --git a/packages/verified-fetch/test/utils/parse-url-string.spec.ts b/packages/verified-fetch/test/utils/parse-url-string.spec.ts index ce86adad..a77ff430 100644 --- a/packages/verified-fetch/test/utils/parse-url-string.spec.ts +++ b/packages/verified-fetch/test/utils/parse-url-string.spec.ts @@ -253,6 +253,21 @@ describe('parseUrlString', () => { expect(result.path).to.equal('some/path/to/file.txt') }) + it('can parse a URL with PeerId+path with a trailing slash', async () => { + ipns.resolve.withArgs(matchPeerId(testPeerId)).resolves({ + cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), + path: '' + }) + const result = await parseUrlString({ + urlString: `ipns://${testPeerId}/some/path/to/dir/`, + ipns, + logger + }) + expect(result.protocol).to.equal('ipns') + expect(result.cid.toString()).to.equal('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + expect(result.path).to.equal('some/path/to/dir/') + }) + it('can parse a URL with PeerId+queryString', async () => { ipns.resolve.withArgs(matchPeerId(testPeerId)).resolves({ cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'), @@ -307,5 +322,51 @@ describe('parseUrlString', () => { query: {} }) }) + + it('should parse an ipns:// url with a path that resolves to a CID with a path with a trailing slash', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const peerId = await createEd25519PeerId() + const recordPath = 'foo/' + const requestPath = 'bar/baz.txt' + + ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ + cid, + path: recordPath + }) + + await expect(parseUrlString({ + urlString: `ipns://${peerId}/${requestPath}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipns', + path: 'foo/bar/baz.txt', + cid, + query: {} + }) + }) + + it('should parse an ipns:// url with a path that resolves to a CID with a path with a trailing slash', async () => { + const cid = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr') + const peerId = await createEd25519PeerId() + const recordPath = '/foo/////bar//' + const requestPath = '///baz///qux.txt' + + ipns.resolve.withArgs(matchPeerId(peerId)).resolves({ + cid, + path: recordPath + }) + + await expect(parseUrlString({ + urlString: `ipns://${peerId}/${requestPath}`, + ipns, + logger + })).to.eventually.deep.equal({ + protocol: 'ipns', + path: 'foo/bar/baz/qux.txt', + cid, + query: {} + }) + }) }) })