xds: nack route configuration with regexes that don't compile

grpc · May 5, 2021 · d1c78c7 · d1c78c7
1 parent 0fc0397
commit d1c78c7
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 10 deletions.
diff --git a/xds/internal/client/rds_test.go b/xds/internal/client/rds_test.go
@@ -25,21 +25,22 @@ import (
 	"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) {
@@ -960,6 +961,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{

diff --git 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()
+			_, err := regexp.Compile(regex)
+			if err != nil {
+				return nil, fmt.Errorf("route %+v contains an invalid regex %q", r, regex)
+			}
+			route.Regex = &regex
 		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()
+				_, err := regexp.Compile(regex)
+				if err != nil {
+					return nil, fmt.Errorf("route %+v contains an invalid regex %q", r, regex)
+				}
+				header.RegexMatch = &regex
 			case *v3routepb.HeaderMatcher_RangeMatch:
 				header.RangeMatch = &Int64Range{
 					Start: ht.RangeMatch.Start,