From da612a365da1094cf20977ce8af9f9cb7a451b39 Mon Sep 17 00:00:00 2001
From: yue9944882 <291271447@qq.com>
Date: Mon, 4 Nov 2019 17:36:17 +0800
Subject: [PATCH] validates non-resoruce-url

---
 pkg/apis/flowcontrol/validation/validation.go | 43 +++++++++-
 .../flowcontrol/validation/validation_test.go | 81 +++++++++++++++++++
 2 files changed, 122 insertions(+), 2 deletions(-)

diff --git a/pkg/apis/flowcontrol/validation/validation.go b/pkg/apis/flowcontrol/validation/validation.go
index fcea4893f0d..74ef9bf6551 100644
--- a/pkg/apis/flowcontrol/validation/validation.go
+++ b/pkg/apis/flowcontrol/validation/validation.go
@@ -18,6 +18,8 @@ package validation
 
 import (
 	"fmt"
+	"strings"
+
 	"k8s.io/apimachinery/pkg/api/validation"
 	apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -191,8 +193,16 @@ func ValidateFlowSchemaNonResourcePolicyRule(rule *flowcontrol.NonResourcePolicy
 
 	if len(rule.NonResourceURLs) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("nonResourceURLs"), "nonResourceURLs must contain at least one value"))
-	} else if len(rule.NonResourceURLs) > 1 && hasWildcard(rule.NonResourceURLs) {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("nonResourceURLs"), rule.NonResourceURLs, "if '*' is present, must not specify other non-resource URLs"))
+	} else if hasWildcard(rule.NonResourceURLs) {
+		if len(rule.NonResourceURLs) > 1 {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("nonResourceURLs"), rule.NonResourceURLs, "if '*' is present, must not specify other non-resource URLs"))
+		}
+	} else {
+		for i, nonResourceURL := range rule.NonResourceURLs {
+			if err := ValidateNonResourceURLPath(nonResourceURL, fldPath.Child("nonResourceURLs").Index(i)); err != nil {
+				allErrs = append(allErrs, err)
+			}
+		}
 	}
 
 	return allErrs
@@ -332,6 +342,35 @@ func ValidatePriorityLevelConfigurationCondition(condition *flowcontrol.Priority
 	return allErrs
 }
 
+// ValidateNonResourceURLPath validates non-resource-url path by following rules:
+//   1. Slash must be the leading character of the path
+//   2. White-space is forbidden in the path
+//   3. Continuous/double slash is forbidden in the path
+//   4. Wildcard "*" should only do suffix glob matching. Note that wildcard also matches slashes.
+func ValidateNonResourceURLPath(path string, fldPath *field.Path) *field.Error {
+	if len(path) == 0 {
+		return field.Invalid(fldPath, path, "must not be empty")
+	}
+	if path == "/" { // root path
+		return nil
+	}
+
+	if !strings.HasPrefix(path, "/") {
+		return field.Invalid(fldPath, path, "must start with slash")
+	}
+	if strings.Contains(path, " ") {
+		return field.Invalid(fldPath, path, "must not contain white-space")
+	}
+	if strings.Contains(path, "//") {
+		return field.Invalid(fldPath, path, "must not contain double slash")
+	}
+	wildcardCount := strings.Count(path, "*")
+	if wildcardCount > 1 || (wildcardCount == 1 && path[len(path)-2:] != "/*") {
+		return field.Invalid(fldPath, path, "wildcard can only do suffix matching")
+	}
+	return nil
+}
+
 func hasWildcard(operations []string) bool {
 	for _, o := range operations {
 		if o == "*" {
diff --git a/pkg/apis/flowcontrol/validation/validation_test.go b/pkg/apis/flowcontrol/validation/validation_test.go
index 42144ec2025..78e00acf7b1 100644
--- a/pkg/apis/flowcontrol/validation/validation_test.go
+++ b/pkg/apis/flowcontrol/validation/validation_test.go
@@ -626,3 +626,84 @@ func TestValidatePriorityLevelConfigurationStatus(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateNonResourceURLPath(t *testing.T) {
+	testCases := []struct {
+		name           string
+		path           string
+		expectingError bool
+	}{
+		{
+			name:           "empty string should fail",
+			path:           "",
+			expectingError: true,
+		},
+		{
+			name:           "no slash should fail",
+			path:           "foo",
+			expectingError: true,
+		},
+		{
+			name:           "single slash should work",
+			path:           "/",
+			expectingError: false,
+		},
+		{
+			name:           "continuous slash should fail",
+			path:           "//",
+			expectingError: true,
+		},
+		{
+			name:           "/foo slash should work",
+			path:           "/foo",
+			expectingError: false,
+		},
+		{
+			name:           "multiple continuous slashes should fail",
+			path:           "/////",
+			expectingError: true,
+		},
+		{
+			name:           "ending up with slash should work",
+			path:           "/apis/",
+			expectingError: false,
+		},
+		{
+			name:           "ending up with wildcard should work",
+			path:           "/healthz/*",
+			expectingError: false,
+		},
+		{
+			name:           "single wildcard inside the path should fail",
+			path:           "/healthz/*/foo",
+			expectingError: true,
+		},
+		{
+			name:           "white-space in the path should fail",
+			path:           "/healthz/foo bar",
+			expectingError: true,
+		},
+		{
+			name:           "wildcard plus plain path should fail",
+			path:           "/health*",
+			expectingError: true,
+		},
+		{
+			name:           "wildcard plus plain path should fail 2",
+			path:           "/health*/foo",
+			expectingError: true,
+		},
+		{
+			name:           "multiple wildcard internal and suffix should fail",
+			path:           "/*/*",
+			expectingError: true,
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			err := ValidateNonResourceURLPath(testCase.path, field.NewPath(""))
+			assert.Equal(t, testCase.expectingError, err != nil,
+				"actual error: %v", err)
+		})
+	}
+}