Skip to content

Commit 9c8aa04

Browse files
committed
grpcutil: add ParseTarget function and refactor call sites
Add ParseTarget that accepts a default scheme parameter and returns *url.URL. This allows callers to use the parsed result and handles the common pattern of falling back to a default scheme. Update clientconn.go and balancer/rls/config.go to use the new function, removing duplicate parsing logic.
1 parent 3379b53 commit 9c8aa04

File tree

4 files changed

+116
-38
lines changed

4 files changed

+116
-38
lines changed

balancer/rls/config.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ import (
2222
"bytes"
2323
"encoding/json"
2424
"fmt"
25-
"net/url"
2625
"time"
2726

2827
"google.golang.org/grpc/balancer"
2928
"google.golang.org/grpc/balancer/rls/internal/keys"
3029
"google.golang.org/grpc/internal"
30+
"google.golang.org/grpc/internal/grpcutil"
3131
"google.golang.org/grpc/internal/pretty"
3232
rlspb "google.golang.org/grpc/internal/proto/grpc_lookup_v1"
3333
"google.golang.org/grpc/resolver"
@@ -195,16 +195,9 @@ func parseRLSProto(rlsProto *rlspb.RouteLookupConfig) (*lbConfig, error) {
195195
if lookupService == "" {
196196
return nil, fmt.Errorf("rls: empty lookup_service in route lookup config %+v", rlsProto)
197197
}
198-
parsedTarget, err := url.Parse(lookupService)
198+
parsedTarget, err := grpcutil.ParseTarget(lookupService, resolver.GetDefaultScheme())
199199
if err != nil {
200-
// url.Parse() fails if scheme is missing. Retry with default scheme.
201-
parsedTarget, err = url.Parse(resolver.GetDefaultScheme() + ":///" + lookupService)
202-
if err != nil {
203-
return nil, fmt.Errorf("rls: invalid target URI in lookup_service %s", lookupService)
204-
}
205-
}
206-
if parsedTarget.Scheme == "" {
207-
parsedTarget.Scheme = resolver.GetDefaultScheme()
200+
return nil, fmt.Errorf("rls: invalid target URI in lookup_service %s: %v", lookupService, err)
208201
}
209202
if resolver.Get(parsedTarget.Scheme) == nil {
210203
return nil, fmt.Errorf("rls: unregistered scheme in lookup_service %s", lookupService)

clientconn.go

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"errors"
2424
"fmt"
2525
"math"
26-
"net/url"
2726
"slices"
2827
"strings"
2928
"sync"
@@ -40,6 +39,7 @@ import (
4039
"google.golang.org/grpc/internal"
4140
"google.golang.org/grpc/internal/channelz"
4241
"google.golang.org/grpc/internal/grpcsync"
42+
"google.golang.org/grpc/internal/grpcutil"
4343
"google.golang.org/grpc/internal/idle"
4444
iresolver "google.golang.org/grpc/internal/resolver"
4545
istats "google.golang.org/grpc/internal/stats"
@@ -1799,11 +1799,11 @@ func (cc *ClientConn) initParsedTargetAndResolverBuilder() error {
17991799
logger.Infof("original dial target is: %q", cc.target)
18001800

18011801
var rb resolver.Builder
1802-
parsedTarget, err := parseTarget(cc.target)
1802+
u, err := grpcutil.ParseTarget(cc.target, "")
18031803
if err == nil {
1804-
rb = cc.getResolver(parsedTarget.URL.Scheme)
1804+
rb = cc.getResolver(u.Scheme)
18051805
if rb != nil {
1806-
cc.parsedTarget = parsedTarget
1806+
cc.parsedTarget = resolver.Target{URL: *u}
18071807
cc.resolverBuilder = rb
18081808
return nil
18091809
}
@@ -1820,32 +1820,19 @@ func (cc *ClientConn) initParsedTargetAndResolverBuilder() error {
18201820
}
18211821

18221822
canonicalTarget := defScheme + ":///" + cc.target
1823-
1824-
parsedTarget, err = parseTarget(canonicalTarget)
1823+
u, err = grpcutil.ParseTarget(canonicalTarget, "")
18251824
if err != nil {
18261825
return err
18271826
}
1828-
rb = cc.getResolver(parsedTarget.URL.Scheme)
1827+
rb = cc.getResolver(u.Scheme)
18291828
if rb == nil {
1830-
return fmt.Errorf("could not get resolver for default scheme: %q", parsedTarget.URL.Scheme)
1829+
return fmt.Errorf("could not get resolver for default scheme: %q", u.Scheme)
18311830
}
1832-
cc.parsedTarget = parsedTarget
1831+
cc.parsedTarget = resolver.Target{URL: *u}
18331832
cc.resolverBuilder = rb
18341833
return nil
18351834
}
18361835

1837-
// parseTarget uses RFC 3986 semantics to parse the given target into a
1838-
// resolver.Target struct containing url. Query params are stripped from the
1839-
// endpoint.
1840-
func parseTarget(target string) (resolver.Target, error) {
1841-
u, err := url.Parse(target)
1842-
if err != nil {
1843-
return resolver.Target{}, err
1844-
}
1845-
1846-
return resolver.Target{URL: *u}, nil
1847-
}
1848-
18491836
// encodeAuthority escapes the authority string based on valid chars defined in
18501837
// https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.
18511838
func encodeAuthority(authority string) string {

internal/grpcutil/target.go

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,39 @@ import (
2525
"google.golang.org/grpc/resolver"
2626
)
2727

28-
// ValidateTargetURI validates that target is a valid RFC 3986 URI
29-
// and that a resolver is registered for its scheme.
30-
func ValidateTargetURI(target string) error {
28+
// ParseTarget parses a gRPC target string into a URL. If parsing fails,
29+
// it prepends defaultScheme and retries. If the scheme is empty after
30+
// parsing, it is set to defaultScheme. If defaultScheme is empty, no
31+
// fallback is attempted.
32+
func ParseTarget(target, defaultScheme string) (*url.URL, error) {
3133
u, err := url.Parse(target)
3234
if err != nil {
33-
return fmt.Errorf("invalid target URI %q: %w", target, err)
35+
if defaultScheme == "" {
36+
return nil, fmt.Errorf("invalid target URI %q: %w", target, err)
37+
}
38+
u, err = url.Parse(defaultScheme + ":///" + target)
39+
if err != nil {
40+
return nil, fmt.Errorf("invalid target URI %q: %w", target, err)
41+
}
3442
}
35-
3643
if u.Scheme == "" {
37-
return fmt.Errorf("target URI %q has no scheme", target)
44+
if defaultScheme == "" {
45+
return nil, fmt.Errorf("target URI %q has no scheme", target)
46+
}
47+
u.Scheme = defaultScheme
3848
}
49+
return u, nil
50+
}
3951

52+
// ValidateTargetURI validates that target is a valid RFC 3986 URI
53+
// and that a resolver is registered for its scheme.
54+
func ValidateTargetURI(target string) error {
55+
u, err := ParseTarget(target, "")
56+
if err != nil {
57+
return err
58+
}
4059
if resolver.Get(u.Scheme) == nil {
4160
return fmt.Errorf("no resolver registered for scheme %q", u.Scheme)
4261
}
43-
4462
return nil
4563
}

internal/grpcutil/target_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,86 @@ import (
2626
_ "google.golang.org/grpc/resolver/passthrough" // Register passthrough resolver
2727
)
2828

29+
func TestParseTarget(t *testing.T) {
30+
tests := []struct {
31+
name string
32+
target string
33+
defaultScheme string
34+
wantScheme string
35+
wantErr bool
36+
errContain string
37+
}{
38+
{
39+
name: "valid dns scheme",
40+
target: "dns:///example.com:443",
41+
defaultScheme: "",
42+
wantScheme: "dns",
43+
wantErr: false,
44+
},
45+
{
46+
name: "valid passthrough scheme",
47+
target: "passthrough:///localhost:8080",
48+
defaultScheme: "",
49+
wantScheme: "passthrough",
50+
wantErr: false,
51+
},
52+
{
53+
name: "missing scheme with default",
54+
target: "/path/to/socket",
55+
defaultScheme: "unix",
56+
wantScheme: "unix",
57+
wantErr: false,
58+
},
59+
{
60+
name: "missing scheme without default",
61+
target: "/path/to/socket",
62+
defaultScheme: "",
63+
wantErr: true,
64+
errContain: "has no scheme",
65+
},
66+
{
67+
name: "host:port parsed as scheme",
68+
target: "localhost:8080",
69+
defaultScheme: "dns",
70+
wantScheme: "localhost",
71+
wantErr: false,
72+
},
73+
{
74+
name: "invalid URI without default",
75+
target: "dns:///example\x00.com",
76+
defaultScheme: "",
77+
wantErr: true,
78+
errContain: "invalid target URI",
79+
},
80+
{
81+
name: "invalid URI with default still fails",
82+
target: "dns:///example\x00.com",
83+
defaultScheme: "dns",
84+
wantErr: true,
85+
errContain: "invalid target URI",
86+
},
87+
}
88+
89+
for _, tt := range tests {
90+
t.Run(tt.name, func(t *testing.T) {
91+
u, err := ParseTarget(tt.target, tt.defaultScheme)
92+
if (err != nil) != tt.wantErr {
93+
t.Errorf("ParseTarget(%q, %q) error = %v, wantErr %v", tt.target, tt.defaultScheme, err, tt.wantErr)
94+
return
95+
}
96+
if tt.wantErr {
97+
if tt.errContain != "" && !strings.Contains(err.Error(), tt.errContain) {
98+
t.Errorf("ParseTarget(%q, %q) error = %v, want error containing %q", tt.target, tt.defaultScheme, err, tt.errContain)
99+
}
100+
return
101+
}
102+
if u.Scheme != tt.wantScheme {
103+
t.Errorf("ParseTarget(%q, %q).Scheme = %q, want %q", tt.target, tt.defaultScheme, u.Scheme, tt.wantScheme)
104+
}
105+
})
106+
}
107+
}
108+
29109
func TestValidateTargetURI(t *testing.T) {
30110
tests := []struct {
31111
name string

0 commit comments

Comments
 (0)