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

DNS multiaddrs support #39

Merged
merged 8 commits into from
Jan 22, 2017
Merged

DNS multiaddrs support #39

merged 8 commits into from
Jan 22, 2017

Conversation

daviddias
Copy link
Member

No description provided.

@daviddias daviddias added the status/in-progress In progress label Jan 19, 2017
@daviddias daviddias requested review from a user and victorb January 19, 2017 15:51
@daviddias daviddias mentioned this pull request Jan 19, 2017
18 tasks
}

/**
* Returns if something is a Multiaddr that is a name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is copy pasta from above :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch :) fixed

case 53: // dns
case 54: // dns4
case 55: // dns6
return buf2str(buf)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/dns, /dns4, and /dns6 have only been reserved for near-future use, and it should be /exp-dns, /exp-dns4, /exp-dns6 for now until we're very confident it's right and have gotten some feedback from others too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling that is going to bite us, in the same way that today we have X- HTTP headers everywhere because the rationale was "We are going to try this and then remove it", in reallity what happen is that so many tools started checking for the X- specifically, that they never got removed.

See: https://tools.ietf.org/html/rfc6648
tl;dr; in the answer here: http://stackoverflow.com/questions/3561381/custom-http-headers-naming-conventions

3. Recommendations for Creators of New Parameters

...

SHOULD NOT prefix their parameter names with "X-" or similar constructs.
4. Recommendations for Protocol Designers

...

SHOULD NOT prohibit parameters with an "X-" prefix or similar constructs from being registered.
MUST NOT stipulate that a parameter with an "X-" prefix or similar constructs needs to be understood as unstandardized.
MUST NOT stipulate that a parameter without an "X-" prefix or similar constructs needs to be understood as standardized.

Right now, multiaddrs with DNS are just used to be passed around, to be announce and to be used for things like the WebSockets dialing to the Signalling Server on WebRTC-Star, I've avoided implementing the .resolve method for names as it is not directly a need for the current sprint and I agree with you, that shed will get some more coats of paint.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @jbenet can shed a few of his thoughts on this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with /dns4 and /dns6 and keep /dns for the future

<@lgierth> daviddias: you've changed my mind -- i think we should go with /dns4 and /dns6 for now, these are straightforward and don't need much thought about how to resolve, and we can keep /dns for the future. what do you think? i should probably carry it over to multiformats/multiaddr
<@daviddias> lgierth: sounds good, I can migrate the code to use /dns4 for now (and support for dns6 too) :)
<@lgierth> and /dns4 /dns6 do address a host unambigiuously (while with /dns there's probably going to be something like RFC6555)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daviddias daviddias merged commit ddaaf98 into master Jan 22, 2017
@daviddias daviddias deleted the feat/dns-support branch January 22, 2017 14:10
@daviddias daviddias removed the status/in-progress In progress label Jan 22, 2017
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 this pull request may close these issues.

3 participants