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

Drop hostname in codec test #247

Merged
merged 1 commit into from
Jun 26, 2018
Merged

Drop hostname in codec test #247

merged 1 commit into from
Jun 26, 2018

Conversation

teeler
Copy link
Contributor

@teeler teeler commented May 29, 2018

Drop the 127.0.0.1 in the codec test to make the test ipv6 compliant.

(it will no longer be listening on localhost explicitly, i'm happy to revert this if you're uncomfortable with that).

Drop the 127.0.0.1 in the codec test to make the test ipv6 compliant.
@teeler
Copy link
Contributor Author

teeler commented May 31, 2018

friendly ping?

@ugorji ugorji merged commit d228c2b into ugorji:master Jun 26, 2018
@ugorji
Copy link
Owner

ugorji commented Jun 26, 2018

Seems this is causing some CI build breaks. Reverting.

@teeler
Copy link
Contributor Author

teeler commented Jun 26, 2018

I'd strongly suggest questioning the setup of your CI tests if this change breaks them... ;) but ¯_(ツ)_/¯

@ugorji
Copy link
Owner

ugorji commented Jun 28, 2018

Seems listening on all addresses, and then trying to dial onto an ipv6 address fails on go 1.7 and go 1.8. See https://travis-ci.org/ugorji/go

We really should only listen on the loopback address (localhost), so as not to possibly fall into this situation, and also so as not to deal with firewall issues and accepting connections from outside.

See golang/go#18806 (for when fix for dialing to an addr without ipv6 configured on the machine was put in place).

Will revert, or adjust the code.

ugorji added a commit that referenced this pull request Jun 28, 2018
This ensures that
- net.Dial works just fine on go 1.7 and 1.8
- firewall concerns do not come up, because the server is
  listening for connections outside local machine

Updates #247
- with aim to revert the PR
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.

2 participants