From 5bca799a2962ac3dd46253c4b9ee4b0d79d3717c Mon Sep 17 00:00:00 2001 From: Arvind Bright Date: Tue, 1 Nov 2022 17:29:39 -0700 Subject: [PATCH 1/3] xds/google-c2p: validate url for no authorities --- xds/googledirectpath/googlec2p.go | 4 ++++ xds/googledirectpath/googlec2p_test.go | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/xds/googledirectpath/googlec2p.go b/xds/googledirectpath/googlec2p.go index 1789334fd32..669c4fc1c48 100644 --- a/xds/googledirectpath/googlec2p.go +++ b/xds/googledirectpath/googlec2p.go @@ -92,6 +92,10 @@ type c2pResolverBuilder struct { } func (c2pResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) { + if t.URL.Host != "" { + return nil, fmt.Errorf("google-c2p URI scheme does not support authorities") + } + if !runDirectPath() { // If not xDS, fallback to DNS. t.Scheme = dnsName diff --git a/xds/googledirectpath/googlec2p_test.go b/xds/googledirectpath/googlec2p_test.go index f32eafbf26c..102ffa57fdc 100644 --- a/xds/googledirectpath/googlec2p_test.go +++ b/xds/googledirectpath/googlec2p_test.go @@ -19,6 +19,7 @@ package googledirectpath import ( + "net/url" "strconv" "testing" "time" @@ -251,3 +252,15 @@ func TestBuildXDS(t *testing.T) { }) } } + +// TestBuildFailsWhenCalledWithAuthority asserts that c2p resolver.build +// fails when called with a URL with an authority. +func TestBuildFailsWhenCalledWithAuthority(t *testing.T) { + builder := resolver.Get(c2pScheme) + targetURL, _ := url.Parse("google-c2p://an-authority/resource") + + _, err := builder.Build(resolver.Target{URL: *targetURL}, nil, resolver.BuildOptions{}) + if err == nil { + t.Fatalf("c2p resolver should not support target's with authorities, but build succeeded") + } +} From b4d181f5e874b25bead146e97f14fd98b7f224f2 Mon Sep 17 00:00:00 2001 From: Arvind Bright Date: Wed, 2 Nov 2022 09:54:32 -0700 Subject: [PATCH 2/3] modify unit test --- xds/googledirectpath/googlec2p_test.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/xds/googledirectpath/googlec2p_test.go b/xds/googledirectpath/googlec2p_test.go index 102ffa57fdc..ffc61b6117a 100644 --- a/xds/googledirectpath/googlec2p_test.go +++ b/xds/googledirectpath/googlec2p_test.go @@ -19,14 +19,15 @@ package googledirectpath import ( - "net/url" "strconv" + "strings" "testing" "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/resolver" "google.golang.org/grpc/xds/internal/xdsclient" @@ -253,14 +254,20 @@ func TestBuildXDS(t *testing.T) { } } -// TestBuildFailsWhenCalledWithAuthority asserts that c2p resolver.build -// fails when called with a URL with an authority. +// TestDialFailsWhenTargetContainsAuthority attempts to Dial a target URI of +// google-c2p scheme with a non-empty authority and verifies that it fails with +// an expected error. func TestBuildFailsWhenCalledWithAuthority(t *testing.T) { - builder := resolver.Get(c2pScheme) - targetURL, _ := url.Parse("google-c2p://an-authority/resource") - - _, err := builder.Build(resolver.Target{URL: *targetURL}, nil, resolver.BuildOptions{}) - if err == nil { - t.Fatalf("c2p resolver should not support target's with authorities, but build succeeded") + uri := "google-c2p://an-authority/resource" + cc, err := grpc.Dial(uri, grpc.WithTransportCredentials(insecure.NewCredentials())) + defer func() { + if cc != nil { + cc.Close() + } + }() + wantErr := "google-c2p URI scheme does not support authorities" + if err == nil || !strings.Contains(err.Error(), wantErr) { + cc.Close() + t.Fatalf("grpc.Dial(%s) returned error: %v, want: %v", uri, err, wantErr) } } From 17b691aa4d4824f6bb2d3e84858854a71e96d5cc Mon Sep 17 00:00:00 2001 From: Arvind Bright Date: Wed, 2 Nov 2022 11:46:27 -0700 Subject: [PATCH 3/3] remove redundant close --- xds/googledirectpath/googlec2p_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/xds/googledirectpath/googlec2p_test.go b/xds/googledirectpath/googlec2p_test.go index ffc61b6117a..f9357e9bf3a 100644 --- a/xds/googledirectpath/googlec2p_test.go +++ b/xds/googledirectpath/googlec2p_test.go @@ -267,7 +267,6 @@ func TestBuildFailsWhenCalledWithAuthority(t *testing.T) { }() wantErr := "google-c2p URI scheme does not support authorities" if err == nil || !strings.Contains(err.Error(), wantErr) { - cc.Close() t.Fatalf("grpc.Dial(%s) returned error: %v, want: %v", uri, err, wantErr) } }