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

Ignore non-IPFS content paths in DNSLink records #369

Closed
martinheidegger opened this issue May 10, 2021 · 4 comments · Fixed by #466 or #462
Closed

Ignore non-IPFS content paths in DNSLink records #369

martinheidegger opened this issue May 10, 2021 · 4 comments · Fixed by #466 or #462
Assignees
Labels
dif/medium Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature

Comments

@martinheidegger
Copy link

Version:
js-ipfs version: 0.5.4-511147bedd51be3151de44a90fefe9425bfbcd50
interface-ipfs-core version: ^0.144.2
ipfs-http-client version: undefined
Repo version: 10
System version: x64/darwin
Node.js version: v16.1.0
Commit: 511147bedd51be3151de44a90fefe9425bfbcd50
Platform
Darwin xxx 20.4.0 Darwin Kernel Version 20.4.0
  • Subsystem:
    dns-nodejs

Severity:

Low - An optional functionality does not work.

Description:

dns-nodejs returns currently the first dnslink= txt entry that can be found for a given domain, independent from the given protocol prefix, which prevents other decentralized protocols from using dnslink. It would probably be good for dnslink-nodejs to look for the ipfs dnslink entry only.

Note: I also opened related issues on go-dnslink#14 and js-dnslink#5.

Steps to reproduce the error:

Create a domain with a dnslink=/hyper/... entry and dnslink=/ipfs option and see how (depending on the order returned by the dns function!) how it will not consistently return the correct entry.

@martinheidegger martinheidegger added the need/triage Needs initial labeling and prioritization label May 10, 2021
@lidel lidel self-assigned this Sep 10, 2021
@lidel lidel added effort/hours Estimated to take one or several hours help wanted Seeking public contribution on this issue P3 Low: Not priority right now labels Sep 10, 2021
@lidel lidel removed their assignment Sep 10, 2021
@lidel
Copy link
Member

lidel commented Sep 10, 2021

👍

The gist here is to ignore DNSLink records with content paths with prefix other than /ipfs/ and /ipns/.
This way other systems can use publish their own DNSLink records without impacting resolution in js-ipfs.

We should also interpret /dnslink as /ipns and try to resolve it to the final form.

@lidel lidel changed the title Limit dnslink-nodejs to use ipfs dnslink only Ignore non-IPFS content paths in DNSLink records Sep 10, 2021
@BigLep BigLep removed the need/triage Needs initial labeling and prioritization label Sep 24, 2021
@lidel lidel added the status/ready Ready to be worked label Apr 22, 2022
@SgtPooki
Copy link
Member

js-ipfs is being deprecated in favor of Helia. You can ipfs/js-ipfs#4336 and read the migration guide.

Please feel to reopen with any comments by 2023-06-02. We will do a final pass on reopened issues afterward (see ipfs/js-ipfs#4336).

Assigning to @achingbrain to answer whether this issue is already resolved in Helia, or if this issue needs to be migrated to that repo!

@achingbrain achingbrain transferred this issue from ipfs/js-ipfs May 30, 2023
@achingbrain achingbrain transferred this issue from ipfs/helia-ipns Jan 8, 2024
@achingbrain achingbrain added dif/medium Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature and removed P3 Low: Not priority right now help wanted Seeking public contribution on this issue effort/hours Estimated to take one or several hours status/ready Ready to be worked labels Jan 8, 2024
@SgtPooki
Copy link
Member

I believe this is fixed by #466?

@achingbrain
Copy link
Member

Yes, #466 will close this

achingbrain added a commit that referenced this issue Mar 14, 2024
Uses the `.dns` property from #465 to resolve DNS `TXT` records.

This allows configuring discrete resolvers for different TLDs, unifies caching across different use of DNS (e.g. dnsaddr multiaddrs), etc.

Refs: ipfs/helia-verified-fetch#13 (comment)
Fixes: #369

BREAKING CHANGE: requires @helia/[email protected] or later, `resolveDns` has been renamed `resolveDNSLink`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dif/medium Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature
Projects
No open projects
Status: Done
5 participants