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.

Sign up for GitHub

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

Jump to bottom

resolver: always fall back to default resolver when target does not follow URI scheme #1889

Merged
merged 2 commits into from
Mar 8, 2018
Merged

resolver: always fall back to default resolver when target does not follow URI scheme #1889

merged 2 commits into from
Mar 8, 2018

Conversation

menghanl
Copy link
Contributor

@menghanl menghanl requested a review from dfawley February 28, 2018 22:33
@menghanl menghanl mentioned this pull request Feb 28, 2018
Merged
dfawley
dfawley reviewed Feb 28, 2018
View reviewed changes
resolver_conn_wrapper_test.go
@@ -70,6 +69,10 @@ func TestParseTargetString(t *testing.T) {
{"google.com", resolver.Target{"", "", "google.com"}},
{"google.com/?a=b", resolver.Target{"", "", "google.com/?a=b"}},
{"/unix/socket/address", resolver.Target{"", "", "/unix/socket/address"}},

// If we can only parse part of the target.
{"://", resolver.Target{"", "", "://"}},
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth testing other partial matches like "[a]:[b]" and "[a]:/[b]"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@dfawley dfawley assigned menghanl and unassigned dfawley Mar 1, 2018
@menghanl menghanl assigned dfawley and unassigned menghanl Mar 1, 2018
@dfawley dfawley assigned menghanl and unassigned dfawley Mar 1, 2018
@dfawley dfawley mentioned this pull request Mar 6, 2018
Closed
@dfawley dfawley changed the title resolver: skip parsing target if target format is not recognized resolver: always fall back to default resolver when target does not follow URI scheme Mar 8, 2018
@dfawley dfawley merged commit 90dca43 into grpc:master Mar 8, 2018
@menghanl menghanl deleted the scheme_parse_failure branch March 26, 2018 20:38
@menghanl menghanl mentioned this pull request Mar 26, 2018
Merged
@menghanl menghanl added this to the 1.11 Release milestone Mar 27, 2018
lyuxuan pushed a commit to lyuxuan/grpc-go that referenced this pull request Apr 4, 2018
@menghanl @lyuxuan
resolver: always fall back to default resolver when target does not f…
7e581cf 
…ollow URI scheme (grpc#1889)

Previously, any target with "://" would be handled according to the URI scheme even though it did not contain a third slash.
@lock lock bot locked as resolved and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dfawley dfawley dfawley approved these changes

Assignees

@menghanl menghanl

Labels
Type: Bug
Projects
None yet
Milestone
1.11 Release
Development

Successfully merging this pull request may close these issues.
2 participants
@menghanl @dfawley