diff --git a/xds/internal/client/client.go b/xds/internal/client/client.go index 2daceede5398..603632801b04 100644 --- a/xds/internal/client/client.go +++ b/xds/internal/client/client.go @@ -24,6 +24,7 @@ import ( "context" "errors" "fmt" + "regexp" "sync" "time" @@ -271,7 +272,9 @@ type VirtualHost struct { // Route is both a specification of how to match a request as well as an // indication of the action to take upon match. type Route struct { - Path, Prefix, Regex *string + Path *string + Prefix *string + Regex *regexp.Regexp // Indicates if prefix/path matching should be case insensitive. The default // is false (case sensitive). CaseInsensitive bool @@ -304,20 +307,20 @@ type WeightedCluster struct { // HeaderMatcher represents header matchers. type HeaderMatcher struct { - Name string `json:"name"` - InvertMatch *bool `json:"invertMatch,omitempty"` - ExactMatch *string `json:"exactMatch,omitempty"` - RegexMatch *string `json:"regexMatch,omitempty"` - PrefixMatch *string `json:"prefixMatch,omitempty"` - SuffixMatch *string `json:"suffixMatch,omitempty"` - RangeMatch *Int64Range `json:"rangeMatch,omitempty"` - PresentMatch *bool `json:"presentMatch,omitempty"` + Name string + InvertMatch *bool + ExactMatch *string + RegexMatch *regexp.Regexp + PrefixMatch *string + SuffixMatch *string + RangeMatch *Int64Range + PresentMatch *bool } // Int64Range is a range for header range match. type Int64Range struct { - Start int64 `json:"start"` - End int64 `json:"end"` + Start int64 + End int64 } // SecurityConfig contains the security configuration received as part of the diff --git a/xds/internal/client/rds_test.go b/xds/internal/client/rds_test.go index cde40ee80dfe..a4aaf03e4ae0 100644 --- a/xds/internal/client/rds_test.go +++ b/xds/internal/client/rds_test.go @@ -22,24 +22,26 @@ package client import ( "fmt" + "regexp" "testing" "time" - v2xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" - v2routepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/route" - v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" - v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" - v3typepb "github.com/envoyproxy/go-control-plane/envoy/type/v3" "github.com/golang/protobuf/proto" - anypb "github.com/golang/protobuf/ptypes/any" - wrapperspb "github.com/golang/protobuf/ptypes/wrappers" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/xds/internal/httpfilter" "google.golang.org/grpc/xds/internal/version" "google.golang.org/protobuf/types/known/durationpb" + + v2xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" + v2routepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/route" + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" + v3typepb "github.com/envoyproxy/go-control-plane/envoy/type/v3" + anypb "github.com/golang/protobuf/ptypes/any" + wrapperspb "github.com/golang/protobuf/ptypes/wrappers" ) func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) { @@ -915,6 +917,51 @@ func (s) TestRoutesProtoToSlice(t *testing.T) { }}, wantErr: false, }, + { + name: "good with regex matchers", + routes: []*v3routepb.Route{ + { + Match: &v3routepb.RouteMatch{ + PathSpecifier: &v3routepb.RouteMatch_SafeRegex{SafeRegex: &v3matcherpb.RegexMatcher{Regex: "/a/"}}, + Headers: []*v3routepb.HeaderMatcher{ + { + Name: "th", + HeaderMatchSpecifier: &v3routepb.HeaderMatcher_SafeRegexMatch{SafeRegexMatch: &v3matcherpb.RegexMatcher{Regex: "tv"}}, + }, + }, + RuntimeFraction: &v3corepb.RuntimeFractionalPercent{ + DefaultValue: &v3typepb.FractionalPercent{ + Numerator: 1, + Denominator: v3typepb.FractionalPercent_HUNDRED, + }, + }, + }, + Action: &v3routepb.Route_Route{ + Route: &v3routepb.RouteAction{ + ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{ + WeightedClusters: &v3routepb.WeightedCluster{ + Clusters: []*v3routepb.WeightedCluster_ClusterWeight{ + {Name: "B", Weight: &wrapperspb.UInt32Value{Value: 60}}, + {Name: "A", Weight: &wrapperspb.UInt32Value{Value: 40}}, + }, + TotalWeight: &wrapperspb.UInt32Value{Value: 100}, + }}}}, + }, + }, + wantRoutes: []*Route{{ + Regex: func() *regexp.Regexp { return regexp.MustCompile("/a/") }(), + Headers: []*HeaderMatcher{ + { + Name: "th", + InvertMatch: newBoolP(false), + RegexMatch: func() *regexp.Regexp { return regexp.MustCompile("tv") }(), + }, + }, + Fraction: newUInt32P(10000), + WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}}, + }}, + wantErr: false, + }, { name: "query is ignored", routes: []*v3routepb.Route{ @@ -960,6 +1007,44 @@ func (s) TestRoutesProtoToSlice(t *testing.T) { }, wantErr: true, }, + { + name: "bad regex in path specifier", + routes: []*v3routepb.Route{ + { + Match: &v3routepb.RouteMatch{ + PathSpecifier: &v3routepb.RouteMatch_SafeRegex{SafeRegex: &v3matcherpb.RegexMatcher{Regex: "??"}}, + Headers: []*v3routepb.HeaderMatcher{ + { + HeaderMatchSpecifier: &v3routepb.HeaderMatcher_PrefixMatch{PrefixMatch: "tv"}, + }, + }, + }, + Action: &v3routepb.Route_Route{ + Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}}, + }, + }, + }, + wantErr: true, + }, + { + name: "bad regex in header specifier", + routes: []*v3routepb.Route{ + { + Match: &v3routepb.RouteMatch{ + PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"}, + Headers: []*v3routepb.HeaderMatcher{ + { + HeaderMatchSpecifier: &v3routepb.HeaderMatcher_SafeRegexMatch{SafeRegexMatch: &v3matcherpb.RegexMatcher{Regex: "??"}}, + }, + }, + }, + Action: &v3routepb.Route_Route{ + Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}}, + }, + }, + }, + wantErr: true, + }, { name: "unrecognized header match specifier", routes: []*v3routepb.Route{ @@ -1063,7 +1148,7 @@ func (s) TestRoutesProtoToSlice(t *testing.T) { } cmpOpts := []cmp.Option{ - cmp.AllowUnexported(Route{}, HeaderMatcher{}, Int64Range{}), + cmp.AllowUnexported(Route{}, HeaderMatcher{}, Int64Range{}, regexp.Regexp{}), cmpopts.EquateEmpty(), cmp.Transformer("FilterConfig", func(fc httpfilter.FilterConfig) string { return fmt.Sprint(fc) @@ -1074,17 +1159,15 @@ func (s) TestRoutesProtoToSlice(t *testing.T) { t.Run(tt.name, func(t *testing.T) { oldFI := env.FaultInjectionSupport env.FaultInjectionSupport = !tt.disableFI + defer func() { env.FaultInjectionSupport = oldFI }() got, err := routesProtoToSlice(tt.routes, nil, false) if (err != nil) != tt.wantErr { - t.Errorf("routesProtoToSlice() error = %v, wantErr %v", err, tt.wantErr) - return + t.Fatalf("routesProtoToSlice() error = %v, wantErr %v", err, tt.wantErr) } - if !cmp.Equal(got, tt.wantRoutes, cmpOpts...) { - t.Errorf("routesProtoToSlice() got = %v, want %v, diff: %v", got, tt.wantRoutes, cmp.Diff(got, tt.wantRoutes, cmpOpts...)) + if diff := cmp.Diff(got, tt.wantRoutes, cmpOpts...); diff != "" { + t.Fatalf("routesProtoToSlice() returned unexpected diff (-got +want):\n%s", diff) } - - env.FaultInjectionSupport = oldFI }) } } diff --git a/xds/internal/client/xds.go b/xds/internal/client/xds.go index c0caf5cceb57..55bcc2e936ca 100644 --- a/xds/internal/client/xds.go +++ b/xds/internal/client/xds.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "net" + "regexp" "strconv" "strings" "time" @@ -437,7 +438,12 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger, case *v3routepb.RouteMatch_Path: route.Path = &pt.Path case *v3routepb.RouteMatch_SafeRegex: - route.Regex = &pt.SafeRegex.Regex + regex := pt.SafeRegex.GetRegex() + re, err := regexp.Compile(regex) + if err != nil { + return nil, fmt.Errorf("route %+v contains an invalid regex %q", r, regex) + } + route.Regex = re default: return nil, fmt.Errorf("route %+v has an unrecognized path specifier: %+v", r, pt) } @@ -452,7 +458,12 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger, case *v3routepb.HeaderMatcher_ExactMatch: header.ExactMatch = &ht.ExactMatch case *v3routepb.HeaderMatcher_SafeRegexMatch: - header.RegexMatch = &ht.SafeRegexMatch.Regex + regex := ht.SafeRegexMatch.GetRegex() + re, err := regexp.Compile(regex) + if err != nil { + return nil, fmt.Errorf("route %+v contains an invalid regex %q", r, regex) + } + header.RegexMatch = re case *v3routepb.HeaderMatcher_RangeMatch: header.RangeMatch = &Int64Range{ Start: ht.RangeMatch.Start, diff --git a/xds/internal/resolver/matcher.go b/xds/internal/resolver/matcher.go index b7b5f3db0e3e..06456a585573 100644 --- a/xds/internal/resolver/matcher.go +++ b/xds/internal/resolver/matcher.go @@ -20,7 +20,6 @@ package resolver import ( "fmt" - "regexp" "strings" "google.golang.org/grpc/internal/grpcrand" @@ -34,11 +33,7 @@ func routeToMatcher(r *xdsclient.Route) (*compositeMatcher, error) { var pathMatcher pathMatcherInterface switch { case r.Regex != nil: - re, err := regexp.Compile(*r.Regex) - if err != nil { - return nil, fmt.Errorf("failed to compile regex %q", *r.Regex) - } - pathMatcher = newPathRegexMatcher(re) + pathMatcher = newPathRegexMatcher(r.Regex) case r.Path != nil: pathMatcher = newPathExactMatcher(*r.Path, r.CaseInsensitive) case r.Prefix != nil: @@ -53,12 +48,8 @@ func routeToMatcher(r *xdsclient.Route) (*compositeMatcher, error) { switch { case h.ExactMatch != nil && *h.ExactMatch != "": matcherT = newHeaderExactMatcher(h.Name, *h.ExactMatch) - case h.RegexMatch != nil && *h.RegexMatch != "": - re, err := regexp.Compile(*h.RegexMatch) - if err != nil { - return nil, fmt.Errorf("failed to compile regex %q, skipping this matcher", *h.RegexMatch) - } - matcherT = newHeaderRegexMatcher(h.Name, re) + case h.RegexMatch != nil: + matcherT = newHeaderRegexMatcher(h.Name, h.RegexMatch) case h.PrefixMatch != nil && *h.PrefixMatch != "": matcherT = newHeaderPrefixMatcher(h.Name, *h.PrefixMatch) case h.SuffixMatch != nil && *h.SuffixMatch != "":