Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
- strip the leading "/" unconditionally in parseTarget()
- do not strip the leading "/" from endpoint in DialContext() since this
  is done in parseTarget()
- move the resolver building to inside the if in
  parseTargetAndFindResolver()
- doc comments in resolver.go
  • Loading branch information
easwars committed Oct 5, 2021
1 parent c36957f commit 7c4733f
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 15 deletions.
18 changes: 9 additions & 9 deletions clientconn.go
Expand Up @@ -259,7 +259,7 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *
// scheme and empty hostname (":port").
// TODO: Define an optional interface on the resolver builder to return the
// authority given the user's dial target.
cc.authority = strings.TrimPrefix(cc.parsedTarget.Endpoint, "/")
cc.authority = cc.parsedTarget.Endpoint
if strings.HasPrefix(cc.authority, ":") {
cc.authority = "localhost" + cc.authority
}
Expand Down Expand Up @@ -1630,16 +1630,18 @@ func (cc *ClientConn) connectionError() error {

func (cc *ClientConn) parseTargetAndFindResolver() (resolver.Builder, error) {
channelz.Infof(logger, cc.channelzID, "original dial target is: %q", cc.target)

var rb resolver.Builder
parsedTarget, err := parseTarget(cc.target)
if err != nil {
channelz.Infof(logger, cc.channelzID, "dial target %q parse failed: %v", cc.target, err)
} else {
channelz.Infof(logger, cc.channelzID, "parsed dial target is: %+v", parsedTarget)
}
rb := cc.getResolver(parsedTarget.Scheme)
if rb != nil {
cc.parsedTarget = parsedTarget
return rb, nil
rb = cc.getResolver(parsedTarget.Scheme)
if rb != nil {
cc.parsedTarget = parsedTarget
return rb, nil
}
}

// We are here because the user's dial target did not contain a scheme or
Expand Down Expand Up @@ -1687,9 +1689,7 @@ func parseTarget(target string) (resolver.Target, error) {
if endpoint == "" {
endpoint = u.Opaque
}
if strings.HasPrefix(target, fmt.Sprintf("%s://", u.Scheme)) {
endpoint = strings.TrimPrefix(endpoint, "/")
}
endpoint = strings.TrimPrefix(endpoint, "/")
return resolver.Target{
Scheme: u.Scheme,
Authority: u.Host,
Expand Down
6 changes: 3 additions & 3 deletions clientconn_parsed_target_test.go
Expand Up @@ -69,15 +69,15 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) {
{target: "unix-abstract:a b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a b"}},
{target: "unix-abstract:a:b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a:b"}},
{target: "unix-abstract:a-b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a-b"}},
{target: "unix-abstract:/ a///://::!@#$%25^&*()b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "/ a///://::!@"}},
{target: "unix-abstract:/ a///://::!@#$%25^&*()b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: " a///://::!@"}},
{target: "unix-abstract:passthrough:abc", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "passthrough:abc"}},
{target: "unix-abstract:unix:///abc", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "unix:///abc"}},
{target: "unix-abstract:///a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a/b/c"}},
{target: "unix-abstract:///", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: ""}},
{target: "passthrough:///unix:///a/b/c", wantParsed: resolver.Target{Scheme: "passthrough", Authority: "", Endpoint: "unix:///a/b/c"}},

// Cases for `scheme:absolute-path`.
{target: "dns:/a/b/c", wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "/a/b/c"}},
{target: "dns:/a/b/c", wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "a/b/c"}},
{target: "unregistered:/a/b/c", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unregistered:/a/b/c"}},
}

Expand Down Expand Up @@ -130,7 +130,7 @@ func (s) TestParsedTarget_WithCustomDialer(t *testing.T) {
},
{
target: "unix:/a/b/c",
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "/a/b/c"},
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"},
wantDialerAddress: "unix:///a/b/c",
},
{
Expand Down
6 changes: 3 additions & 3 deletions resolver/resolver.go
Expand Up @@ -217,11 +217,11 @@ type ClientConn interface {
// Examples:
//
// - "dns://some_authority/foo.bar"
// Target{Scheme: "dns", Authority: "some_authority", Endpoint: "/foo.bar"}
// Target{Scheme: "dns", Authority: "some_authority", Endpoint: "foo.bar"}
// - "foo.bar"
// Target{Scheme: resolver.GetDefaultScheme(), Endpoint: "/foo.bar"}
// Target{Scheme: resolver.GetDefaultScheme(), Endpoint: "foo.bar"}
// - "unknown_scheme://authority/endpoint"
// Target{Scheme: resolver.GetDefaultScheme(), Endpoint: "/unknown_scheme://authority/endpoint"}
// Target{Scheme: resolver.GetDefaultScheme(), Endpoint: "unknown_scheme://authority/endpoint"}
type Target struct {
Scheme string
Authority string
Expand Down

0 comments on commit 7c4733f

Please sign in to comment.