Skip to content
This repository was archived by the owner on Jul 21, 2023. It is now read-only.

fix: ipv6 naming with multiaddr-to-uri package#81

Merged
jacobheun merged 2 commits into
masterfrom
chore/use-multiaddr-to-uri-package
Jan 24, 2019
Merged

fix: ipv6 naming with multiaddr-to-uri package#81
jacobheun merged 2 commits into
masterfrom
chore/use-multiaddr-to-uri-package

Conversation

@vasco-santos

@vasco-santos vasco-santos commented Jan 23, 2019

Copy link
Copy Markdown
Member

When dht is enabled ipfs/js-ipfs#856, we found an issue where some browser nodes were using ipv6. As soon as we connect them, the connection fails as a result of using an invalid url.

Example: ws://::1:4002

Node url implementation expects a ipv6 address in the following format: ws://[::1]:4002

In addition, we do not have to maintain the multiaddr to url code previously here.

@ghost ghost assigned vasco-santos Jan 23, 2019
@ghost ghost added the status/in-progress In progress label Jan 23, 2019

@jacobheun jacobheun left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should get some tests added for this. Currently there are several skipped, empty ipv6 tests. Right now we're only testing filtering. It would be great to get listening and dialing tests in place for at least basic dialing and listening.

@vasco-santos

Copy link
Copy Markdown
Member Author

@jacobheun I agree, I will add those tests

@vasco-santos vasco-santos force-pushed the chore/use-multiaddr-to-uri-package branch from 94f7d00 to b7a9359 Compare January 24, 2019 10:47
@vasco-santos vasco-santos force-pushed the chore/use-multiaddr-to-uri-package branch from b7a9359 to dd15f5a Compare January 24, 2019 11:10
@jacobheun jacobheun merged commit 93ef7c3 into master Jan 24, 2019
@jacobheun jacobheun deleted the chore/use-multiaddr-to-uri-package branch January 24, 2019 15:22
@ghost ghost removed the status/in-progress In progress label Jan 24, 2019
@jacobheun

Copy link
Copy Markdown
Contributor

This fix is available in v0.12.2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants