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

bug: @helia/ipns should allow dnslink paths. #402

Closed
SgtPooki opened this issue Jan 24, 2024 · 1 comment · Fixed by #410
Closed

bug: @helia/ipns should allow dnslink paths. #402

SgtPooki opened this issue Jan 24, 2024 · 1 comment · Fixed by #410
Assignees

Comments

@SgtPooki
Copy link
Member

async resolve (key: PeerId, options: ResolveOptions = {}): Promise<CID> {
const routingKey = peerIdToRoutingKey(key)
const record = await this.#findIpnsRecord(routingKey, options)
return this.#resolve(record.value, options)
}
async resolveDns (domain: string, options: ResolveDNSOptions = {}): Promise<CID> {
const resolvers = options.resolvers ?? this.defaultResolvers
const dnslink = await Promise.any(
resolvers.map(async resolver => resolver(domain, options))
)
return this.#resolve(dnslink, options)
}

some dnslinks may be /ipfs/Qm.../somePath/to/whatever and currently we do not resolve those properly

if (parts.length === 3) {

parts are not guaranteed to be length===3

cc @aschmahmann

@SgtPooki
Copy link
Member Author

SgtPooki commented Jan 30, 2024

This bug is impacting even dnsLink values with trailing path. From #392 (comment):

/ipns/app.aave.com is failing ipns resolution even though we are getting an answer back:

I created a demo for testing IPNS resolution at https://codepen.io/SgtPooki/pen/ZEPrJYp?editors=1011

Log:
dnslink:query: "app.aave.com"
dnslink:query: "app.aave.com"
dnslink:answer: {"Status":0,"TC":false,"RD":true,"RA":true,"AD":true,"CD":false,"Question":[{"name":"app.aave.com.","type":16}],"Authority":[{"name":"aave.com.","type":6,"TTL":1800,"data":"art.ns.cloudflare.com. dns.cloudflare.com. 2332124099 10000 2400 604800 1800"}],"Comment":"Response from 173.245.59.102."}
dnslink:query: "_dnslink.app.aave.com"
dnslink:answer: {"Status":0,"TC":false,"RD":true,"RA":true,"AD":true,"CD":false,"Question":[{"name":"_dnslink.app.aave.com.","type":16}],"Answer":[{"name":"_dnslink.app.aave.com.","type":16,"TTL":300,"data":"dnslink=/ipfs/bafybeifcaqowoyito3qvsmbwbiugsu4umlxn4ehu223hvtubbfvwyuxjoe/"}],"Comment":"Response from 172.64.32.152."}
dnslink:answer: {"Status":0,"TC":false,"RD":true,"RA":true,"AD":true,"CD":false,"Question":[{"name":"app.aave.com","type":16}],"Authority":[{"name":"aave.com","type":6,"TTL":1800,"data":"art.ns.cloudflare.com. dns.cloudflare.com. 2332124099 10000 2400 604800 1800"}]}
dnslink:cache: "/ipfs/bafybeifcaqowoyito3qvsmbwbiugsu4umlxn4ehu223hvtubbfvwyuxjoe/"

Result: Invalid value

SgtPooki added a commit that referenced this issue Jan 30, 2024
@SgtPooki SgtPooki self-assigned this Jan 30, 2024
SgtPooki added a commit that referenced this issue Jan 30, 2024
SgtPooki added a commit that referenced this issue Jan 30, 2024
BREAKING CHANGE: ipns resolve now supports paths, so the return type is
now `{ path: string, cid: CID }` instead of just `CID`

\#### Before

\```typescript
const cidFromPeerId = await ipns.resolve(peerId)
const cidFromDnsLink = await ipns.resolve(domainName)
\```

\#### After

\```typescript
const { cid, path } = await ipns.resolve(peerId)
const { cid, path } = await ipns.resolve(domainName)
\```
@SgtPooki SgtPooki linked a pull request Jan 30, 2024 that will close this issue
achingbrain added a commit that referenced this issue Jan 31, 2024
Fixes #402

Adds support for publishing/resolving paths.  Eg:

- `/ipfs/QmFoo/deep/link.txt`
- `/ipns/example.com/deep/link.html`

BREAKING CHANGE: to support paths in `@helia/ipns`, the return type of `ipns.resolve` is now `{ path: string, cid: CID }` instead of just `CID`

**Before**

```typescript
const cidFromPeerId = await ipns.resolve(peerId)
const cidFromDnsLink = await ipns.resolve(domainName)
```

**After**

```typescript
const { cid, path } = await ipns.resolve(peerId)
const { cid, path } = await ipns.resolve(domainName)
```

---------

Co-authored-by: achingbrain <alex@achingbrain.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant