diff --git a/balancer/rls/config.go b/balancer/rls/config.go index 9693c8ba9590..42e9b9abbbb5 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,7 +29,7 @@ import ( "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/pretty" rlspb "google.golang.org/grpc/internal/proto/grpc_lookup_v1" - "google.golang.org/grpc/resolver" + iresolver "google.golang.org/grpc/internal/resolver" "google.golang.org/grpc/serviceconfig" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/types/known/durationpb" @@ -195,19 +194,8 @@ 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) - 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) + if err := iresolver.ValidateTargetURI(lookupService); err != nil { + return nil, fmt.Errorf("rls: invalid lookup_service %q: %v", lookupService, err) } lookupServiceTimeout, err := convertDuration(rlsProto.GetLookupServiceTimeout()) diff --git a/balancer/rls/config_test.go b/balancer/rls/config_test.go index c1aff0c9cb8d..96d2312e4c13 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 lookup_service", }, { desc: "invalid lookup service timeout", diff --git a/internal/resolver/target.go b/internal/resolver/target.go new file mode 100644 index 000000000000..2496231652ad --- /dev/null +++ b/internal/resolver/target.go @@ -0,0 +1,74 @@ +/* + * + * 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/internal" + "google.golang.org/grpc/resolver" +) + +// ValidateTargetURI reports whether target is a valid gRPC dial target. It is +// intended for validating targets received via configuration (e.g. xDS +// bootstrap server_uri, RLS lookup_service) before dial time. +// +// A target is valid if: +// - it parses as an RFC 3986 authority-form URI (via net/url.Parse) whose +// scheme has a resolver builder registered in the global registry +// (resolver.Get), or +// - it does not parse as an authority-form URI (e.g. a "host:port" string +// such as "trafficdirector.googleapis.com:443", which parses as an opaque +// URI, or a string that does not parse at all), but is accepted after +// applying the default scheme, mirroring grpc.NewClient's fallback +// behavior for schemeless targets. +// +// Unlike grpc.NewClient, an authority-form URI ("scheme://...") with a scheme +// that has no registered resolver is rejected instead of falling back to the +// default scheme, so that scheme typos in configuration surface as errors. +// +// Per-channel resolvers registered via grpc.WithResolvers are not visible to +// this function. +func ValidateTargetURI(target string) error { + if target == "" { + return fmt.Errorf("resolver: target URI cannot be empty") + } + // Mirror grpc.NewClient's choice of default scheme: "dns", unless the + // user overrode it via resolver.SetDefaultScheme. + defScheme := "dns" + if internal.UserSetDefaultScheme { + defScheme = resolver.GetDefaultScheme() + } + u, err := url.Parse(target) + if err != nil || u.Opaque != "" { + // Not an authority-form URI: treat it as a host:port shorthand and + // apply the default scheme, as grpc.NewClient does. + if u, err = url.Parse(defScheme + ":///" + target); err != nil { + return fmt.Errorf("resolver: invalid target URI %q: %v", target, err) + } + } + if u.Scheme == "" { + u.Scheme = defScheme + } + if resolver.Get(u.Scheme) == nil { + return fmt.Errorf("resolver: target URI %q uses scheme %q which has no registered resolver", target, u.Scheme) + } + return nil +} diff --git a/internal/resolver/target_test.go b/internal/resolver/target_test.go new file mode 100644 index 000000000000..60fa1d598962 --- /dev/null +++ b/internal/resolver/target_test.go @@ -0,0 +1,98 @@ +/* + * + * 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 ( + "strings" + "testing" + + "google.golang.org/grpc/internal" + "google.golang.org/grpc/resolver" + + _ "google.golang.org/grpc/internal/resolver/dns" // Register the default (dns) resolver for fallback tests. +) + +// testResolverBuilder is a minimal resolver.Builder used only to register +// schemes for ValidateTargetURI tests. +type testResolverBuilder struct{ scheme string } + +func (b *testResolverBuilder) Build(resolver.Target, resolver.ClientConn, resolver.BuildOptions) (resolver.Resolver, error) { + return nil, nil +} + +func (b *testResolverBuilder) Scheme() string { return b.scheme } + +func init() { + resolver.Register(&testResolverBuilder{scheme: "iresolver-test"}) +} + +func TestValidateTargetURI(t *testing.T) { + tests := []struct { + name string + target string + wantErr bool + }{ + {name: "registered scheme with authority and endpoint", target: "iresolver-test:///endpoint", wantErr: false}, + // url.Parse canonicalizes the scheme to lowercase (RFC 3986 3.1), + // so an uppercase scheme still matches the registered lowercase one. + {name: "registered scheme uppercase input", target: "IRESOLVER-TEST:///endpoint", wantErr: false}, + // Opaque (host:port) forms fall back to the default scheme, as + // grpc.NewClient does. + {name: "host:port without scheme", target: "my-service:50051", wantErr: false}, + {name: "host:port with dotted host", target: "trafficdirector.googleapis.com:443", wantErr: false}, + {name: "ip:port without scheme", target: "127.0.0.1:443", wantErr: false}, + {name: "registered scheme opaque form", target: "iresolver-test:endpoint", wantErr: false}, + // A string that does not parse as a URI is accepted if it parses + // after the default-scheme fallback, matching grpc.NewClient. + {name: "unparseable URI accepted via fallback", target: "://bad", wantErr: false}, + // Parses with an empty scheme (not opaque), so the default scheme is + // applied directly. + {name: "absolute path without scheme", target: "/var/run/foo.sock", wantErr: false}, + // An invalid percent-escape fails to parse both as-is and after the + // default-scheme fallback. + {name: "invalid percent-escape", target: "%zz", wantErr: true}, + {name: "empty target", target: "", wantErr: true}, + // Authority-form URIs with an unregistered scheme are rejected, so + // that scheme typos in configuration surface as errors. + {name: "unregistered scheme", target: "no-such-scheme:///endpoint", wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := ValidateTargetURI(tc.target) + if (err != nil) != tc.wantErr { + t.Fatalf("ValidateTargetURI(%q) = %v, wantErr %v", tc.target, err, tc.wantErr) + } + if err != nil && !strings.Contains(err.Error(), tc.target) && tc.target != "" { + t.Errorf("ValidateTargetURI(%q) error %q does not mention target", tc.target, err) + } + }) + } +} + +func TestValidateTargetURI_UserSetDefaultScheme(t *testing.T) { + resolver.SetDefaultScheme("iresolver-test") + defer func() { + // Reset the default scheme as though it was never set by the user. + resolver.SetDefaultScheme("passthrough") + internal.UserSetDefaultScheme = false + }() + if err := ValidateTargetURI("my-service:50051"); err != nil { + t.Fatalf("ValidateTargetURI(%q) with user-set default scheme = %v, want nil", "my-service:50051", err) + } +} diff --git a/internal/xds/bootstrap/bootstrap.go b/internal/xds/bootstrap/bootstrap.go index 79fb79020cf8..44555df70d3b 100644 --- a/internal/xds/bootstrap/bootstrap.go +++ b/internal/xds/bootstrap/bootstrap.go @@ -35,6 +35,7 @@ import ( "google.golang.org/grpc/credentials/tls/certprovider" "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/envconfig" + iresolver "google.golang.org/grpc/internal/resolver" "google.golang.org/grpc/xds/bootstrap" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" @@ -399,6 +400,9 @@ func (sc *ServerConfig) UnmarshalJSON(data []byte) error { if sc.serverURI == "" { return fmt.Errorf("xds: `server_uri` field in server config cannot be empty: %s", string(data)) } + if err := iresolver.ValidateTargetURI(sc.serverURI); err != nil { + return fmt.Errorf("xds: invalid `server_uri` field in server config %s: %v", string(data), err) + } if sc.credsDialOption == nil { return fmt.Errorf("xds: `channel_creds` field in server config cannot be empty: %s", string(data)) } diff --git a/internal/xds/bootstrap/bootstrap_test.go b/internal/xds/bootstrap/bootstrap_test.go index 0dd87416a30c..506daa0eef7c 100644 --- a/internal/xds/bootstrap/bootstrap_test.go +++ b/internal/xds/bootstrap/bootstrap_test.go @@ -512,11 +512,24 @@ func (s) TestGetConfiguration_Failure(t *testing.T) { ] }] }`, + "unregisteredSchemeInServerURI": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "badscheme:///trafficdirector.googleapis.com", + "channel_creds": [{ "type": "insecure" }] + }] + }`, } cancel := setupBootstrapOverride(bootstrapFileMap) defer cancel() - for _, name := range []string{"nonExistentBootstrapFile", "badJSON", "noBalancerName", "emptyXdsServer"} { + for _, name := range []string{"nonExistentBootstrapFile", "badJSON", "noBalancerName", "emptyXdsServer", "unregisteredSchemeInServerURI"} { t.Run(name, func(t *testing.T) { testGetConfigurationWithFileNameEnv(t, name, true, nil) testGetConfigurationWithFileContentEnv(t, name, true, nil) diff --git a/internal/xds/server/filter_chain_manager_test.go b/internal/xds/server/filter_chain_manager_test.go index 6335e3582f11..a8db0da33b7d 100644 --- a/internal/xds/server/filter_chain_manager_test.go +++ b/internal/xds/server/filter_chain_manager_test.go @@ -95,7 +95,7 @@ func newFilterChainManagerForTesting(t *testing.T, lis *v3listenerpb.Listener) * bc, err := bootstrap.NewConfigFromContents([]byte(`{ "xds_servers": [ { - "server_uri": "ipv4:///127.0.0.1:443", + "server_uri": "passthrough:///127.0.0.1:443", "channel_creds": [ { "type": "insecure"