Skip to content

add unix protocol support#84

Merged
jacobheun merged 3 commits into
masterfrom
feat/unix
Feb 25, 2019
Merged

add unix protocol support#84
jacobheun merged 3 commits into
masterfrom
feat/unix

Conversation

@jacobheun
Copy link
Copy Markdown
Contributor

@jacobheun jacobheun commented Feb 22, 2019

This also updates the protocol table to match the latest at multiaddr/protocols.csv

resolves #82

@ghost ghost assigned jacobheun Feb 22, 2019
@ghost ghost added the status/in-progress In progress label Feb 22, 2019
@jacobheun jacobheun marked this pull request as ready for review February 22, 2019 11:59
@jacobheun jacobheun changed the title [WIP] add unix protocol support add unix protocol support Feb 22, 2019
Copy link
Copy Markdown
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread src/codec.js
if (proto.path) {
tuples.push([
part,
// TODO: should we need to check each path part to see if it's a proto?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we can go with this now, and when we add new protocols we thing about it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah. It should be reasonable for us to have an address that's /unix/a/b/c/d/e/f/ip4/0.0.0.0/tcp/1234, but users would need to avoid addresses like /unix/c/p2p/d.sock/ip4/0.0.0.0/tcp/12341, since it has a proto name in the unix path. Having at least basic support for unix addresses is the important thing right now.

@jacobheun jacobheun merged commit d4d3d9b into master Feb 25, 2019
@jacobheun jacobheun deleted the feat/unix branch February 25, 2019 15:08
@ghost ghost removed the status/in-progress In progress label Feb 25, 2019
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.

Add unix path protocol

2 participants