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: X-Ipfs-Path is set correctly #46

Merged
merged 2 commits into from
Apr 20, 2024
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
1 change: 1 addition & 0 deletions packages/verified-fetch/src/utils/parse-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export async function parseResource (resource: Resource, { ipns, logger }: Parse
protocol: 'ipfs',
path: '',
query: {},
ipfsPath: `/ipfs/${cid.toString()}`,
ttl: 29030400 // 1 year for ipfs content
} satisfies ParsedUrlStringResults
}
Expand Down
13 changes: 12 additions & 1 deletion packages/verified-fetch/src/utils/parse-url-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ interface ParsedUrlStringResultsBase extends ResolveResult {
protocol: 'ipfs' | 'ipns'
query: ParsedUrlQuery

/**
* The value for the IPFS gateway spec compliant header `X-Ipfs-Path` on the
* response.
* The value of this header should be the original requested content path,
* prior to any path resolution or traversal.
*
* @see https://specs.ipfs.tech/http-gateways/path-gateway/#x-ipfs-path-response-header
*/
ipfsPath: string

/**
* seconds as a number
*/
Expand Down Expand Up @@ -248,7 +258,8 @@ export async function parseUrlString ({ urlString, ipns, logger }: ParseUrlStrin
cid,
path: joinPaths(resolvedPath, urlPath ?? ''),
query,
ttl
ttl,
ipfsPath: `/${protocol}/${cidOrPeerIdOrDnsLink}${urlPath != null && urlPath !== '' ? `/${urlPath}` : ''}`
} satisfies ParsedUrlStringResults
}

Expand Down
5 changes: 3 additions & 2 deletions packages/verified-fetch/src/verified-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,13 +490,15 @@ export class VerifiedFetch {
let query: ParsedUrlStringResults['query']
let ttl: ParsedUrlStringResults['ttl']
let protocol: ParsedUrlStringResults['protocol']
let ipfsPath: string
try {
const result = await parseResource(resource, { ipns: this.ipns, logger: this.helia.logger }, options)
cid = result.cid
path = result.path
query = result.query
ttl = result.ttl
protocol = result.protocol
ipfsPath = result.ipfsPath
} catch (err: any) {
options?.signal?.throwIfAborted()
this.log.error('error parsing resource %s', resource, err)
Expand Down Expand Up @@ -558,8 +560,7 @@ export class VerifiedFetch {
response.headers.set('etag', getETag({ cid, reqFormat, weak: false }))

setCacheControlHeader({ response, ttl, protocol })
// https://specs.ipfs.tech/http-gateways/path-gateway/#x-ipfs-path-response-header
response.headers.set('X-Ipfs-Path', resource.toString())
response.headers.set('X-Ipfs-Path', ipfsPath)

// set Content-Disposition header
let contentDisposition: string | undefined
Expand Down
48 changes: 45 additions & 3 deletions packages/verified-fetch/test/parse-resource.spec.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import { defaultLogger } from '@libp2p/logger'
import { createEd25519PeerId } from '@libp2p/peer-id-factory'
import { expect } from 'aegir/chai'
import { CID } from 'multiformats/cid'
import sinon from 'sinon'
import { stubInterface } from 'sinon-ts'
import { stubInterface, type StubbedInstance } from 'sinon-ts'
import { parseResource } from '../src/utils/parse-resource.js'
import type { IPNS } from '@helia/ipns'

const testCID = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr')
const peerId = await createEd25519PeerId()

describe('parseResource', () => {
it('does not call @helia/ipns for CID', async () => {
const testCID = CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr')
const shouldNotBeCalled1 = sinon.stub().throws(new Error('should not be called'))
const shouldNotBeCalled2 = sinon.stub().throws(new Error('should not be called'))
const { cid, path, query } = await parseResource(testCID, {
const { cid, path, query, ipfsPath } = await parseResource(testCID, {
ipns: stubInterface<IPNS>({
resolveDNSLink: shouldNotBeCalled1,
resolve: shouldNotBeCalled2
Expand All @@ -23,10 +26,49 @@ describe('parseResource', () => {
expect(cid.toString()).to.equal(testCID.toString())
expect(path).to.equal('')
expect(query).to.deep.equal({})
expect(ipfsPath).to.equal(`/ipfs/${testCID.toString()}`)
})

it('throws an error if given an invalid resource', async () => {
// @ts-expect-error - purposefully invalid input
await expect(parseResource({}, stubInterface<IPNS>())).to.be.rejectedWith('Invalid resource.')
})

describe('ipfsPath', () => {
let ipnsStub: StubbedInstance<IPNS>

beforeEach(async () => {
ipnsStub = stubInterface<IPNS>({
resolveDNSLink: sinon.stub().returns({ cid: testCID }),
resolve: sinon.stub().returns({ cid: testCID })
})
});

[
// resource without paths
{ resource: testCID, expectedValue: `/ipfs/${testCID}` },
{ resource: `ipfs://${testCID}`, expectedValue: `/ipfs/${testCID}` },
{ resource: `http://example.com/ipfs/${testCID}`, expectedValue: `/ipfs/${testCID}` },
{ resource: `ipns://${peerId}`, expectedValue: `/ipns/${peerId}` },
{ resource: `http://example.com/ipns/${peerId}`, expectedValue: `/ipns/${peerId}` },
{ resource: 'ipns://specs.ipfs.tech', expectedValue: '/ipns/specs.ipfs.tech' },
{ resource: 'http://example.com/ipns/specs.ipfs.tech', expectedValue: '/ipns/specs.ipfs.tech' },
// resources with paths
{ resource: `ipfs://${testCID}/foobar`, expectedValue: `/ipfs/${testCID}/foobar` },
{ resource: `http://example.com/ipfs/${testCID}/foobar`, expectedValue: `/ipfs/${testCID}/foobar` },
{ resource: `ipns://${peerId}/foobar`, expectedValue: `/ipns/${peerId}/foobar` },
{ resource: `http://example.com/ipns/${peerId}/foobar`, expectedValue: `/ipns/${peerId}/foobar` },
{ resource: 'ipns://specs.ipfs.tech/foobar', expectedValue: '/ipns/specs.ipfs.tech/foobar' },
{ resource: 'http://example.com/ipns/specs.ipfs.tech/foobar', expectedValue: '/ipns/specs.ipfs.tech/foobar' }
].forEach(({ resource, expectedValue }) => {
it(`should return the correct ipfsPath for "${resource}"`, async () => {
const { ipfsPath } = await parseResource(resource, {
ipns: ipnsStub,
logger: defaultLogger()
})

expect(ipfsPath).to.equal(expectedValue)
})
})
})
})
1 change: 1 addition & 0 deletions packages/verified-fetch/test/verified-fetch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ describe('@helia/verifed-fetch', () => {
expect(ipfsResponse).to.be.ok()
expect(ipfsResponse.status).to.equal(301)
expect(ipfsResponse.headers.get('location')).to.equal(`ipfs://${res.cid}/foo/`)
expect(ipfsResponse.headers.get('X-Ipfs-Path')).to.equal(`/ipfs/${res.cid}/foo`)
expect(ipfsResponse.url).to.equal(`ipfs://${res.cid}/foo`)
})

Expand Down
Loading