internal/resolver: add target URI parsing with resolver validation by HueCodes · Pull Request #8876 · grpc/grpc-go

HueCodes · 2026-02-02T19:08:59Z

Add resolver validation to ParseTarget in grpcutil. The function
parses a gRPC target URI string and verifies that a resolver is
registered for the parsed scheme.

Revert clientconn.go to url.Parse since it uses cc.getResolver()
for custom dial-option resolvers. Remove the redundant resolver.Get
check from rls/config.go.

RELEASE NOTES: n/a

codecov · 2026-02-02T19:12:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.07%. Comparing base (2abe1f0) to head (383f46a).
⚠️ Report is 52 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8876      +/-   ##
==========================================
- Coverage   83.17%   83.07%   -0.10%     
==========================================
  Files         414      412       -2     
  Lines       32751    32887     +136     
==========================================
+ Hits        27241    27322      +81     
- Misses       4091     4177      +86     
+ Partials     1419     1388      -31

Files with missing lines	Coverage Δ
balancer/rls/config.go	`86.46% <100.00%> (+0.85%)`	⬆️
clientconn.go	`90.77% <100.00%> (-0.03%)`	⬇️
internal/resolver/target.go	`100.00% <100.00%> (ø)`

... and 85 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

arjan-bal · 2026-02-03T08:34:51Z

Hi @HueCodes, thanks for picking up this issue.

To ensure we have a reusable API, we should update existing call sites to use the new implementation. Specifically:

ClientConn:

grpc-go/clientconn.go

Lines 1802 to 1827 in b7b1cce

    
           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 
        
           	} 
        
           } 
        
           // 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. 
        
           defScheme := cc.dopts.defaultScheme 
        
           if internal.UserSetDefaultScheme { 
        
           	defScheme = resolver.GetDefaultScheme() 
        
           } 
        
           canonicalTarget := defScheme + ":///" + cc.target 
        
           parsedTarget, err = parseTarget(canonicalTarget) 
        
           if err != nil { 
        
           	return err 
        
           }

Route Lookup Service:

grpc-go/balancer/rls/config.go

Lines 198 to 205 in b7b1cce

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

Both of these locations share common behavior:

If parsing fails, they attempt to append the default scheme and re-parse.
They return a url.URL object.

To handle this, the new function would need to accept a default scheme param and return a url.Url as the result. I think we should design the API to handle these two operations internally. This would allow us to refactor the existing code to call the new API as well. @easwars, what do you think?

HueCodes · 2026-02-03T13:06:44Z

Thanks for the response! I will work on fixing this today when I am off work.

easwars · 2026-02-03T19:01:18Z

Agree with the comments by @arjan-bal.

Another place where we would have to use the new API is here:

grpc-go/internal/xds/bootstrap/bootstrap.go

Line 399 in b7b1cce

if sc.serverURI == "" {

Currently, that code only verifies that the server_uri is not empty. But we need to ensure that it is a valid URI. I think this will cause a whole bunch of unit tests to fail because we simply pass a non-empty string for this value in a lot of tests. So, this could also be done as a follow-up if it ends up touching too many test files. Thanks.

HueCodes · 2026-02-04T01:22:13Z

Thanks for taking the time to review this. I appreciate it. I addressed the issues:

Added ParseTarget function that accepts a default scheme parameter and returns *url.URL
Updated clientconn.go and balancer/rls/config.go to use the new function
Left xds/bootstrap for a follow-up as suggested, since it may touch many test files

Let me know if there are any other changes you would like me to make.

internal/grpcutil/target_test.go

internal/grpcutil/target.go

clientconn.go

internal/grpcutil/target.go

arjan-bal · 2026-02-11T10:16:37Z

internal/grpcutil/target_test.go

+		name          string
+		target        string
+		defaultScheme string
+		wantScheme    string


In addition to the scheme, I think we should also validate the host and the path (via Target.Endpoint()) since they're the most important components for gRPC. I've left a separate comment about returning a resolver.Target instead of a URL.

Move ParseTarget to resolver package per PR grpc#8876 review feedback. Add ParseTargetWithRegistry to support custom resolver registries. Return resolver.Target instead of *url.URL for better API ergonomics. Changes: - Add resolver.ParseTarget using global resolver registry - Add resolver.ParseTargetWithRegistry accepting custom registry function - Return resolver.Target instead of *url.URL - Use %v instead of %w in error messages per review - Update balancer/rls/config.go to use resolver.ParseTarget - Simplify clientconn.initParsedTargetAndResolverBuilder - Remove internal/grpcutil/target.go and tests

resolver/resolver.go

resolver/target_test.go

arjan-bal · 2026-02-18T07:45:38Z

resolver/resolver.go

I didn't realize that adding the function here would make the ParseTarget public. The issue explicitly mentions that the API should be internal. Can you please move the function to the internal/resolver package?

grpc-go/internal/resolver/config_selector.go

Lines 19 to 20 in c1a9239

// Package resolver provides internal resolver-related functionality.

package resolver

Apologies for the back and forth.

Thank you for taking the time to review my code. I am learning a ton as this is very helpful. I should be done tonight and will push the changes then.

Fixes grpc#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

arjan-bal

Almost looks good, some minor comments.

arjan-bal · 2026-03-17T10:41:12Z

clientconn.go

 	defScheme := cc.dopts.defaultScheme
 	if internal.UserSetDefaultScheme {
 		defScheme = resolver.GetDefaultScheme()
 	}


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.

arjan-bal · 2026-03-17T10:47:49Z

internal/resolver/target_test.go

+		errContain    string
+	}{
+		{
+			name:       "known scheme resolves",


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.

arjan-bal · 2026-03-17T10:52:23Z

internal/resolver/target.go

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


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.

No strong reason, I removed it.

Simplify ParseTarget to a single conditional instead of separate hierarchical/opaque branches. Compute the default scheme in clientconn.go before calling ParseTarget so the two-pass logic lives in one place. Switch tests to black-box _test package with blank imports for resolver registration. Use underscores in subtest names and %v for error wrapping.

Restore the distinction between hierarchical URIs (scheme://...) and opaque URIs (host:port) when the scheme is unregistered. Hierarchical URIs indicate an explicit scheme choice so ParseTarget should not silently fall back to the default scheme. Opaque and empty-scheme URIs still fall back as before. Fixes the rls config_test ParseConfigErrors/unregistered_scheme case.

clientconn needs to allow fallback for unregistered hierarchical schemes (e.g. nonexist:///foo falls back to passthrough:///nonexist:///foo), while ParseTarget rejects them so that callers like RLS config validation get strict scheme checking. Restore the original two-pass approach in clientconn: first try parsing as-is, then retry with the default scheme prepended.

arjan-bal · 2026-03-24T19:11:21Z

internal/resolver/target.go

+	// Hierarchical URI with an unregistered scheme: the caller explicitly
+	// chose this scheme, so do not silently fall back.
+	if err == nil && u.Scheme != "" && u.Opaque == "" {
+		return resolver.Target{}, fmt.Errorf("no resolver registered for scheme %q in target %q", u.Scheme, target)
+	}


Special handling for opaque URLs like this isn't present in the existing code. I assume this is required because the two places where the target is parsed behave differently:

In clientconn.go, if the URL is parsed successfully but the builder for the requested scheme is not found, the default scheme is prepended, and the target is re-parsed.

In rls/config.go, if the URL is parsed successfully but the builder for the requested scheme is not found, an error is returned directly. The default scheme is only used when either URL parsing fails, or it succeeds but the scheme is empty.

If we pass localhost:8080 to the RLS server parsing logic, it will parse it as an opaque URL, look for a resolver for localhost and fail. I suspect that RLS config parsing should be consistent across all gRPC implementations, as it comes from an xDS management server whereas gRPC channel target parsing differs slightly across them.

@easwars, can you comment on whether it makes sense to unify these two usages? If so, what is the expected behavior?

arjan-bal self-assigned this Feb 3, 2026

arjan-bal self-requested a review February 3, 2026 06:27

arjan-bal added this to the 1.80 Release milestone Feb 3, 2026

arjan-bal added the Type: Feature New features or improvements in behavior label Feb 3, 2026

arjan-bal assigned easwars and unassigned arjan-bal Feb 3, 2026

easwars assigned HueCodes and unassigned easwars Feb 3, 2026

easwars added the Status: Requires Reporter Clarification label Feb 3, 2026

HueCodes force-pushed the validate-target-uri branch from ea82c57 to 9c8aa04 Compare February 4, 2026 01:11

arjan-bal assigned arjan-bal and unassigned HueCodes Feb 4, 2026

arjan-bal removed the Status: Requires Reporter Clarification label Feb 4, 2026

arjan-bal reviewed Feb 5, 2026

View reviewed changes

internal/grpcutil/target_test.go Outdated Show resolved Hide resolved

internal/grpcutil/target.go Outdated Show resolved Hide resolved

arjan-bal assigned HueCodes and unassigned arjan-bal Feb 5, 2026

HueCodes changed the title ~~grpcutil: add ValidateTargetURI function~~ grpcutil: add target URI parsing with resolver validation Feb 7, 2026

eshitachandwani assigned arjan-bal and unassigned HueCodes Feb 9, 2026

arjan-bal reviewed Feb 11, 2026

View reviewed changes

arjan-bal assigned HueCodes and unassigned arjan-bal Feb 11, 2026

eshitachandwani assigned arjan-bal and unassigned HueCodes Feb 18, 2026

arjan-bal reviewed Feb 18, 2026

View reviewed changes

arjan-bal assigned HueCodes and unassigned arjan-bal Feb 18, 2026

HueCodes force-pushed the validate-target-uri branch 2 times, most recently from 48e43ac to 3a28eea Compare February 20, 2026 05:50

HueCodes force-pushed the validate-target-uri branch from 3a28eea to 623f036 Compare February 24, 2026 06:06

mbissa modified the milestones: 1.80 Release, 1.81 Release Mar 6, 2026

arjan-bal assigned arjan-bal and unassigned HueCodes Mar 17, 2026

arjan-bal added Type: Internal Cleanup Refactors, etc and removed Type: Feature New features or improvements in behavior labels Mar 17, 2026

arjan-bal reviewed Mar 17, 2026

View reviewed changes

arjan-bal assigned HueCodes and unassigned arjan-bal Mar 17, 2026

arjan-bal changed the title ~~grpcutil: add target URI parsing with resolver validation~~ internal/resolver: add target URI parsing with resolver validation Mar 17, 2026

HueCodes added 3 commits March 21, 2026 22:56

easwars assigned arjan-bal and unassigned HueCodes Mar 24, 2026

arjan-bal reviewed Mar 24, 2026

View reviewed changes

arjan-bal assigned easwars and unassigned arjan-bal Mar 24, 2026

	// Package resolver provides internal resolver-related functionality.
	package resolver

Conversation

HueCodes commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

arjan-bal commented Feb 3, 2026

Uh oh!

HueCodes commented Feb 3, 2026

Uh oh!

easwars commented Feb 3, 2026

Uh oh!

HueCodes commented Feb 4, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arjan-bal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HueCodes commented Feb 2, 2026 •

edited

Loading

codecov bot commented Feb 2, 2026 •

edited

Loading