add unit tests and fix failing cases

Author: bohde

modules/util/truncate.go

@@ -41,6 +41,8 @@ func SplitStringAtByteN(input string, n int) (left, right string) {
 
 // SplitTrimSpace splits the string at given separator and trims leading and trailing space
 func SplitTrimSpace(input, sep string) []string {
+	// Trim initial leading & trailing space
+	input = strings.TrimSpace(input)
 	// replace CRLF with LF
 	input = strings.ReplaceAll(input, "\r\n", "\n")
 

modules/validation/binding.go

@@ -64,7 +64,7 @@ func addValidURLListBindingRule() {
 	// URL validation rule
 	binding.AddRule(&binding.Rule{
 		IsMatch: func(rule string) bool {
-			return strings.HasPrefix(rule, "ValidUrlList")
+			return strings.EqualFold(rule, "ValidUrlList")
 		},
 		IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
 			str := fmt.Sprintf("%v", val)
@@ -78,7 +78,7 @@ func addValidURLListBindingRule() {
 			for _, u := range urls {
 				if !IsValidURL(u) {
 					ok = false
-					errs.Add([]string{"RedirectURIs"}, binding.ERR_URL, u)
+					errs.Add([]string{name}, binding.ERR_URL, u)
 				}
 			}
 
@@ -91,7 +91,7 @@ func addValidURLBindingRule() {
 	// URL validation rule
 	binding.AddRule(&binding.Rule{
 		IsMatch: func(rule string) bool {
-			return strings.HasPrefix(rule, "ValidUrl")
+			return strings.EqualFold(rule, "ValidUrl")
 		},
 		IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
 			str := fmt.Sprintf("%v", val)

modules/validation/binding_test.go

@@ -27,6 +27,7 @@ type (
 	TestForm struct {
 		BranchName   string `form:"BranchName" binding:"GitRefName"`
 		URL          string `form:"ValidUrl" binding:"ValidUrl"`
+		URLs         string `form:"ValidUrls" binding:"ValidUrlList"`
 		GlobPattern  string `form:"GlobPattern" binding:"GlobPattern"`
 		RegexPattern string `form:"RegexPattern" binding:"RegexPattern"`
 	}

modules/validation/validurllist_test.go

@@ -0,0 +1,157 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package validation
+
+import (
+	"testing"
+
+	"gitea.com/go-chi/binding"
+)
+
+// This is a copy of all the URL tests cases, plus additional ones to
+// account for multiple URLs
+var urlListValidationTestCases = []validationTestCase{
+	{
+		description: "Empty URL",
+		data: TestForm{
+			URLs: "",
+		},
+		expectedErrors: binding.Errors{},
+	},
+	{
+		description: "URL without port",
+		data: TestForm{
+			URLs: "http://test.lan/",
+		},
+		expectedErrors: binding.Errors{},
+	},
+	{
+		description: "URL with port",
+		data: TestForm{
+			URLs: "http://test.lan:3000/",
+		},
+		expectedErrors: binding.Errors{},
+	},
+	{
+		description: "URL with IPv6 address without port",
+		data: TestForm{
+			URLs: "http://[::1]/",
+		},
+		expectedErrors: binding.Errors{},
+	},
+	{
+		description: "URL with IPv6 address with port",
+		data: TestForm{
+			URLs: "http://[::1]:3000/",
+		},
+		expectedErrors: binding.Errors{},
+	},
+	{
+		description: "Invalid URL",
+		data: TestForm{
+			URLs: "http//test.lan/",
+		},
+		expectedErrors: binding.Errors{
+			binding.Error{
+				FieldNames:     []string{"URLs"},
+				Classification: binding.ERR_URL,
+				Message:        "http//test.lan/",
+			},
+		},
+	},
+	{
+		description: "Invalid schema",
+		data: TestForm{
+			URLs: "ftp://test.lan/",
+		},
+		expectedErrors: binding.Errors{
+			binding.Error{
+				FieldNames:     []string{"URLs"},
+				Classification: binding.ERR_URL,
+				Message:        "ftp://test.lan/",
+			},
+		},
+	},
+	{
+		description: "Invalid port",
+		data: TestForm{
+			URLs: "http://test.lan:3x4/",
+		},
+		expectedErrors: binding.Errors{
+			binding.Error{
+				FieldNames:     []string{"URLs"},
+				Classification: binding.ERR_URL,
+				Message:        "http://test.lan:3x4/",
+			},
+		},
+	},
+	{
+		description: "Invalid port with IPv6 address",
+		data: TestForm{
+			URLs: "http://[::1]:3x4/",
+		},
+		expectedErrors: binding.Errors{
+			binding.Error{
+				FieldNames:     []string{"URLs"},
+				Classification: binding.ERR_URL,
+				Message:        "http://[::1]:3x4/",
+			},
+		},
+	},
+	{
+		description: "Multi URLs",
+		data: TestForm{
+			URLs: "http://test.lan:3000/\nhttp://test.local/",
+		},
+		expectedErrors: binding.Errors{},
+	},
+	{
+		description: "Multi URLs with newline",
+		data: TestForm{
+			URLs: "http://test.lan:3000/\nhttp://test.local/\n",
+		},
+		expectedErrors: binding.Errors{},
+	},
+	{
+		description: "List with invalid entry",
+		data: TestForm{
+			URLs: "http://test.lan:3000/\nhttp://[::1]:3x4/",
+		},
+		expectedErrors: binding.Errors{
+			binding.Error{
+				FieldNames:     []string{"URLs"},
+				Classification: binding.ERR_URL,
+				Message:        "http://[::1]:3x4/",
+			},
+		},
+	},
+	{
+		description: "List with two invalid entries",
+		data: TestForm{
+			URLs: "ftp://test.lan:3000/\nhttp://[::1]:3x4/\n",
+		},
+		expectedErrors: binding.Errors{
+			binding.Error{
+				FieldNames:     []string{"URLs"},
+				Classification: binding.ERR_URL,
+				Message:        "ftp://test.lan:3000/",
+			},
+			binding.Error{
+				FieldNames:     []string{"URLs"},
+				Classification: binding.ERR_URL,
+				Message:        "http://[::1]:3x4/",
+			},
+		},
+	},
+}
+
+func Test_ValidURLListValidation(t *testing.T) {
+	AddBindingRules()
+
+	for _, testCase := range urlListValidationTestCases {
+		t.Run(testCase.description, func(t *testing.T) {
+			performValidationTest(t, testCase)
+		})
+	}
+}