dont modify regex, update tests

lyft · Jul 18, 2023 · 861a3d0 · 861a3d0
1 parent 8e26f7b
commit 861a3d0
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 13 deletions.
diff --git a/backend/module/proxy/proxy.go b/backend/module/proxy/proxy.go
@@ -36,21 +36,17 @@ func New(cfg *any.Any, log *zap.Logger, scope tally.Scope) (module.Module, error
 		return nil, err
 	}
 
-	// Validate that each services constructs a parsable URL
 	for _, service := range config.Services {
 		for _, ar := range service.AllowedRequests {
-			var path string
 			switch t := ar.PathType.(type) {
 			case *proxyv1cfg.AllowRequest_Path:
-				path = t.Path
-			case *proxyv1cfg.AllowRequest_PathRegex:
-				path = t.PathRegex
+				// For exact path type, validate that each services constructs a parsable URL
+				_, err := url.Parse(fmt.Sprintf("%s%s", service.Host, t.Path))
+				if err != nil {
+					return nil, fmt.Errorf("unable to parse the configured URL for service [%s]", service.Name)
+				}
 			default:
-				return nil, fmt.Errorf("path type not supported: %s", t)
-			}
-			_, err := url.Parse(fmt.Sprintf("%s%s", service.Host, path))
-			if err != nil {
-				return nil, fmt.Errorf("unable to parse the configured URL for service [%s]", service.Name)
+				break
 			}
 		}
 	}
@@ -225,8 +221,7 @@ func isAllowedRequest(services []*proxyv1cfg.Service, service, path, method stri
 						return true, nil
 					}
 				case *proxyv1cfg.AllowRequest_PathRegex:
-					// MatchString reports whether the string contains any match of the regular expression so we add the ‘^’ and ‘$’ to ensure the regex matches the full string
-					r := regexp.MustCompile(fmt.Sprintf("^%s$", t.PathRegex))
+					r := regexp.MustCompile(t.PathRegex)
 					if r.MatchString(path) {
 						return true, nil
 					}

diff --git a/backend/module/proxy/proxy_test.go b/backend/module/proxy/proxy_test.go
@@ -39,6 +39,7 @@ func generateServicesConfig(host string) []*proxyv1cfg.Service {
 			AllowedRequests: []*proxyv1cfg.AllowRequest{
 				{PathType: &proxyv1cfg.AllowRequest_Path{Path: "/meow"}, Method: "GET"},
 				{PathType: &proxyv1cfg.AllowRequest_Path{Path: "/nom"}, Method: "POST"},
+				{PathType: &proxyv1cfg.AllowRequest_PathRegex{PathRegex: `^/meow/\w+/\w+$`}, Method: "GET"},
 			},
 		},
 	}
@@ -270,7 +271,7 @@ func TestIsAllowedRequest(t *testing.T) {
 			shouldError: false,
 		},
 		{
-			id:          "Regex matches request path string",
+			id:          "Path string matches regex",
 			service:     "cat",
 			path:        "/cat/bengal/1",
 			method:      "POST",
@@ -293,6 +294,38 @@ func TestIsAllowedRequest(t *testing.T) {
 			expect:      false,
 			shouldError: false,
 		},
+		{
+			id:          "Regex does not have a rule for end of string",
+			service:     "cat",
+			path:        "/cat/bengal/1/comments",
+			method:      "POST",
+			expect:      true,
+			shouldError: false,
+		},
+		{
+			id:          "Path string matches regex",
+			service:     "meow",
+			path:        "/meow/foo/sound",
+			method:      "POST",
+			expect:      true,
+			shouldError: false,
+		},
+		{
+			id:          "Path string does not match regex last character",
+			service:     "meow",
+			path:        "/meow/foo/sound/play",
+			method:      "POST",
+			expect:      false,
+			shouldError: false,
+		},
+		{
+			id:          "Path string does not match regex first character",
+			service:     "meow",
+			path:        "animals/meow/foo/sound/play",
+			method:      "POST",
+			expect:      false,
+			shouldError: false,
+		},
 	}
 
 	services := generateServicesConfig("http://test.test")