Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions balancer/rls/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ 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/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"
Expand Down Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion balancer/rls/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
42 changes: 10 additions & 32 deletions clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"errors"
"fmt"
"math"
"net/url"
"slices"
"strings"
"sync"
Expand Down Expand Up @@ -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()
}
Comment on lines 1812 to 1815
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move the computation of the default scheme higher up and pass the default scheme to the first call to iresolver.ParseTarget? That should allow us to get rid of the second ParseTarget below.


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 {
Expand Down
72 changes: 72 additions & 0 deletions internal/resolver/target.go
Original file line number Diff line number Diff line change
@@ -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)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this check isn't present in the current implementation. Is there a strong reason to add this? If no, I would suggest maintaining the existing behaviour.

Copy link
Author

Choose a reason for hiding this comment

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

No strong reason, I removed it.

// 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)
}
201 changes: 201 additions & 0 deletions internal/resolver/target_test.go
Original file line number Diff line number Diff line change
@@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Could you replace spaces with underscores (_) in the subtest names? The Go test harness automatically rewrites spaces, which makes it difficult to search the logs or run specific subtests via the test filter (-run flag) when debugging.

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)
}
})
}
}
Loading