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

grpc: better RFC 3986 compliant target parsing #4817

Merged
merged 24 commits into from Oct 14, 2021
Merged

grpc: better RFC 3986 compliant target parsing #4817

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
fb12d07
grpc: increase compliance with RFC 3986 while parsing the dial target
easwars Sep 24, 2021
cee1b44
fix resolver in load_balancing example
easwars Sep 27, 2021
d19cd9b
fix resolver in name_resolving example
easwars Sep 27, 2021
0d0c1a5
fix typo
easwars Sep 27, 2021
c98608a
make unix target canonical in test
easwars Sep 27, 2021
fb4b9c0
add `Unparsed` field to `resolver.Target`
easwars Sep 29, 2021
2750421
make getServerName a method on cc instead of ac
easwars Sep 29, 2021
c522792
some comments
easwars Sep 29, 2021
4f56e2e
use passthrough whenever custom dialer is used
easwars Sep 29, 2021
ae490d0
resolver builder
easwars Sep 29, 2021
99fdc43
strip leading "/" from endpoint for backward compatibility
easwars Sep 30, 2021
f578ae5
moved these tests to clientconn_parsed_target_test.go
easwars Sep 30, 2021
1fa26dc
revert unwanted import change
easwars Sep 30, 2021
7936989
Merge branch 'master' into url_parse_attempt_3
easwars Sep 30, 2021
499f603
update comment for `ParsedURL`
easwars Sep 30, 2021
c5c4383
add tests for `scheme:absolute-path`
easwars Sep 30, 2021
5cff707
return error if ParsedURL is nil in unix resolver
easwars Sep 30, 2021
c36957f
store the parsed url by value in `resolver.Target`
easwars Oct 1, 2021
8546581
review comments
easwars Oct 5, 2021
78fcb25
review comments
easwars Oct 11, 2021
4fb05c3
edit todo for optional resolver interface
easwars Oct 11, 2021
c6057e5
ClientHandshake comments
easwars Oct 11, 2021
f230c53
deprecate old fields of resolver.Target and rename `ParsedURL` to `URL`
easwars Oct 11, 2021
5f96946
validate parsed URL in unit tests
easwars Oct 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion balancer/grpclb/grpclb.go
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@ import (
"context"
"errors"
"fmt"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -135,7 +136,7 @@ func (b *lbBuilder) Build(cc balancer.ClientConn, opt balancer.BuildOptions) bal

lb := &lbBalancer{
cc: newLBCacheClientConn(cc),
target: opt.Target.Endpoint,
target: strings.TrimPrefix(opt.Target.Endpoint, "/"),
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a behavior change.
We should try to keep the value of the existing fields unchanged.

Should gRPC trim the leading slash? What's the reason to not do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unix:/absolute-path parses as Scheme: "unix" Endpoint: "/path" and we cannot remove it in that case.

Endpoint according to the RFC does contain the leading slash, and implementations in C strip the leading /.

opt: opt,
fallbackTimeout: b.fallbackTimeout,
doneCh: make(chan struct{}),
Expand Down
8 changes: 6 additions & 2 deletions balancer/rls/internal/config.go
View file
Open in desktop
Expand Up @@ -22,15 +22,16 @@ import (
"bytes"
"encoding/json"
"fmt"
"net/url"
"time"

"github.com/golang/protobuf/jsonpb"
"github.com/golang/protobuf/ptypes"
durationpb "github.com/golang/protobuf/ptypes/duration"

"google.golang.org/grpc/balancer"
"google.golang.org/grpc/balancer/rls/internal/keys"
rlspb "google.golang.org/grpc/balancer/rls/internal/proto/grpc_lookup_v1"
"google.golang.org/grpc/internal/grpcutil"
"google.golang.org/grpc/resolver"
"google.golang.org/grpc/serviceconfig"
)
Expand Down Expand Up @@ -201,7 +202,10 @@ func (*rlsBB) ParseConfig(c json.RawMessage) (serviceconfig.LoadBalancingConfig,
if lookupService == "" {
return nil, fmt.Errorf("rls: empty lookup_service in service config {%+v}", string(c))
}
parsedTarget := grpcutil.ParseTarget(lookupService, false)
parsedTarget, err := url.Parse(lookupService)
if err != nil {
return nil, fmt.Errorf("rls: invalid target URI in lookup_service {%s}", lookupService)
}
if parsedTarget.Scheme == "" {
parsedTarget.Scheme = resolver.GetDefaultScheme()
}
Expand Down
140 changes: 102 additions & 38 deletions clientconn.go
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ import (
"errors"
"fmt"
"math"
"net/url"
"reflect"
"strings"
"sync"
Expand All @@ -37,7 +38,6 @@ import (
"google.golang.org/grpc/internal/backoff"
"google.golang.org/grpc/internal/channelz"
"google.golang.org/grpc/internal/grpcsync"
"google.golang.org/grpc/internal/grpcutil"
iresolver "google.golang.org/grpc/internal/resolver"
"google.golang.org/grpc/internal/transport"
"google.golang.org/grpc/keepalive"
Expand Down Expand Up @@ -248,37 +248,30 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *
}

// Determine the resolver to use.
cc.parsedTarget = grpcutil.ParseTarget(cc.target, cc.dopts.copts.Dialer != nil)
channelz.Infof(logger, cc.channelzID, "parsed scheme: %q", cc.parsedTarget.Scheme)
resolverBuilder := cc.getResolver(cc.parsedTarget.Scheme)
if resolverBuilder == nil {
// If resolver builder is still nil, the parsed target's scheme is
// not registered. Fallback to default resolver and set Endpoint to
// the original target.
channelz.Infof(logger, cc.channelzID, "scheme %q not registered, fallback to default scheme", cc.parsedTarget.Scheme)
cc.parsedTarget = resolver.Target{
Scheme: resolver.GetDefaultScheme(),
Endpoint: target,
}
resolverBuilder = cc.getResolver(cc.parsedTarget.Scheme)
if resolverBuilder == nil {
return nil, fmt.Errorf("could not get resolver for default scheme: %q", cc.parsedTarget.Scheme)
}
resolverBuilder, err := cc.parseTargetAndFindResolver()
if err != nil {
return nil, err
}

creds := cc.dopts.copts.TransportCredentials
if creds != nil && creds.Info().ServerName != "" {
cc.authority = creds.Info().ServerName
} else if cc.dopts.insecure && cc.dopts.authority != "" {
cc.authority = cc.dopts.authority
} else if strings.HasPrefix(cc.target, "unix:") || strings.HasPrefix(cc.target, "unix-abstract:") {
// Determine the authority for the channel. For now, we will use the parsed
// endpoint from the user's dial target (with a leading "/" stripped), as
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is talking about the leading /.
Delete that? Because the parsed endpoint's leading / is removed during parse()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored this logic into a separate method. The comment is also gone now.

// the default value. We do have some special handling for targets with unix
// scheme and empty hostname (":port").
// TODO: Define an optional interface on the name resolver to return the
// authority given the user's dial target.
dfawley marked this conversation as resolved.
Show resolved Hide resolved
cc.authority = strings.TrimPrefix(cc.parsedTarget.Endpoint, "/")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still necessary? I thought the leading slash is stripped in parse()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not required anymore. Deleted it.

if strings.HasPrefix(cc.authority, ":") {
cc.authority = "localhost" + cc.authority
menghanl marked this conversation as resolved.
Show resolved Hide resolved
}
if cc.parsedTarget.Scheme == "unix" || cc.parsedTarget.Scheme == "unix-abstract" {
cc.authority = "localhost"
} else if strings.HasPrefix(cc.parsedTarget.Endpoint, ":") {
cc.authority = "localhost" + cc.parsedTarget.Endpoint
} else {
// Use endpoint from "scheme://authority/endpoint" as the default
// authority for ClientConn.
cc.authority = cc.parsedTarget.Endpoint
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we eliminate this by making sure the addresses output by the unix resolver set ServerName to localhost? If so, let's do that instead.. I would rather avoid as many special cases as possible in generic places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason we don't want to rely on the resolver.Address.ServerName to determine the channel's authority at this point in time is because if we do that then we wont be able to pass the correct authority to balancer.Build(). The resolver.Address.ServerName is better used as an override.

Copy link
Member

Choose a reason for hiding this comment

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

But for the unix resolver, the balancer is going to be pick first, which doesn't/shouldn't care about the authority. Most balancers shouldn't care, right -- just RLS/gRPCLB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, none of our balancers care, except rls/grpclb.
But given we are passing that to the balancer in its BuildOptions and the Target() method on the balancer.ClientConn is deprecated in favor of the former, we should ideally set the correct value there, irrespective of whether or not one of our balancers cares about it. Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is your gripe here only about the special handling for unix or something else? Also, once we have the optional interface on the resolver builder to get the authority, we will not need this special case.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, for the special handling of unix and unix-abstract. We shouldn't need to do this if we set ServerName in those resolvers instead. Any balancer used with those schemes shouldn't care about the authority. Only RLS and gRPCLB care about the "channel authority" (so they can use the same one with their backends), and it doesn't make sense to use those with a unix address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These special checks remain for now. But as promised I will take care of them in a follow up PR where I will introduce the optional interface on the resolver builder.

// The only authority override supported at this point in time is the one
// using the WithAuthority dial option, and this trumps over any per-address
// overrides specified by the name resolver in the
// resolver.Address.ServerName field.
dfawley marked this conversation as resolved.
Show resolved Hide resolved
if cc.dopts.authority != "" {
cc.authority = cc.dopts.authority
}

if cc.dopts.scChan != nil && !scSet {
Expand Down Expand Up @@ -902,10 +895,7 @@ func (ac *addrConn) tryUpdateAddrs(addrs []resolver.Address) bool {
// ac.state is Ready, try to find the connected address.
var curAddrFound bool
for _, a := range addrs {
// a.ServerName takes precedent over ClientConn authority, if present.
if a.ServerName == "" {
a.ServerName = ac.cc.authority
}
a.ServerName = ac.getServerName(a)
if reflect.DeepEqual(ac.curAddr, a) {
curAddrFound = true
break
Expand All @@ -919,6 +909,25 @@ func (ac *addrConn) tryUpdateAddrs(addrs []resolver.Address) bool {
return curAddrFound
}

// getServerName determines the serverName to be used in the connection
// handshake. The default value for the serverName is the authority on the
// ClientConn, which either comes from the user's dial target or through an
// authority override specified using the WithAuthority dial option. Name
// resolvers can specify a per-address override for the serverName through the
// resolver.Address.ServerName field which is used only if the WithAuthority
// dial option was not used. The rationale is that per-address authority
// overrides specified by the name resolver can represent a security risk, while
// an override specified by the user is more dependable since they probably know
// what they are doing.
func (ac *addrConn) getServerName(addr resolver.Address) string {
dfawley marked this conversation as resolved.
Show resolved Hide resolved
if ac.cc.dopts.authority == "" {
if addr.ServerName != "" {
return addr.ServerName
}
}
easwars marked this conversation as resolved.
Show resolved Hide resolved
return ac.cc.authority
}

func getMethodConfig(sc *ServiceConfig, method string) MethodConfig {
if sc == nil {
return MethodConfig{}
Expand Down Expand Up @@ -1275,11 +1284,7 @@ func (ac *addrConn) createTransport(addr resolver.Address, copts transport.Conne
prefaceReceived := grpcsync.NewEvent()
connClosed := grpcsync.NewEvent()

// addr.ServerName takes precedent over ClientConn authority, if present.
if addr.ServerName == "" {
addr.ServerName = ac.cc.authority
}

addr.ServerName = ac.getServerName(addr)
hctx, hcancel := context.WithCancel(ac.ctx)
hcStarted := false // protected by ac.mu

Expand Down Expand Up @@ -1621,3 +1626,62 @@ func (cc *ClientConn) connectionError() error {
defer cc.lceMu.Unlock()
return cc.lastConnectionError
}

func (cc *ClientConn) parseTargetAndFindResolver() (resolver.Builder, error) {
channelz.Infof(logger, cc.channelzID, "original dial target is: %q", cc.target)
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this inside else

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. Thanks.

if rb != nil {
cc.parsedTarget = parsedTarget
return rb, nil
}

// We are here because the user's dial target did not contain a scheme or
// specified an unregistered scheme. We should fallback to the default
// scheme, except when no scheme was specified in the user's dial target and
// a custom dialer was specified. In the latter case, we should always use
// passthrough scheme.
defScheme := resolver.GetDefaultScheme()
if parsedTarget.Scheme == "" && cc.dopts.copts.Dialer != nil {
defScheme = "passthrough"
}
dfawley marked this conversation as resolved.
Show resolved Hide resolved
channelz.Infof(logger, cc.channelzID, "fallback to scheme %q", defScheme)
canonicalTarget := defScheme + ":///" + cc.target

parsedTarget, err = parseTarget(canonicalTarget)
if err != nil {
channelz.Infof(logger, cc.channelzID, "dial target %q parse failed: %v", canonicalTarget, err)
return nil, err
}
channelz.Infof(logger, cc.channelzID, "parsed dial target is: %+v", parsedTarget)
rb = cc.getResolver(parsedTarget.Scheme)
if rb == nil {
return nil, fmt.Errorf("could not get resolver for default scheme: %q", parsedTarget.Scheme)
}
cc.parsedTarget = parsedTarget
return rb, nil
}

// parseTarget uses RFC 3986 semantics to parse the given target into a
// resolver.Target struct containing scheme, authority and endpoint. Query
// params are stripped from the endpoint.
func parseTarget(target string) (resolver.Target, error) {
u, err := url.Parse(target)
if err != nil {
return resolver.Target{}, err
}
path := u.Path
if path == "" {
path = u.Opaque
}
return resolver.Target{
Scheme: u.Scheme,
Authority: u.Host,
Endpoint: path,
}, nil
}
17 changes: 9 additions & 8 deletions clientconn_authority_test.go
View file
Open in desktop
Expand Up @@ -47,10 +47,12 @@ func (s) TestClientConnAuthority(t *testing.T) {
wantAuthority: "Non-Existent.Server:8080",
},
{
name: "override-via-creds",
target: "Non-Existent.Server:8080",
opts: []DialOption{WithTransportCredentials(creds)},
wantAuthority: serverNameOverride,
name: "override-via-creds",
target: "Non-Existent.Server:8080",
opts: []DialOption{WithTransportCredentials(creds)},
// Overriding authority from the transport credentials is not
// supported anymore.
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this a behavior change? "not supported" and "not recommended" and "deprecated" is fine but we shouldn't change behavior that users may be relying upon. If we're reasonably sure nobody is using this (IIRC we added it only for grpclb?) then maybe we can try it and be ready to revert it, but if it's easy enough to keep the behavior, then we should.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Supporting this would mean adding another condition to the logic in DialContext around lines 250-270 where we determine channel authority. Should be totally doable. But since we are supporting WithAuthority for transport creds as well, supporting this would be quite odd.

Again, using the value specified through OverrideServerName as the channel authority is not something that is documented. That method documentation just says:

	// OverrideServerName overrides the server name used to verify the hostname
	// on the returned certificates from the server.

So, if the transport creds uses the value specified through this call in its handshake, that should be good enough, right? This is what our TLS creds implementation currently does. https://github.com/grpc/grpc-go/blob/master/credentials/tls.go#L76-L83

Copy link
Member

Choose a reason for hiding this comment

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

That logic seems fine, but then the comment should be a little different here. It should say the channel authority is not impacted, but the credentials implementation may still validate using that authority instead of the one provided by the channel.

However, is this okay for security reasons, to have the channel thinking the authority is one thing, but the credentials silently using a different authority? It's a bit of a weird thing in the first place, but it seems like it should notify the channel if it's happening. Should we at least deprecate all things related to the credentials override?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep supporting this, unless it's too messy. But it sounds not too bad to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-added the support.

wantAuthority: "Non-Existent.Server:8080",
},
{
name: "override-via-WithAuthority",
Expand All @@ -59,11 +61,10 @@ func (s) TestClientConnAuthority(t *testing.T) {
wantAuthority: "authority-override",
},
{
name: "override-via-creds-and-WithAuthority",
target: "Non-Existent.Server:8080",
// WithAuthority override works only for insecure creds.
name: "override-via-creds-and-WithAuthority",
target: "Non-Existent.Server:8080",
opts: []DialOption{WithTransportCredentials(creds), WithAuthority("authority-override")},
wantAuthority: serverNameOverride,
wantAuthority: "authority-override",
},
{
name: "unix relative",
Expand Down
66 changes: 33 additions & 33 deletions clientconn_parsed_target_test.go
View file
Open in desktop
Expand Up @@ -35,45 +35,43 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) {
wantParsed resolver.Target
}{
// No scheme is specified.
{target: "", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: ""}},
{target: "://", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://"}},
{target: ":///", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: ":///"}},
{target: "://a/", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://a/"}},
{target: ":///a", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: ":///a"}},
{target: "://a/b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://a/b"}},
{target: "/", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/"}},
{target: "a/b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a/b"}},
{target: "a//b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a//b"}},
{target: "google.com", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "google.com"}},
{target: "google.com/?a=b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "google.com/?a=b"}},
{target: "/unix/socket/address", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/unix/socket/address"}},
{target: "", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/"}},
{target: "://", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/://"}},
{target: ":///", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/:///"}},
{target: "://a/", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/://a/"}},
{target: ":///a", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/:///a"}},
{target: "://a/b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/://a/b"}},
{target: "/", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "//"}},
{target: "a/b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a/b"}},
{target: "a//b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a//b"}},
{target: "google.com", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/google.com"}},
{target: "google.com/?a=b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/google.com/"}},
{target: "/unix/socket/address", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "//unix/socket/address"}},

// An unregistered scheme is specified.
{target: "a:///", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a:///"}},
{target: "a://b/", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a://b/"}},
{target: "a:///b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a:///b"}},
{target: "a://b/c", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a://b/c"}},
{target: "a:b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a:b"}},
{target: "a:/b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a:/b"}},
{target: "a://b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a://b"}},
{target: "a:///", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a:///"}},
{target: "a://b/", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a://b/"}},
{target: "a:///b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a:///b"}},
{target: "a://b/c", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a://b/c"}},
{target: "a:b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a:b"}},
{target: "a:/b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a:/b"}},
{target: "a://b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/a://b"}},

// A registered scheme is specified.
{target: "dns:///google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "google.com"}},
{target: "dns://a.server.com/google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "google.com"}},
{target: "dns://a.server.com/google.com/?a=b", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "google.com/?a=b"}},
{target: "dns:///google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "/google.com"}},
{target: "dns://a.server.com/google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "/google.com"}},
{target: "dns://a.server.com/google.com/?a=b", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "/google.com/"}},
{target: "unix:///a/b/c", wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "/a/b/c"}},
{target: "unix-abstract:a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a/b/c"}},
{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///://::!@#$%^&*()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: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: "unix-abstract://authority", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "//authority"}},
{target: "unix://domain", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unix://domain"}},
{target: "passthrough:///unix:///a/b/c", wantParsed: resolver.Target{Scheme: "passthrough", Authority: "", Endpoint: "unix:///a/b/c"}},
{target: "passthrough:///unix:///a/b/c", wantParsed: resolver.Target{Scheme: "passthrough", Authority: "", Endpoint: "/unix:///a/b/c"}},
}

for _, test := range tests {
Expand All @@ -94,7 +92,9 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) {
func (s) TestParsedTarget_Failure_WithoutCustomDialer(t *testing.T) {
targets := []string{
"unix://a/b/c",
"unix://authority",
"unix-abstract://authority/a/b/c",
"unix-abstract://authority",
}

for _, target := range targets {
Expand All @@ -118,13 +118,13 @@ func (s) TestParsedTarget_WithCustomDialer(t *testing.T) {
// different behaviors with a custom dialer.
{
target: "unix:a/b/c",
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unix:a/b/c"},
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"},
wantDialerAddress: "unix:a/b/c",
},
{
target: "unix:/a/b/c",
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unix:/a/b/c"},
wantDialerAddress: "unix:/a/b/c",
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "/a/b/c"},
wantDialerAddress: "unix:///a/b/c",
},
{
target: "unix:///a/b/c",
Expand All @@ -133,22 +133,22 @@ func (s) TestParsedTarget_WithCustomDialer(t *testing.T) {
},
{
target: "dns:///127.0.0.1:50051",
wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "127.0.0.1:50051"},
wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "/127.0.0.1:50051"},
wantDialerAddress: "127.0.0.1:50051",
},
{
target: ":///127.0.0.1:50051",
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: ":///127.0.0.1:50051"},
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/:///127.0.0.1:50051"},
wantDialerAddress: ":///127.0.0.1:50051",
},
{
target: "dns://authority/127.0.0.1:50051",
wantParsed: resolver.Target{Scheme: "dns", Authority: "authority", Endpoint: "127.0.0.1:50051"},
wantParsed: resolver.Target{Scheme: "dns", Authority: "authority", Endpoint: "/127.0.0.1:50051"},
wantDialerAddress: "127.0.0.1:50051",
},
{
target: "://authority/127.0.0.1:50051",
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://authority/127.0.0.1:50051"},
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/://authority/127.0.0.1:50051"},
wantDialerAddress: "://authority/127.0.0.1:50051",
},
}
Expand Down