diff --git a/balancer/rls/config.go b/balancer/rls/config.go index 9693c8ba9590..aa7ca5b6f355 100644 --- a/balancer/rls/config.go +++ b/balancer/rls/config.go @@ -22,12 +22,12 @@ import ( "bytes" "encoding/json" "fmt" - "net/url" "time" "google.golang.org/grpc/balancer" "google.golang.org/grpc/balancer/rls/internal/keys" "google.golang.org/grpc/internal" + "google.golang.org/grpc/internal/grpcutil" "google.golang.org/grpc/internal/pretty" rlspb "google.golang.org/grpc/internal/proto/grpc_lookup_v1" "google.golang.org/grpc/resolver" @@ -195,16 +195,9 @@ func parseRLSProto(rlsProto *rlspb.RouteLookupConfig) (*lbConfig, error) { if lookupService == "" { return nil, fmt.Errorf("rls: empty lookup_service in route lookup config %+v", rlsProto) } - parsedTarget, err := url.Parse(lookupService) + parsedTarget, err := grpcutil.ParseTarget(lookupService, resolver.GetDefaultScheme()) if err != nil { - // url.Parse() fails if scheme is missing. Retry with default scheme. - parsedTarget, err = url.Parse(resolver.GetDefaultScheme() + ":///" + lookupService) - if err != nil { - return nil, fmt.Errorf("rls: invalid target URI in lookup_service %s", lookupService) - } - } - if parsedTarget.Scheme == "" { - parsedTarget.Scheme = resolver.GetDefaultScheme() + return nil, fmt.Errorf("rls: invalid target URI in lookup_service %s: %v", lookupService, err) } if resolver.Get(parsedTarget.Scheme) == nil { return nil, fmt.Errorf("rls: unregistered scheme in lookup_service %s", lookupService) diff --git a/clientconn.go b/clientconn.go index 5dec2dacc0ba..4d50dcd62df1 100644 --- a/clientconn.go +++ b/clientconn.go @@ -23,7 +23,6 @@ import ( "errors" "fmt" "math" - "net/url" "slices" "strings" "sync" @@ -40,6 +39,7 @@ import ( "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/channelz" "google.golang.org/grpc/internal/grpcsync" + "google.golang.org/grpc/internal/grpcutil" "google.golang.org/grpc/internal/idle" iresolver "google.golang.org/grpc/internal/resolver" istats "google.golang.org/grpc/internal/stats" @@ -1799,11 +1799,11 @@ func (cc *ClientConn) initParsedTargetAndResolverBuilder() error { logger.Infof("original dial target is: %q", cc.target) var rb resolver.Builder - parsedTarget, err := parseTarget(cc.target) + u, err := grpcutil.ParseTarget(cc.target, "") if err == nil { - rb = cc.getResolver(parsedTarget.URL.Scheme) + rb = cc.getResolver(u.Scheme) if rb != nil { - cc.parsedTarget = parsedTarget + cc.parsedTarget = resolver.Target{URL: *u} cc.resolverBuilder = rb return nil } @@ -1820,32 +1820,19 @@ func (cc *ClientConn) initParsedTargetAndResolverBuilder() error { } canonicalTarget := defScheme + ":///" + cc.target - - parsedTarget, err = parseTarget(canonicalTarget) + u, err = grpcutil.ParseTarget(canonicalTarget, "") if err != nil { return err } - rb = cc.getResolver(parsedTarget.URL.Scheme) + rb = cc.getResolver(u.Scheme) if rb == nil { - return fmt.Errorf("could not get resolver for default scheme: %q", parsedTarget.URL.Scheme) + return fmt.Errorf("could not get resolver for default scheme: %q", u.Scheme) } - cc.parsedTarget = parsedTarget + cc.parsedTarget = resolver.Target{URL: *u} cc.resolverBuilder = rb return nil } -// parseTarget uses RFC 3986 semantics to parse the given target into a -// resolver.Target struct containing url. 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 - } - - return resolver.Target{URL: *u}, nil -} - // encodeAuthority escapes the authority string based on valid chars defined in // https://datatracker.ietf.org/doc/html/rfc3986#section-3.2. func encodeAuthority(authority string) string { diff --git a/internal/grpcutil/target.go b/internal/grpcutil/target.go new file mode 100644 index 000000000000..3ac7d07c4e8e --- /dev/null +++ b/internal/grpcutil/target.go @@ -0,0 +1,63 @@ +/* + * + * Copyright 2025 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package grpcutil + +import ( + "fmt" + "net/url" + + "google.golang.org/grpc/resolver" +) + +// ParseTarget parses a gRPC target string into a URL. If parsing fails, +// it prepends defaultScheme and retries. If the scheme is empty after +// parsing, it is set to defaultScheme. If defaultScheme is empty, no +// fallback is attempted. +func ParseTarget(target, defaultScheme string) (*url.URL, error) { + u, err := url.Parse(target) + if err != nil { + if defaultScheme == "" { + return nil, fmt.Errorf("invalid target URI %q: %w", target, err) + } + u, err = url.Parse(defaultScheme + ":///" + target) + if err != nil { + return nil, fmt.Errorf("invalid target URI %q: %w", target, err) + } + } + if u.Scheme == "" { + if defaultScheme == "" { + return nil, fmt.Errorf("target URI %q has no scheme", target) + } + u.Scheme = defaultScheme + } + return u, nil +} + +// ValidateTargetURI validates that target is a valid RFC 3986 URI +// and that a resolver is registered for its scheme. +func ValidateTargetURI(target string) error { + u, err := ParseTarget(target, "") + if err != nil { + return err + } + if resolver.Get(u.Scheme) == nil { + return fmt.Errorf("no resolver registered for scheme %q", u.Scheme) + } + return nil +} diff --git a/internal/grpcutil/target_test.go b/internal/grpcutil/target_test.go new file mode 100644 index 000000000000..8910b828741d --- /dev/null +++ b/internal/grpcutil/target_test.go @@ -0,0 +1,166 @@ +/* + * + * Copyright 2025 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package grpcutil + +import ( + "strings" + "testing" + + _ "google.golang.org/grpc/resolver/dns" // Register dns resolver + _ "google.golang.org/grpc/resolver/passthrough" // Register passthrough resolver +) + +func TestParseTarget(t *testing.T) { + tests := []struct { + name string + target string + defaultScheme string + wantScheme string + wantErr bool + errContain string + }{ + { + name: "valid dns scheme", + target: "dns:///example.com:443", + defaultScheme: "", + wantScheme: "dns", + wantErr: false, + }, + { + name: "valid passthrough scheme", + target: "passthrough:///localhost:8080", + defaultScheme: "", + wantScheme: "passthrough", + wantErr: false, + }, + { + name: "missing scheme with default", + target: "/path/to/socket", + defaultScheme: "unix", + wantScheme: "unix", + wantErr: false, + }, + { + name: "missing scheme without default", + target: "/path/to/socket", + defaultScheme: "", + wantErr: true, + errContain: "has no scheme", + }, + { + name: "host:port parsed as scheme", + target: "localhost:8080", + defaultScheme: "dns", + wantScheme: "localhost", + wantErr: false, + }, + { + name: "invalid URI without default", + target: "dns:///example\x00.com", + defaultScheme: "", + wantErr: true, + errContain: "invalid target URI", + }, + { + name: "invalid URI with default still fails", + target: "dns:///example\x00.com", + defaultScheme: "dns", + wantErr: true, + errContain: "invalid target URI", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + u, err := ParseTarget(tt.target, tt.defaultScheme) + if (err != nil) != tt.wantErr { + t.Errorf("ParseTarget(%q, %q) error = %v, wantErr %v", tt.target, tt.defaultScheme, err, tt.wantErr) + return + } + if tt.wantErr { + if tt.errContain != "" && !strings.Contains(err.Error(), tt.errContain) { + t.Errorf("ParseTarget(%q, %q) error = %v, want error containing %q", tt.target, tt.defaultScheme, err, tt.errContain) + } + return + } + if u.Scheme != tt.wantScheme { + t.Errorf("ParseTarget(%q, %q).Scheme = %q, want %q", tt.target, tt.defaultScheme, u.Scheme, tt.wantScheme) + } + }) + } +} + +func TestValidateTargetURI(t *testing.T) { + tests := []struct { + name string + target string + wantErr bool + errContain string + }{ + { + name: "valid dns scheme", + target: "dns:///example.com:443", + wantErr: false, + }, + { + name: "valid passthrough scheme", + target: "passthrough:///localhost:8080", + wantErr: false, + }, + { + name: "missing scheme", + target: "/path/to/socket", + wantErr: true, + errContain: "has no scheme", + }, + { + name: "host:port parsed as scheme", + target: "example.com:443", + wantErr: true, + errContain: "no resolver registered for scheme", + }, + { + name: "unregistered scheme", + target: "unknown:///example.com:443", + wantErr: true, + errContain: "no resolver registered for scheme", + }, + { + name: "invalid URI with control character", + target: "dns:///example\x00.com", + wantErr: true, + errContain: "invalid target URI", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTargetURI(tt.target) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateTargetURI(%q) error = %v, wantErr %v", tt.target, err, tt.wantErr) + return + } + if tt.wantErr && tt.errContain != "" { + if !strings.Contains(err.Error(), tt.errContain) { + t.Errorf("ValidateTargetURI(%q) error = %v, want error containing %q", tt.target, err, tt.errContain) + } + } + }) + } +}