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

credentials: Use net.SplitHostPort to safely parse IPv6 authorities in ClientHandshake #3082

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

jpbetz
Copy link
Contributor

@jpbetz jpbetz commented Oct 8, 2019

Correctly split authorities with IPv6 addresses and no port, e.g. [::1].

@jpbetz jpbetz changed the title Use net.SplitHostPort safely parse IPv6 authorities in ClientHandshake Use net.SplitHostPort to safely parse IPv6 authorities in ClientHandshake Oct 8, 2019
@jpbetz jpbetz changed the title Use net.SplitHostPort to safely parse IPv6 authorities in ClientHandshake credentials: Use net.SplitHostPort to safely parse IPv6 authorities in ClientHandshake Oct 8, 2019
@jpbetz
Copy link
Contributor Author

jpbetz commented Oct 8, 2019

cc @liggitt

Copy link
Contributor

@easwars easwars left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

@easwars easwars added the Type: Internal Cleanup Refactors, etc label Oct 8, 2019
@easwars easwars added this to the 1.25 Release milestone Oct 8, 2019
@jpbetz
Copy link
Contributor Author

jpbetz commented Oct 8, 2019

Looks like Travis CI does not support IPv6: "Failed to listen: listen tcp [::1]:0: bind: cannot assign requested address" (https://travis-ci.org/grpc/grpc-go/jobs/595347032). I'll comment out the IPv6 test case with a comment explaining.

@liggitt
Copy link

liggitt commented Oct 8, 2019

Or detect lack of ipv6 support and skip the test

@easwars
Copy link
Contributor

easwars commented Oct 8, 2019

Something like this should suffice. Thanks.

	if err != nil {
		if strings.Contains(err.Error(), "bind: cannot assign requested address") {
			t.Skip("missing IPv6 support")
		}

credentials/credentials_test.go Outdated Show resolved Hide resolved
credentials/credentials_test.go Outdated Show resolved Hide resolved
@easwars easwars merged commit f07f2cf into grpc:master Oct 9, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants