From 3379b532f30edc4c6baf9c214e95fd8fcfbe0bd5 Mon Sep 17 00:00:00 2001 From: Hugh Date: Mon, 2 Feb 2026 11:06:48 -0800 Subject: [PATCH 1/4] grpcutil: add ValidateTargetURI function Fixes #8747 Add an internal API to validate gRPC target URI strings. The function checks that the target parses as a valid RFC 3986 URI and that a resolver is registered for its scheme. RELEASE NOTES: n/a --- internal/grpcutil/target.go | 45 +++++++++++++++++ internal/grpcutil/target_test.go | 86 ++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 internal/grpcutil/target.go create mode 100644 internal/grpcutil/target_test.go diff --git a/internal/grpcutil/target.go b/internal/grpcutil/target.go new file mode 100644 index 000000000000..60fe22f0d94d --- /dev/null +++ b/internal/grpcutil/target.go @@ -0,0 +1,45 @@ +/* + * + * 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" +) + +// 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 := url.Parse(target) + if err != nil { + return fmt.Errorf("invalid target URI %q: %w", target, err) + } + + if u.Scheme == "" { + return fmt.Errorf("target URI %q has no scheme", target) + } + + 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..d713a8ac92ce --- /dev/null +++ b/internal/grpcutil/target_test.go @@ -0,0 +1,86 @@ +/* + * + * 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 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) + } + } + }) + } +} From 3a28eeaa6fcc148cd84ab856cf7506a065a6f899 Mon Sep 17 00:00:00 2001 From: Hugh Date: Thu, 19 Feb 2026 21:44:00 -0800 Subject: [PATCH 2/4] internal/resolver: add ParseTarget for target URI validation Fixes #8747 Add ParseTarget to internal/resolver. The function parses a gRPC target string into a resolver.Target and verifies that a resolver is registered for the parsed scheme using a caller-supplied lookup function. Scheme lookup rules: - Registered scheme (hierarchical or opaque form): accepted immediately. - Unregistered scheme in hierarchical URI (scheme://...): rejected; the caller chose this scheme explicitly, so no silent fallback occurs. - Opaque URI (e.g. host:port) or empty-scheme URI with an unregistered scheme: retried by prepending defaultScheme + ":///" if provided. Accepting a builder func instead of calling resolver.Get directly lets clientconn.go pass cc.getResolver, which also checks resolvers registered via dial options. Update clientconn.go and balancer/rls/config.go to use ParseTarget, removing the duplicate parsing and resolver-lookup logic from both. RELEASE NOTES: n/a --- balancer/rls/config.go | 16 +-- balancer/rls/config_test.go | 2 +- clientconn.go | 42 ++----- internal/resolver/target.go | 72 +++++++++++ internal/resolver/target_test.go | 201 +++++++++++++++++++++++++++++++ 5 files changed, 287 insertions(+), 46 deletions(-) create mode 100644 internal/resolver/target.go create mode 100644 internal/resolver/target_test.go diff --git a/balancer/rls/config.go b/balancer/rls/config.go index 9693c8ba9590..8f9a899eeae4 100644 --- a/balancer/rls/config.go +++ b/balancer/rls/config.go @@ -22,7 +22,6 @@ import ( "bytes" "encoding/json" "fmt" - "net/url" "time" "google.golang.org/grpc/balancer" @@ -30,6 +29,7 @@ import ( "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/pretty" rlspb "google.golang.org/grpc/internal/proto/grpc_lookup_v1" + iresolver "google.golang.org/grpc/internal/resolver" "google.golang.org/grpc/resolver" "google.golang.org/grpc/serviceconfig" "google.golang.org/protobuf/encoding/protojson" @@ -195,19 +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) + _, err = iresolver.ParseTarget(lookupService, resolver.GetDefaultScheme(), resolver.Get) 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() - } - if resolver.Get(parsedTarget.Scheme) == nil { - return nil, fmt.Errorf("rls: unregistered scheme in lookup_service %s", lookupService) + return nil, fmt.Errorf("rls: invalid target URI in lookup_service %s: %v", lookupService, err) } lookupServiceTimeout, err := convertDuration(rlsProto.GetLookupServiceTimeout()) diff --git a/balancer/rls/config_test.go b/balancer/rls/config_test.go index c1aff0c9cb8d..650d5e350a99 100644 --- a/balancer/rls/config_test.go +++ b/balancer/rls/config_test.go @@ -263,7 +263,7 @@ func (s) TestParseConfigErrors(t *testing.T) { "lookupService": "badScheme:///target" } }`), - wantErr: "rls: unregistered scheme in lookup_service", + wantErr: "rls: invalid target URI in lookup_service", }, { desc: "invalid lookup service timeout", diff --git a/clientconn.go b/clientconn.go index 5dec2dacc0ba..d0b85e1bf40d 100644 --- a/clientconn.go +++ b/clientconn.go @@ -23,7 +23,6 @@ import ( "errors" "fmt" "math" - "net/url" "slices" "strings" "sync" @@ -1798,54 +1797,33 @@ func (cc *ClientConn) connectionError() error { func (cc *ClientConn) initParsedTargetAndResolverBuilder() error { logger.Infof("original dial target is: %q", cc.target) - var rb resolver.Builder - parsedTarget, err := parseTarget(cc.target) - if err == nil { - rb = cc.getResolver(parsedTarget.URL.Scheme) - if rb != nil { - cc.parsedTarget = parsedTarget - cc.resolverBuilder = rb - return nil - } + // Try the target as given first. cc.getResolver checks both globally + // registered resolvers and any resolver registered via dial options. + if parsedTarget, err := iresolver.ParseTarget(cc.target, "", cc.getResolver); err == nil { + cc.parsedTarget = parsedTarget + cc.resolverBuilder = cc.getResolver(parsedTarget.URL.Scheme) + return 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 a custom dialer is specified in which case, we should - // always use passthrough scheme. For either case, we need to respect any overridden - // global defaults set by the user. + // always use passthrough scheme. For either case, we need to respect any + // overridden global defaults set by the user. defScheme := cc.dopts.defaultScheme if internal.UserSetDefaultScheme { defScheme = resolver.GetDefaultScheme() } - canonicalTarget := defScheme + ":///" + cc.target - - parsedTarget, err = parseTarget(canonicalTarget) + parsedTarget, err := iresolver.ParseTarget(defScheme+":///"+cc.target, "", cc.getResolver) if err != nil { return err } - rb = cc.getResolver(parsedTarget.URL.Scheme) - if rb == nil { - return fmt.Errorf("could not get resolver for default scheme: %q", parsedTarget.URL.Scheme) - } cc.parsedTarget = parsedTarget - cc.resolverBuilder = rb + cc.resolverBuilder = cc.getResolver(parsedTarget.URL.Scheme) 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/resolver/target.go b/internal/resolver/target.go new file mode 100644 index 000000000000..d8382b18b3a7 --- /dev/null +++ b/internal/resolver/target.go @@ -0,0 +1,72 @@ +/* + * + * Copyright 2026 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 resolver + +import ( + "fmt" + "net/url" + + "google.golang.org/grpc/resolver" +) + +// ParseTarget parses a gRPC target string into a resolver.Target, verifying +// that a resolver is registered for the parsed scheme using builder. +// +// Hierarchical URIs (scheme://authority/path) with a non-empty scheme are +// validated directly: if builder returns nil for the scheme an error is +// returned immediately with no fallback. +// +// For opaque URIs (e.g. "host:port" where url.URL.Opaque is non-empty), +// empty-scheme URIs, and parse failures, ParseTarget retries by prepending +// defaultScheme + ":///" if defaultScheme is non-empty. +// +// builder is a function that returns the resolver.Builder for a given scheme, +// or nil if no resolver is registered. Pass resolver.Get to use the global +// resolver registry, or a custom lookup function (e.g. cc.getResolver) to +// also consider resolvers registered via dial options. +func ParseTarget(target, defaultScheme string, builder func(string) resolver.Builder) (resolver.Target, error) { + u, err := url.Parse(target) + if err == nil && u.Scheme != "" { + if builder(u.Scheme) != nil { + // Recognised scheme (hierarchical or opaque form) — use as-is. + return resolver.Target{URL: *u}, nil + } + if u.Opaque == "" { + // Unregistered scheme in hierarchical URI form (scheme://...): the + // caller explicitly chose this scheme; do not silently fall back. + return resolver.Target{}, fmt.Errorf("no resolver registered for scheme %q in target %q", u.Scheme, target) + } + // Opaque URI (e.g. "host:port") with unregistered scheme: treat the + // same as an empty-scheme URI and fall through to the retry below. + } + // Parse error, empty scheme, or opaque URI with unregistered scheme: + // retry by prepending defaultScheme if one is provided. + if defaultScheme != "" { + if u2, err2 := url.Parse(defaultScheme + ":///" + target); err2 == nil && builder(u2.Scheme) != nil { + return resolver.Target{URL: *u2}, nil + } + } + if err != nil { + return resolver.Target{}, fmt.Errorf("invalid target URI %q: %v", target, err) + } + if u.Scheme == "" { + return resolver.Target{}, fmt.Errorf("target URI %q has no scheme", target) + } + return resolver.Target{}, fmt.Errorf("no resolver registered for scheme %q in target %q", u.Scheme, target) +} diff --git a/internal/resolver/target_test.go b/internal/resolver/target_test.go new file mode 100644 index 000000000000..0ad63c471207 --- /dev/null +++ b/internal/resolver/target_test.go @@ -0,0 +1,201 @@ +/* + * + * Copyright 2026 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 resolver_test + +import ( + "strings" + "testing" + + iresolver "google.golang.org/grpc/internal/resolver" + _ "google.golang.org/grpc/internal/resolver/passthrough" // Register passthrough resolver. + "google.golang.org/grpc/resolver" + _ "google.golang.org/grpc/resolver/dns" // Register dns 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", + wantScheme: "dns", + }, + { + name: "valid passthrough scheme", + target: "passthrough:///localhost:8080", + wantScheme: "passthrough", + }, + { + name: "valid dns scheme with default", + target: "dns:///example.com:443", + defaultScheme: "dns", + wantScheme: "dns", + }, + { + name: "missing scheme falls back to default", + target: "/path/to/socket", + defaultScheme: "passthrough", + wantScheme: "passthrough", + }, + { + name: "missing scheme without default", + target: "/path/to/socket", + wantErr: true, + errContain: "has no scheme", + }, + { + name: "host:port retries with default scheme", + target: "localhost:8080", + defaultScheme: "passthrough", + wantScheme: "passthrough", + }, + { + name: "host:port without default", + target: "localhost:8080", + wantErr: true, + errContain: "no resolver registered for scheme", + }, + { + name: "unregistered scheme", + target: "unknown:///example.com:443", + wantErr: true, + errContain: "no resolver registered for scheme", + }, + { + // Explicit hierarchical URI with unknown scheme is rejected even when + // a default is provided. Only opaque URIs (host:port) fall back. + name: "unregistered explicit scheme is rejected", + target: "unknown:///foo", + defaultScheme: "passthrough", + wantErr: true, + errContain: "no resolver registered for scheme", + }, + { + name: "invalid URI", + target: "dns:///example\x00.com", + wantErr: true, + errContain: "invalid target URI", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := iresolver.ParseTarget(tt.target, tt.defaultScheme, resolver.Get) + 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 = %q, want it to contain %q", tt.target, tt.defaultScheme, err, tt.errContain) + } + return + } + if got.URL.Scheme != tt.wantScheme { + t.Errorf("ParseTarget(%q, %q).URL.Scheme = %q, want %q", tt.target, tt.defaultScheme, got.URL.Scheme, tt.wantScheme) + } + }) + } +} + +func TestParseTargetWithCustomBuilder(t *testing.T) { + // A registry that only recognises "passthrough". This mirrors the + // cc.getResolver pattern in ClientConn, which may include resolvers + // registered via dial options that are invisible to resolver.Get. + passthroughOnly := func(scheme string) resolver.Builder { + if scheme == "passthrough" { + return resolver.Get("passthrough") + } + return nil + } + + tests := []struct { + name string + target string + defaultScheme string + wantScheme string + wantErr bool + errContain string + }{ + { + name: "known scheme resolves", + target: "passthrough:///service:8080", + wantScheme: "passthrough", + }, + { + name: "dns not in custom registry", + target: "dns:///example.com:443", + wantErr: true, + errContain: "no resolver registered for scheme", + }, + { + // Explicit hierarchical URI: dns is not in the custom registry, and + // unlike an opaque "host:port" URI, explicit schemes are not retried. + name: "unregistered explicit scheme rejected even with default", + target: "dns:///example.com:443", + defaultScheme: "passthrough", + wantErr: true, + errContain: "no resolver registered for scheme", + }, + { + // Opaque URI (host:port form) falls back to the default scheme. + name: "host:port falls back to custom default", + target: "service:8080", + defaultScheme: "passthrough", + wantScheme: "passthrough", + }, + { + name: "missing scheme without default", + target: "/path", + wantErr: true, + errContain: "has no scheme", + }, + { + name: "missing scheme uses default", + target: "/path", + defaultScheme: "passthrough", + wantScheme: "passthrough", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := iresolver.ParseTarget(tt.target, tt.defaultScheme, passthroughOnly) + 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 = %q, want it to contain %q", tt.target, tt.defaultScheme, err, tt.errContain) + } + return + } + if got.URL.Scheme != tt.wantScheme { + t.Errorf("ParseTarget(%q, %q).URL.Scheme = %q, want %q", tt.target, tt.defaultScheme, got.URL.Scheme, tt.wantScheme) + } + }) + } +} From c2fdf5fd04322fd1be443e6a4e40614b92d965c4 Mon Sep 17 00:00:00 2001 From: Hugh Date: Mon, 30 Mar 2026 13:05:06 -0700 Subject: [PATCH 3/4] internal/resolver: simplify ParseTarget fallback logic Remove the opaque vs hierarchical URI distinction in ParseTarget. The function now always falls back to the default scheme when the parsed scheme is unregistered, matching existing clientconn behavior. Consolidate clientconn.go to a single ParseTarget call by computing the default scheme upfront and passing it as the defaultScheme parameter. Update RLS config test to reflect unified fallback behavior, where unregistered schemes fall back to the default scheme instead of returning an error. RELEASE NOTES: n/a --- balancer/rls/config_test.go | 13 ------------ clientconn.go | 19 +++++------------ internal/resolver/target.go | 34 ++++++++++-------------------- internal/resolver/target_test.go | 36 +++++++++++++------------------- 4 files changed, 31 insertions(+), 71 deletions(-) diff --git a/balancer/rls/config_test.go b/balancer/rls/config_test.go index 650d5e350a99..da418d4e295c 100644 --- a/balancer/rls/config_test.go +++ b/balancer/rls/config_test.go @@ -252,19 +252,6 @@ func (s) TestParseConfigErrors(t *testing.T) { }`), wantErr: "rls: empty lookup_service in route lookup config", }, - { - desc: "unregistered scheme in lookup service URI", - input: []byte(`{ - "routeLookupConfig": { - "grpcKeybuilders": [{ - "names": [{"service": "service", "method": "method"}], - "headers": [{"key": "k1", "names": ["v1"]}] - }], - "lookupService": "badScheme:///target" - } - }`), - wantErr: "rls: invalid target URI in lookup_service", - }, { desc: "invalid lookup service timeout", input: []byte(`{ diff --git a/clientconn.go b/clientconn.go index d0b85e1bf40d..fdc790d2e7c7 100644 --- a/clientconn.go +++ b/clientconn.go @@ -1797,25 +1797,16 @@ func (cc *ClientConn) connectionError() error { func (cc *ClientConn) initParsedTargetAndResolverBuilder() error { logger.Infof("original dial target is: %q", cc.target) - // Try the target as given first. cc.getResolver checks both globally - // registered resolvers and any resolver registered via dial options. - if parsedTarget, err := iresolver.ParseTarget(cc.target, "", cc.getResolver); err == nil { - cc.parsedTarget = parsedTarget - cc.resolverBuilder = cc.getResolver(parsedTarget.URL.Scheme) - return 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 a custom dialer is specified in which case, we should - // always use passthrough scheme. For either case, we need to respect any - // overridden global defaults set by the user. + // Compute the fallback scheme up front so ParseTarget can handle + // the retry internally in a single call. When a custom dialer is + // specified we use passthrough; otherwise respect any global default + // the user may have overridden. defScheme := cc.dopts.defaultScheme if internal.UserSetDefaultScheme { defScheme = resolver.GetDefaultScheme() } - parsedTarget, err := iresolver.ParseTarget(defScheme+":///"+cc.target, "", cc.getResolver) + parsedTarget, err := iresolver.ParseTarget(cc.target, defScheme, cc.getResolver) if err != nil { return err } diff --git a/internal/resolver/target.go b/internal/resolver/target.go index d8382b18b3a7..aa507ad06f50 100644 --- a/internal/resolver/target.go +++ b/internal/resolver/target.go @@ -28,13 +28,10 @@ import ( // ParseTarget parses a gRPC target string into a resolver.Target, verifying // that a resolver is registered for the parsed scheme using builder. // -// Hierarchical URIs (scheme://authority/path) with a non-empty scheme are -// validated directly: if builder returns nil for the scheme an error is -// returned immediately with no fallback. -// -// For opaque URIs (e.g. "host:port" where url.URL.Opaque is non-empty), -// empty-scheme URIs, and parse failures, ParseTarget retries by prepending -// defaultScheme + ":///" if defaultScheme is non-empty. +// If the target parses successfully and builder recognises the scheme, the +// parsed target is returned directly. When the scheme is unregistered, empty, +// or parsing fails, ParseTarget retries by prepending defaultScheme + ":///" +// if defaultScheme is non-empty. // // builder is a function that returns the resolver.Builder for a given scheme, // or nil if no resolver is registered. Pass resolver.Get to use the global @@ -42,23 +39,14 @@ import ( // also consider resolvers registered via dial options. func ParseTarget(target, defaultScheme string, builder func(string) resolver.Builder) (resolver.Target, error) { u, err := url.Parse(target) - if err == nil && u.Scheme != "" { - if builder(u.Scheme) != nil { - // Recognised scheme (hierarchical or opaque form) — use as-is. - return resolver.Target{URL: *u}, nil - } - if u.Opaque == "" { - // Unregistered scheme in hierarchical URI form (scheme://...): the - // caller explicitly chose this scheme; do not silently fall back. - return resolver.Target{}, fmt.Errorf("no resolver registered for scheme %q in target %q", u.Scheme, target) - } - // Opaque URI (e.g. "host:port") with unregistered scheme: treat the - // same as an empty-scheme URI and fall through to the retry below. + if err == nil && u.Scheme != "" && builder(u.Scheme) != nil { + return resolver.Target{URL: *u}, nil } - // Parse error, empty scheme, or opaque URI with unregistered scheme: - // retry by prepending defaultScheme if one is provided. - if defaultScheme != "" { - if u2, err2 := url.Parse(defaultScheme + ":///" + target); err2 == nil && builder(u2.Scheme) != nil { + // Parse error, empty scheme, or unregistered scheme: retry with the + // default scheme if one is provided. + if defaultScheme != "" && builder(defaultScheme) != nil { + canonicalTarget := defaultScheme + ":///" + target + if u2, err2 := url.Parse(canonicalTarget); err2 == nil { return resolver.Target{URL: *u2}, nil } } diff --git a/internal/resolver/target_test.go b/internal/resolver/target_test.go index 0ad63c471207..d0fe69c6c7b9 100644 --- a/internal/resolver/target_test.go +++ b/internal/resolver/target_test.go @@ -38,29 +38,29 @@ func TestParseTarget(t *testing.T) { errContain string }{ { - name: "valid dns scheme", + name: "valid_dns_scheme", target: "dns:///example.com:443", wantScheme: "dns", }, { - name: "valid passthrough scheme", + name: "valid_passthrough_scheme", target: "passthrough:///localhost:8080", wantScheme: "passthrough", }, { - name: "valid dns scheme with default", + name: "valid_dns_scheme_with_default", target: "dns:///example.com:443", defaultScheme: "dns", wantScheme: "dns", }, { - name: "missing scheme falls back to default", + name: "missing_scheme_falls_back_to_default", target: "/path/to/socket", defaultScheme: "passthrough", wantScheme: "passthrough", }, { - name: "missing scheme without default", + name: "missing_scheme_without_default", target: "/path/to/socket", wantErr: true, errContain: "has no scheme", @@ -78,22 +78,19 @@ func TestParseTarget(t *testing.T) { errContain: "no resolver registered for scheme", }, { - name: "unregistered scheme", + name: "unregistered_scheme", target: "unknown:///example.com:443", wantErr: true, errContain: "no resolver registered for scheme", }, { - // Explicit hierarchical URI with unknown scheme is rejected even when - // a default is provided. Only opaque URIs (host:port) fall back. - name: "unregistered explicit scheme is rejected", + name: "unregistered_scheme_falls_back_to_default", target: "unknown:///foo", defaultScheme: "passthrough", - wantErr: true, - errContain: "no resolver registered for scheme", + wantScheme: "passthrough", }, { - name: "invalid URI", + name: "invalid_URI", target: "dns:///example\x00.com", wantErr: true, errContain: "invalid target URI", @@ -140,24 +137,21 @@ func TestParseTargetWithCustomBuilder(t *testing.T) { errContain string }{ { - name: "known scheme resolves", + name: "known_scheme_resolves", target: "passthrough:///service:8080", wantScheme: "passthrough", }, { - name: "dns not in custom registry", + name: "dns_not_in_custom_registry", target: "dns:///example.com:443", wantErr: true, errContain: "no resolver registered for scheme", }, { - // Explicit hierarchical URI: dns is not in the custom registry, and - // unlike an opaque "host:port" URI, explicit schemes are not retried. - name: "unregistered explicit scheme rejected even with default", + name: "unregistered_scheme_falls_back_to_custom_default", target: "dns:///example.com:443", defaultScheme: "passthrough", - wantErr: true, - errContain: "no resolver registered for scheme", + wantScheme: "passthrough", }, { // Opaque URI (host:port form) falls back to the default scheme. @@ -167,13 +161,13 @@ func TestParseTargetWithCustomBuilder(t *testing.T) { wantScheme: "passthrough", }, { - name: "missing scheme without default", + name: "missing_scheme_without_default", target: "/path", wantErr: true, errContain: "has no scheme", }, { - name: "missing scheme uses default", + name: "missing_scheme_uses_default", target: "/path", defaultScheme: "passthrough", wantScheme: "passthrough", From 89ed8d14231520e45ad936de90add8d10e3ca460 Mon Sep 17 00:00:00 2001 From: Hugh Date: Mon, 30 Mar 2026 15:05:13 -0700 Subject: [PATCH 4/4] internal/resolver: remove unused grpcutil/target.go, fix test names Remove internal/grpcutil/target.go and its test. The ValidateTargetURI function was unused outside its own test and introduced a transitive dependency from encoding/ through internal/grpcutil/ to resolver/ and credentials/, pulling crypto/tls into lightweight packages. Replace spaces with underscores in subtest names per review feedback. --- internal/grpcutil/target.go | 45 ----------------- internal/grpcutil/target_test.go | 86 -------------------------------- internal/resolver/target_test.go | 6 +-- 3 files changed, 3 insertions(+), 134 deletions(-) delete mode 100644 internal/grpcutil/target.go delete mode 100644 internal/grpcutil/target_test.go diff --git a/internal/grpcutil/target.go b/internal/grpcutil/target.go deleted file mode 100644 index 60fe22f0d94d..000000000000 --- a/internal/grpcutil/target.go +++ /dev/null @@ -1,45 +0,0 @@ -/* - * - * 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" -) - -// 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 := url.Parse(target) - if err != nil { - return fmt.Errorf("invalid target URI %q: %w", target, err) - } - - if u.Scheme == "" { - return fmt.Errorf("target URI %q has no scheme", target) - } - - 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 deleted file mode 100644 index d713a8ac92ce..000000000000 --- a/internal/grpcutil/target_test.go +++ /dev/null @@ -1,86 +0,0 @@ -/* - * - * 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 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) - } - } - }) - } -} diff --git a/internal/resolver/target_test.go b/internal/resolver/target_test.go index d0fe69c6c7b9..2f02e60c2782 100644 --- a/internal/resolver/target_test.go +++ b/internal/resolver/target_test.go @@ -66,13 +66,13 @@ func TestParseTarget(t *testing.T) { errContain: "has no scheme", }, { - name: "host:port retries with default scheme", + name: "host:port_retries_with_default_scheme", target: "localhost:8080", defaultScheme: "passthrough", wantScheme: "passthrough", }, { - name: "host:port without default", + name: "host:port_without_default", target: "localhost:8080", wantErr: true, errContain: "no resolver registered for scheme", @@ -155,7 +155,7 @@ func TestParseTargetWithCustomBuilder(t *testing.T) { }, { // Opaque URI (host:port form) falls back to the default scheme. - name: "host:port falls back to custom default", + name: "host:port_falls_back_to_custom_default", target: "service:8080", defaultScheme: "passthrough", wantScheme: "passthrough",