diff --git a/.golangci.yml b/.golangci.yml index 32d133aa098..f7837291561 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -52,6 +52,8 @@ linters: desc: do not use the go-chi cache package, use gitea's cache system - pkg: github.com/pkg/errors desc: use builtin errors package instead + - pkg: gitea.com/go-chi/binding + desc: use our wrapper migrations: files: - '**/modelmigration/**/*.go' diff --git a/go.mod b/go.mod index 50d76250d94..26731e6c595 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ toolchain go1.26.6 require ( connectrpc.com/connect v1.20.0 - gitea.com/go-chi/binding v0.0.0-20260414111559-654cea7ac60a + gitea.com/go-chi/binding v0.0.0-20260818211407-ac8602c87be9 gitea.com/go-chi/cache v0.2.1 gitea.com/go-chi/captcha v0.0.0-20240315150714-fb487f629098 gitea.com/go-chi/session v0.0.0-20260708011333-ebced8a7a2d6 diff --git a/go.sum b/go.sum index edfdc158927..f8cbc120b93 100644 --- a/go.sum +++ b/go.sum @@ -8,8 +8,8 @@ dario.cat/mergo v1.0.2 h1:85+piFYR1tMbRrLcDwR18y4UKJ3aH1Tbzi24VRW1TK8= dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA= filippo.io/edwards25519 v1.2.0 h1:crnVqOiS4jqYleHd9vaKZ+HKtHfllngJIiOpNpoJsjo= filippo.io/edwards25519 v1.2.0/go.mod h1:xzAOLCNug/yB62zG1bQ8uziwrIqIuxhctzJT18Q77mc= -gitea.com/go-chi/binding v0.0.0-20260414111559-654cea7ac60a h1:JHoBrfuTSF9Ke9aNfSYj1XRPBHjKPgCApVprnt2Am0M= -gitea.com/go-chi/binding v0.0.0-20260414111559-654cea7ac60a/go.mod h1:FOsLJIMdpiHzBp3Vby6Wfkdw2ppGscrjgU1IC7E4/zQ= +gitea.com/go-chi/binding v0.0.0-20260818211407-ac8602c87be9 h1:J/NzRmGh7olgthuZ096FNdJcQWuEpJb3ycEi6yArlGY= +gitea.com/go-chi/binding v0.0.0-20260818211407-ac8602c87be9/go.mod h1:q1SSPpkC9A0gfNnoqqZ3My6kEHIpKN6QsTI+Zx73B/o= gitea.com/go-chi/cache v0.2.1 h1:bfAPkvXlbcZxPCpcmDVCWoHgiBSBmZN/QosnZvEC0+g= gitea.com/go-chi/cache v0.2.1/go.mod h1:Qic0HZ8hOHW62ETGbonpwz8WYypj9NieU9659wFUJ8Q= gitea.com/go-chi/captcha v0.0.0-20240315150714-fb487f629098 h1:p2ki+WK0cIeNQuqjR98IP2KZQKRzJJiV7aTeMAFwaWo= diff --git a/modules/issue/template/template.go b/modules/issue/template/template.go index 6f2372021d0..97d854f2348 100644 --- a/modules/issue/template/template.go +++ b/modules/issue/template/template.go @@ -11,13 +11,21 @@ import ( "slices" "strconv" "strings" + "sync" "gitea.dev/modules/container" api "gitea.dev/modules/structs" - - "gitea.com/go-chi/binding" ) +var globalVars = sync.OnceValue(func() (ret struct { + nonAlphaDashPattern, minQuotesRegex *regexp.Regexp +}, +) { + ret.nonAlphaDashPattern = regexp.MustCompile(`[^\w-]`) + ret.minQuotesRegex = regexp.MustCompilePOSIX("^`{3,}") + return ret +}) + // Validate checks whether an IssueTemplate is considered valid, and returns the first error func Validate(template *api.IssueTemplate) error { if err := validateMetadata(template); err != nil { @@ -150,7 +158,7 @@ func validateID(field *api.IssueFormField, idx int, ids container.Set[string]) e // If the ID is empty in yaml, template.Unmarshal will auto autofill it, so it cannot be empty return position.Errorf("'id' is required") } - if binding.AlphaDashPattern.MatchString(field.ID) { + if globalVars().nonAlphaDashPattern.MatchString(field.ID) { return position.Errorf("'id' should contain only alphanumeric, '-' and '_'") } if !ids.Add(field.ID) { @@ -471,13 +479,11 @@ func (o *valuedOption) VisibleInContent() bool { return true } -var minQuotesRegex = regexp.MustCompilePOSIX("^`{3,}") - // minQuotes return 3 or more back-quotes. // If n back-quotes exists, use n+1 back-quotes to quote. func minQuotes(value string) string { ret := "```" - for _, v := range minQuotesRegex.FindAllString(value, -1) { + for _, v := range globalVars().minQuotesRegex.FindAllString(value, -1) { if len(v) >= len(ret) { ret = v + "`" } diff --git a/modules/private/manager.go b/modules/private/manager.go index 51a703200b7..559de8dc577 100644 --- a/modules/private/manager.go +++ b/modules/private/manager.go @@ -80,14 +80,6 @@ func SetLogSQL(ctx context.Context, on bool) ResponseExtra { return requestJSONClientMsg(req, "Log SQL setting set") } -// LoggerOptions represents the options for the add logger call -type LoggerOptions struct { - Logger string - Writer string - Mode string - Config map[string]any -} - // Processes return the current processes from this gitea instance func Processes(ctx context.Context, out io.Writer, flat, noSystem, stacktraces, json bool, cancel string) ResponseExtra { reqURL := setting.LocalURL + fmt.Sprintf("api/internal/manager/processes?flat=%t&no-system=%t&stacktraces=%t&json=%t&cancel-pid=%s", flat, noSystem, stacktraces, json, url.QueryEscape(cancel)) diff --git a/modules/structs/form.go b/modules/structs/form.go index 4bbb48b9306..1b245769ad5 100644 --- a/modules/structs/form.go +++ b/modules/structs/form.go @@ -4,20 +4,14 @@ package structs import ( - "net/http" - - "gitea.dev/modules/reqctx" "gitea.dev/modules/translation/i18n" - "gitea.com/go-chi/binding" + "gitea.com/go-chi/binding" //nolint:depguard // avoid cycle import ) // ValidateContext is a special context for form validation middleware type ValidateContext struct { Locale i18n.LocaleTranslation - Data reqctx.ContextData - Req *http.Request - Resp http.ResponseWriter } type FormDefaultValidator struct{} diff --git a/modules/validation/binding.go b/modules/validation/binding.go index b4d2d9d1577..c93771dc71b 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -4,17 +4,20 @@ package validation import ( - "fmt" + "context" "io" + "reflect" "regexp" "strings" + "sync" "gitea.dev/modules/auth" "gitea.dev/modules/git" "gitea.dev/modules/glob" "gitea.dev/modules/json" + "gitea.dev/modules/util" - "gitea.com/go-chi/binding" + "gitea.com/go-chi/binding" //nolint:depguard // this package wraps it ) const ( @@ -41,178 +44,75 @@ func (j jsonProvider) NewEncoder(writer io.Writer) binding.JSONEncoder { return json.NewEncoder(writer) } +func newFieldError(field reflect.StructField, cls, msg string) *BindingError { + return &BindingError{[]string{field.Name}, cls, msg} //nolint:govet // make sure no missing fields +} + // AddBindingRules adds additional binding rules -func AddBindingRules() { +func AddBindingRules(b *binding.Binder) { binding.JSONProvider = jsonProvider{} - addGitRefNameBindingRule() - addValidURLBindingRule() - addValidSiteURLBindingRule() - addGlobPatternRule() - addRegexPatternRule() - addGlobOrRegexPatternRule() - addUsernamePatternRule() - addValidGroupTeamMapRule() - addSlugPatternRule() -} - -func addGitRefNameBindingRule() { - // Git ref name validation rule - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "GitRefName" - }, - IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { - str := fmt.Sprintf("%v", val) - - if !git.IsValidRefPattern(str) { - errs.Add([]string{name}, ErrGitRefName, "GitRefName") - return false, errs - } - return true, errs - }, - }) -} - -func addValidURLBindingRule() { - // URL validation rule - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "ValidUrl" - }, - IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { - str := fmt.Sprintf("%v", val) - if len(str) != 0 && !IsValidURL(str) { - errs.Add([]string{name}, binding.ERR_URL, "Url") - return false, errs - } - - return true, errs - }, - }) -} - -func addValidSiteURLBindingRule() { - // URL validation rule - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "ValidSiteUrl" - }, - IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { - str := fmt.Sprintf("%v", val) - if len(str) != 0 && !IsValidSiteURL(str) { - errs.Add([]string{name}, binding.ERR_URL, "Url") - return false, errs - } - - return true, errs - }, - }) -} - -func addSlugPatternRule() { - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "BadgeSlug" - }, - IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { - str := fmt.Sprintf("%v", val) - if !IsValidBadgeSlug(str) { - errs.Add([]string{name}, ErrInvalidBadgeSlug, "invalid badge slug") - return false, errs - } - return true, errs - }, - }) -} - -func addGlobPatternRule() { - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "GlobPattern" - }, - IsValid: globPatternValidator, - }) -} - -func globPatternValidator(errs binding.Errors, name string, val any) (bool, binding.Errors) { - str := fmt.Sprintf("%v", val) - - if len(str) != 0 { - if _, err := glob.Compile(str); err != nil { - errs.Add([]string{name}, ErrGlobPattern, err.Error()) - return false, errs + b.AddRuleNonZero("GitRefName", func(ctx context.Context, f *binding.ValidationField) *binding.Error { + if !git.IsValidRefPattern(f.ValueMustString()) { + return newFieldError(f.StructField, ErrGitRefName, "GitRefName") } + return nil + }) + b.AddRuleNonZero("ValidUrl", func(ctx context.Context, f *binding.ValidationField) *binding.Error { + if !IsValidURL(f.ValueMustString()) { + return newFieldError(f.StructField, binding.ERR_URL, "Url") + } + return nil + }) + b.AddRuleNonZero("ValidSiteUrl", func(ctx context.Context, f *binding.ValidationField) *binding.Error { + if !IsValidSiteURL(f.ValueMustString()) { + return newFieldError(f.StructField, binding.ERR_URL, "Url") + } + return nil + }) + b.AddRuleNonZero("BadgeSlug", func(ctx context.Context, f *binding.ValidationField) *binding.Error { + if !IsValidBadgeSlug(f.ValueMustString()) { + return newFieldError(f.StructField, ErrInvalidBadgeSlug, "invalid badge slug") + } + return nil + }) + + ruleGlobPattern := func(_ context.Context, f *binding.ValidationField) *binding.Error { + if _, err := glob.Compile(f.ValueMustString()); err != nil { + return newFieldError(f.StructField, ErrGlobPattern, err.Error()) + } + return nil } - - return true, errs -} - -func addRegexPatternRule() { - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "RegexPattern" - }, - IsValid: regexPatternValidator, - }) -} - -func regexPatternValidator(errs binding.Errors, name string, val any) (bool, binding.Errors) { - str := fmt.Sprintf("%v", val) - - if _, err := regexp.Compile(str); err != nil { - errs.Add([]string{name}, ErrRegexPattern, err.Error()) - return false, errs + b.AddRuleNonZero("GlobPattern", ruleGlobPattern) + ruleRegexPattern := func(_ context.Context, f *binding.ValidationField, val string) *binding.Error { + if _, err := regexp.Compile(val); err != nil { + return newFieldError(f.StructField, ErrRegexPattern, err.Error()) + } + return nil } - - return true, errs -} - -func addGlobOrRegexPatternRule() { - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "GlobOrRegexPattern" - }, - IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { - str := strings.TrimSpace(fmt.Sprintf("%v", val)) - - if len(str) >= 2 && strings.HasPrefix(str, "/") && strings.HasSuffix(str, "/") { - return regexPatternValidator(errs, name, str[1:len(str)-1]) - } - return globPatternValidator(errs, name, val) - }, + b.AddRuleNonZero("RegexPattern", func(ctx context.Context, f *binding.ValidationField) *binding.Error { + return ruleRegexPattern(ctx, f, f.ValueMustString()) }) -} - -func addUsernamePatternRule() { - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "Username" - }, - IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { - str := fmt.Sprintf("%v", val) - if !IsValidUsername(str) { - errs.Add([]string{name}, ErrUsername, "invalid username") - return false, errs - } - return true, errs - }, + b.AddRuleNonZero("GlobOrRegexPattern", func(ctx context.Context, f *binding.ValidationField) *binding.Error { + str := f.ValueMustString() + if len(str) >= 2 && strings.HasPrefix(str, "/") && strings.HasSuffix(str, "/") { + return ruleRegexPattern(ctx, f, str[1:len(str)-1]) + } + return ruleGlobPattern(ctx, f) }) -} -func addValidGroupTeamMapRule() { - binding.AddRule(&binding.Rule{ - IsMatch: func(rule string) bool { - return rule == "ValidGroupTeamMap" - }, - IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { - _, err := auth.UnmarshalGroupTeamMapping(fmt.Sprintf("%v", val)) - if err != nil { - errs.Add([]string{name}, ErrInvalidGroupTeamMap, err.Error()) - return false, errs - } + b.AddRuleNonZero("Username", func(ctx context.Context, f *binding.ValidationField) *binding.Error { + if !IsValidUsername(f.ValueMustString()) { + return newFieldError(f.StructField, ErrUsername, "invalid username") + } + return nil + }) - return true, errs - }, + b.AddRuleNonZero("ValidGroupTeamMap", func(ctx context.Context, f *binding.ValidationField) *binding.Error { + _, err := auth.UnmarshalGroupTeamMapping(f.ValueMustString()) + if err != nil { + return newFieldError(f.StructField, ErrInvalidGroupTeamMap, err.Error()) + } + return nil }) } @@ -238,3 +138,14 @@ func validPort(p string) bool { } return true } + +var Binder = sync.OnceValue(func() *binding.Binder { + b := binding.NewBinder().WithDefaultRules().WithNameMapper(util.ToSnakeCase) + AddBindingRules(b) + return b +}) + +type ( + BindingErrors = binding.Errors + BindingError = binding.Error +) diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go index ee893c516e8..e3b0187a577 100644 --- a/modules/validation/binding_test.go +++ b/modules/validation/binding_test.go @@ -4,24 +4,16 @@ package validation import ( - "net/http" - "net/http/httptest" "testing" - "gitea.com/go-chi/binding" - chi "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" ) -const ( - testRoute = "/test" -) - type ( validationTestCase struct { description string data any - expectedErrors binding.Errors + expectedErrors BindingErrors } TestForm struct { @@ -33,24 +25,5 @@ type ( ) func performValidationTest(t *testing.T, testCase validationTestCase) { - httpRecorder := httptest.NewRecorder() - m := chi.NewRouter() - - m.Post(testRoute, func(resp http.ResponseWriter, req *http.Request) { - assert.Equal(t, testCase.expectedErrors, binding.Validate(req, testCase.data)) - }) - - req, err := http.NewRequest(http.MethodPost, testRoute, nil) - if err != nil { - panic(err) - } - req.Header.Add("Content-Type", "x-www-form-urlencoded") - m.ServeHTTP(httpRecorder, req) - - switch httpRecorder.Code { - case http.StatusNotFound: - panic("Routing is messed up in test fixture (got 404): check methods and paths") - case http.StatusInternalServerError: - panic("Something bad happened on '" + testCase.description + "'") - } + assert.Equal(t, testCase.expectedErrors, Binder().Validate(t.Context(), testCase.data)) } diff --git a/modules/validation/glob_pattern_test.go b/modules/validation/glob_pattern_test.go index 094f307380d..44b1cac8e09 100644 --- a/modules/validation/glob_pattern_test.go +++ b/modules/validation/glob_pattern_test.go @@ -7,8 +7,6 @@ import ( "testing" "gitea.dev/modules/glob" - - "gitea.com/go-chi/binding" ) func getGlobPatternErrorString(pattern string) string { @@ -21,29 +19,27 @@ func getGlobPatternErrorString(pattern string) string { } func Test_GlobPatternValidation(t *testing.T) { - AddBindingRules() - globValidationTestCases := []validationTestCase{ { description: "Empty glob pattern", - data: TestForm{ + data: &TestForm{ GlobPattern: "", }, }, { description: "Valid glob", - data: TestForm{ + data: &TestForm{ GlobPattern: "{master,release*}", }, }, { description: "Invalid glob", - data: TestForm{ + data: &TestForm{ GlobPattern: "[a-", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"GlobPattern"}, Classification: ErrGlobPattern, Message: getGlobPatternErrorString("[a-"), diff --git a/modules/validation/refname_test.go b/modules/validation/refname_test.go index f08aa8aa357..19e5cc5b915 100644 --- a/modules/validation/refname_test.go +++ b/modules/validation/refname_test.go @@ -5,38 +5,35 @@ package validation import ( "testing" - - "gitea.com/go-chi/binding" ) func Test_GitRefNameValidation(t *testing.T) { - AddBindingRules() gitRefNameValidationTestCases := []validationTestCase{ { description: "Reference name contains only characters", - data: TestForm{ + data: &TestForm{ BranchName: "test", }, }, { description: "Reference name contains single slash", - data: TestForm{ + data: &TestForm{ BranchName: "feature/test", }, }, { description: "Reference name has allowed special characters", - data: TestForm{ + data: &TestForm{ BranchName: "debian/1%1.6.0-2", }, }, { description: "Reference name contains backslash", - data: TestForm{ + data: &TestForm{ BranchName: "feature\\test", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -45,11 +42,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name starts with dot", - data: TestForm{ + data: &TestForm{ BranchName: ".test", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -58,11 +55,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name ends with dot", - data: TestForm{ + data: &TestForm{ BranchName: "test.", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -71,11 +68,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name starts with slash", - data: TestForm{ + data: &TestForm{ BranchName: "/test", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -84,11 +81,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name ends with slash", - data: TestForm{ + data: &TestForm{ BranchName: "test/", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -97,11 +94,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name ends with .lock", - data: TestForm{ + data: &TestForm{ BranchName: "test.lock", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -110,11 +107,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name contains multiple consecutive dots", - data: TestForm{ + data: &TestForm{ BranchName: "te..st", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -123,11 +120,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name contains multiple consecutive slashes", - data: TestForm{ + data: &TestForm{ BranchName: "te//st", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -136,11 +133,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name is single @", - data: TestForm{ + data: &TestForm{ BranchName: "@", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -149,11 +146,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name has @{", - data: TestForm{ + data: &TestForm{ BranchName: "branch@{", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -162,11 +159,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name has unallowed special character ~", - data: TestForm{ + data: &TestForm{ BranchName: "~debian/1%1.6.0-2", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -175,11 +172,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name has unallowed special character *", - data: TestForm{ + data: &TestForm{ BranchName: "*debian/1%1.6.0-2", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -188,11 +185,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name has unallowed special character ?", - data: TestForm{ + data: &TestForm{ BranchName: "?debian/1%1.6.0-2", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -201,11 +198,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name has unallowed special character ^", - data: TestForm{ + data: &TestForm{ BranchName: "^debian/1%1.6.0-2", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -214,11 +211,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name has unallowed special character :", - data: TestForm{ + data: &TestForm{ BranchName: "debian:jessie", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -227,11 +224,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name has unallowed special character (whitespace)", - data: TestForm{ + data: &TestForm{ BranchName: "debian jessie", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", @@ -240,11 +237,11 @@ func Test_GitRefNameValidation(t *testing.T) { }, { description: "Reference name has unallowed special character [", - data: TestForm{ + data: &TestForm{ BranchName: "debian[jessie", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"BranchName"}, Classification: ErrGitRefName, Message: "GitRefName", diff --git a/modules/validation/regex_pattern_test.go b/modules/validation/regex_pattern_test.go index 7f600da6866..1bd582c67c8 100644 --- a/modules/validation/regex_pattern_test.go +++ b/modules/validation/regex_pattern_test.go @@ -6,8 +6,6 @@ package validation import ( "regexp" "testing" - - "gitea.com/go-chi/binding" ) func getRegexPatternErrorString(pattern string) string { @@ -18,29 +16,27 @@ func getRegexPatternErrorString(pattern string) string { } func Test_RegexPatternValidation(t *testing.T) { - AddBindingRules() - regexValidationTestCases := []validationTestCase{ { description: "Empty regex pattern", - data: TestForm{ + data: &TestForm{ RegexPattern: "", }, }, { description: "Valid regex", - data: TestForm{ + data: &TestForm{ RegexPattern: `(\d{1,3})+`, }, }, { description: "Invalid regex", - data: TestForm{ + data: &TestForm{ RegexPattern: "[a-", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"RegexPattern"}, Classification: ErrRegexPattern, Message: getRegexPatternErrorString("[a-"), diff --git a/modules/validation/validurl_test.go b/modules/validation/validurl_test.go index 52f6a191555..d2f0eba3994 100644 --- a/modules/validation/validurl_test.go +++ b/modules/validation/validurl_test.go @@ -5,92 +5,88 @@ package validation import ( "testing" - - "gitea.com/go-chi/binding" ) func Test_ValidURLValidation(t *testing.T) { - AddBindingRules() - urlValidationTestCases := []validationTestCase{ { description: "Empty URL", - data: TestForm{ + data: &TestForm{ URL: "", }, }, { description: "URL without port", - data: TestForm{ + data: &TestForm{ URL: "http://test.lan/", }, }, { description: "URL with port", - data: TestForm{ + data: &TestForm{ URL: "http://test.lan:3000/", }, }, { description: "URL with IPv6 address without port", - data: TestForm{ + data: &TestForm{ URL: "http://[::1]/", }, }, { description: "URL with IPv6 address with port", - data: TestForm{ + data: &TestForm{ URL: "http://[::1]:3000/", }, }, { description: "Invalid URL", - data: TestForm{ + data: &TestForm{ URL: "http//test.lan/", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"URL"}, - Classification: binding.ERR_URL, + Classification: "UrlError", Message: "Url", }, }, }, { description: "Invalid schema", - data: TestForm{ + data: &TestForm{ URL: "ftp://test.lan/", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"URL"}, - Classification: binding.ERR_URL, + Classification: "UrlError", Message: "Url", }, }, }, { description: "Invalid port", - data: TestForm{ + data: &TestForm{ URL: "http://test.lan:3x4/", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"URL"}, - Classification: binding.ERR_URL, + Classification: "UrlError", Message: "Url", }, }, }, { description: "Invalid port with IPv6 address", - data: TestForm{ + data: &TestForm{ URL: "http://[::1]:3x4/", }, - expectedErrors: binding.Errors{ - binding.Error{ + expectedErrors: BindingErrors{ + BindingError{ FieldNames: []string{"URL"}, - Classification: binding.ERR_URL, + Classification: "UrlError", Message: "Url", }, }, diff --git a/modules/web/middleware/binding.go b/modules/web/middleware/binding.go index 829cc699215..1f9321cf786 100644 --- a/modules/web/middleware/binding.go +++ b/modules/web/middleware/binding.go @@ -5,6 +5,7 @@ package middleware import ( + "net/http" "reflect" "strings" @@ -14,7 +15,7 @@ import ( "gitea.dev/modules/util" "gitea.dev/modules/validation" - "gitea.com/go-chi/binding" + "gitea.com/go-chi/binding" //nolint:depguard // this package wraps it ) type ( @@ -23,14 +24,36 @@ type ( ) type Form interface { - Validate(ctx *ValidateContext, errs binding.Errors) binding.Errors + Validate(ctx *ValidateContext, errs validation.BindingErrors) validation.BindingErrors } -func init() { - binding.SetNameMapper(util.ToSnakeCase) +// BindFormAny binds the request to the form of type T and returns the pointer to the form and any binding errors. +// Only the rules defined in the struct field's "binding" tag are applied. +// It can bind to any struct, doesn't call the struct's "Form.Validate" interface. +func BindFormAny[T any](req *http.Request, binder *binding.Binder, _ T) (ret *T, _ validation.BindingErrors) { + typ := reflect.TypeFor[T]() + if typ.Kind() != reflect.Struct { + panic("BindFormAny: template type must be a struct and the function returns its pointer") + } + form := new(T) + errs := binder.Bind(req, form) + return form, errs } -// AssignForm assign form values back to the template data. +// BindFormValidate binds the request to the form of type T which must be a pointer implementing Form interface +// After binding, the Form.Validate is also called so we can do more validation checks +func BindFormValidate[T Form](req *http.Request, binder *binding.Binder) (ret T, _ validation.BindingErrors) { + locale := req.Context().Value(translation.ContextKey).(translation.Locale) //nolint:forcetypeassert // must exist + ptrType := reflect.TypeFor[T]() + structType := ptrType.Elem() + ptrVal := reflect.New(structType) + form := ptrVal.Interface().(Form) //nolint:forcetypeassert // must implement Form + errs := binder.Bind(req, form) + errs = form.Validate(&ValidateContext{Locale: locale}, errs) + return form.(T), errs //nolint:forcetypeassert // must be type T +} + +// AssignForm assign form values back to the template data, the template variable names are in "snake_case" func AssignForm(form any, data map[string]any) { typ := reflect.TypeOf(form) val := reflect.ValueOf(form) @@ -65,12 +88,12 @@ func getRuleBody(field reflect.StructField, ruleName string) string { return "" } -func AddValidationError(errs binding.Errors, fieldName, errorMsg string) binding.Errors { +func AddValidationError(errs validation.BindingErrors, fieldName, errorMsg string) validation.BindingErrors { errs.Add([]string{fieldName}, validation.ErrCustomMessage, errorMsg) return errs } -func getFieldDisplayNameForMessage(f Form, l translation.Locale, fieldNames []string) (field reflect.StructField, ok bool, displayName string) { +func getFieldDisplayNameForMessage(f any, l translation.Locale, fieldNames []string) (field reflect.StructField, ok bool, displayName string) { if len(fieldNames) == 0 { return field, false, "" } @@ -106,7 +129,7 @@ func getFieldDisplayNameForMessage(f Form, l translation.Locale, fieldNames []st return field, true, displayName } -func BuildValidationErrorForUser(f Form, l translation.Locale, bindingErrs binding.Errors) (errorMessage, errorFieldName string, fieldNames []string) { +func BuildValidationErrorForUser(f any, l translation.Locale, bindingErrs validation.BindingErrors) (errorMessage, errorFieldName string, fieldNames []string) { if bindingErrs.Len() == 0 { return "", "", nil } diff --git a/modules/web/middleware/binding_test.go b/modules/web/middleware/binding_test.go index bebe5fdea3d..0cd8dd6c79c 100644 --- a/modules/web/middleware/binding_test.go +++ b/modules/web/middleware/binding_test.go @@ -4,13 +4,11 @@ package middleware import ( - "net/http" - "net/http/httptest" "testing" "gitea.dev/modules/translation" + "gitea.dev/modules/validation" - "gitea.com/go-chi/binding" "github.com/stretchr/testify/assert" ) @@ -22,7 +20,7 @@ type testRangeForm struct { func TestBuildValidationErrorForUser(t *testing.T) { // an out-of-range value must reach its own message instead of the panicking "default" branch form := &testRangeForm{Hours: 2000} - errs := binding.Validate(httptest.NewRequest(http.MethodPost, "/", nil), form) + errs := validation.Binder().Validate(t.Context(), form) errorMessage, errorFieldName, fieldNames := BuildValidationErrorForUser(form, translation.MockLocale{}, errs) assert.Equal(t, "form.range_error:form.Hours,0,1000", errorMessage) assert.Equal(t, "Hours", errorFieldName) diff --git a/modules/web/router.go b/modules/web/router.go index 2e3a90f1cfc..d1384028390 100644 --- a/modules/web/router.go +++ b/modules/web/router.go @@ -15,33 +15,26 @@ import ( "gitea.dev/modules/reqctx" "gitea.dev/modules/setting" "gitea.dev/modules/translation" + "gitea.dev/modules/validation" "gitea.dev/modules/web/middleware" "gitea.dev/modules/web/types" - "gitea.com/go-chi/binding" "github.com/go-chi/chi/v5" ) // Bind binding the request form to a form object and assign context data -func Bind[T interface { - *E - middleware.Form -}, E any]() http.HandlerFunc { +func Bind[T middleware.Form]() http.HandlerFunc { return func(resp http.ResponseWriter, req *http.Request) { + form, errs := middleware.BindFormValidate[T](req, validation.Binder()) + ctx := reqctx.FromContext(req.Context()) data := ctx.GetData() locale := ctx.Value(translation.ContextKey).(translation.Locale) //nolint:forcetypeassert // must exist - - obj := new(E) - var form T = obj - vctx := &middleware.ValidateContext{Locale: locale, Data: data, Req: req, Resp: resp} - errs := binding.Bind(req, obj) - errs = form.Validate(vctx, errs) - SetForm(data, obj) + SetForm(data, form) // Legacy template error handling: try to restore the form's values as much as possible, // especially for RenderWithErrDeprecated to re-render the form with errors. - middleware.AssignForm(obj, data) + middleware.AssignForm(form, data) errorMessage, errorFieldName, _ := middleware.BuildValidationErrorForUser(form, locale, errs) if errorMessage != "" { data["HasError"] = true diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index f15b3b49b5d..b2c932f4af6 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -79,7 +79,9 @@ import ( "gitea.dev/modules/setting" api "gitea.dev/modules/structs" "gitea.dev/modules/util" + "gitea.dev/modules/validation" "gitea.dev/modules/web" + "gitea.dev/modules/web/middleware" "gitea.dev/routers/api/v1/activitypub" "gitea.dev/routers/api/v1/admin" "gitea.dev/routers/api/v1/misc" @@ -99,7 +101,6 @@ import ( _ "gitea.dev/routers/api/v1/swagger" // for swagger generation - "gitea.com/go-chi/binding" chi_middleware "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/cors" ) @@ -884,15 +885,14 @@ func mustEnableAttachments(ctx *context.APIContext) { } // bind binding an obj to a func(ctx *context.APIContext) -func bind[T any](_ T) any { +func bind[T any](tmpl T) any { return func(ctx *context.APIContext) { - theObj := new(T) // create a new form obj for every request but not use obj directly - errs := binding.Bind(ctx.Req, theObj) + form, errs := middleware.BindFormAny(ctx.Req, validation.Binder(), tmpl) if len(errs) > 0 { ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("%s: %s", errs[0].FieldNames, errs[0].Error())) return } - web.SetForm(ctx, theObj) + web.SetForm(ctx, form) } } diff --git a/routers/private/internal.go b/routers/private/internal.go index f229c5b3ec1..2519b4ee3cb 100644 --- a/routers/private/internal.go +++ b/routers/private/internal.go @@ -13,12 +13,12 @@ import ( "gitea.dev/modules/log" "gitea.dev/modules/private" "gitea.dev/modules/setting" + "gitea.dev/modules/validation" "gitea.dev/modules/web" + "gitea.dev/modules/web/middleware" "gitea.dev/routers/common" "gitea.dev/routers/web/misc" "gitea.dev/services/context" - - "gitea.com/go-chi/binding" ) func authInternal(next http.Handler) http.Handler { @@ -42,11 +42,14 @@ func authInternal(next http.Handler) http.Handler { } // bind binding an obj to a handler -func bind[T any](_ T) any { +func bind[T any](tmpl T) any { return func(ctx *context.PrivateContext) { - theObj := new(T) // create a new form obj for every request but not use obj directly - binding.Bind(ctx.Req, theObj) - web.SetForm(ctx, theObj) + form, errs := middleware.BindFormAny(ctx.Req, validation.Binder(), tmpl) + if len(errs) > 0 { + errMsg, _, _ := middleware.BuildValidationErrorForUser(form, ctx.Locale, errs) + ctx.PrivateInternalErrorf("invalid request: %v", errMsg) + } + web.SetForm(ctx, form) } } diff --git a/routers/web/web.go b/routers/web/web.go index bc5d80fd1b0..fcdad7dc65f 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -20,7 +20,6 @@ import ( "gitea.dev/modules/setting" "gitea.dev/modules/storage" "gitea.dev/modules/structs" - "gitea.dev/modules/validation" "gitea.dev/modules/web" "gitea.dev/modules/web/middleware" "gitea.dev/modules/web/routing" @@ -345,8 +344,6 @@ func addProjectBoardRoutes(m *web.Router) { // registerWebRoutes register routes func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { - validation.AddBindingRules() - // middleware: required to be signed in or signed out reqSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: true}) reqSignOut := verifyAuthWithOptions(&common.VerifyOptions{SignOutRequired: true}) diff --git a/services/context/context.go b/services/context/context.go index 6f4cf63cfa7..a1688111c12 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -23,11 +23,10 @@ import ( "gitea.dev/modules/templates" "gitea.dev/modules/translation" "gitea.dev/modules/util" + "gitea.dev/modules/validation" "gitea.dev/modules/web" "gitea.dev/modules/web/middleware" web_types "gitea.dev/modules/web/types" - - "gitea.com/go-chi/binding" ) // Render represents a template render @@ -83,28 +82,6 @@ func GetWebContext(ctx context.Context) *Context { return webCtx } -// GetValidateContext gets a context for middleware form validation -func GetValidateContext(req *http.Request) (ctx *middleware.ValidateContext) { - if ctxAPI, ok := req.Context().Value(apiContextKey).(*APIContext); ok { - ctx = &middleware.ValidateContext{ - Data: ctxAPI.Data, - Locale: ctxAPI.Locale, - Req: ctxAPI.Req, - Resp: ctxAPI.Resp, - } - } else if ctxWeb, ok := req.Context().Value(WebContextKey).(*Context); ok { - ctx = &middleware.ValidateContext{ - Data: ctxWeb.Data, - Locale: ctxWeb.Locale, - Req: ctxWeb.Req, - Resp: ctxWeb.Resp, - } - } else { - panic("invalid context, expect either APIContext or Context") - } - return ctx -} - func NewTemplateContextForWeb(ctx reqctx.RequestContext, req *http.Request, locale translation.Locale) TemplateContext { tmplCtx := NewTemplateContext(ctx, req) tmplCtx["Locale"] = locale @@ -294,21 +271,16 @@ func (ctx *Context) JSONErrorNotFound(optMsg ...string) { ctx.JSON(http.StatusNotFound, buildJsonErrorMap(msg)) } -func GetFetchActionForm[T interface { - *E - middleware.Form -}, E any](ctx *Context) *E { +func GetFetchActionForm[T middleware.Form](ctx *Context) (ret T) { if web.IsFormSet(ctx) { panic("don't mix fetch-action form validation with template-based form validation") } - form := T(new(E)) - errs := binding.Bind(ctx.Req, form) - errs = form.Validate(GetValidateContext(ctx.Req), errs) + form, errs := middleware.BindFormValidate[T](ctx.Req, validation.Binder()) errorMessage, fieldName, _ := middleware.BuildValidationErrorForUser(form, ctx.Locale, errs) if errorMessage != "" { ctx.Resp.Header().Set("Content-Type", "application/json") ctx.JSONErrorWithField(errorMessage, fieldName) - return nil + return ret } return form } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 2929536c318..a53be1869e6 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -12,10 +12,9 @@ import ( "gitea.dev/modules/json" "gitea.dev/modules/structs" "gitea.dev/modules/util" + "gitea.dev/modules/validation" "gitea.dev/modules/web/middleware" "gitea.dev/services/webhook" - - "gitea.com/go-chi/binding" ) // CreateRepoForm form for creating repository @@ -263,7 +262,7 @@ type NewSlackHookForm struct { WebhookForm } -func (f *NewSlackHookForm) Validate(ctx *middleware.ValidateContext, errs binding.Errors) binding.Errors { +func (f *NewSlackHookForm) Validate(ctx *middleware.ValidateContext, errs validation.BindingErrors) validation.BindingErrors { if !webhook.IsValidSlackChannel(strings.TrimSpace(f.Channel)) { errs = middleware.AddValidationError(errs, "Channel", ctx.Locale.TrString("repo.settings.add_webhook.invalid_channel_name")) } @@ -564,7 +563,7 @@ type WikiEditForm struct { Message string } -func (f *WikiEditForm) Validate(ctx *middleware.ValidateContext, errs binding.Errors) binding.Errors { +func (f *WikiEditForm) Validate(ctx *middleware.ValidateContext, errs validation.BindingErrors) validation.BindingErrors { f.Title = strings.TrimSpace(f.Title) if f.Title == "" { errs = middleware.AddValidationError(errs, "title", ctx.Locale.TrString("repo.issues.new.title_empty")) diff --git a/services/forms/user_form.go b/services/forms/user_form.go index bec7ba928b9..bcf28db8173 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -14,36 +14,34 @@ import ( "gitea.dev/modules/util" "gitea.dev/modules/validation" "gitea.dev/modules/web/middleware" - - "gitea.com/go-chi/binding" ) // InstallForm form for installation page type InstallForm struct { middleware.FormDefaultValidator - DbType string `binding:"Required"` - DbHost string - DbUser string + DbType string `binding:"TrimSpace;Required"` + DbHost string `binding:"TrimSpace"` + DbUser string `binding:"TrimSpace"` DbPasswd string - DbName string - SSLMode string - DbPath string - DbSchema string + DbName string `binding:"TrimSpace"` + SSLMode string `binding:"TrimSpace"` + DbPath string `binding:"TrimSpace"` + DbSchema string `binding:"TrimSpace"` - AppName string `binding:"Required" locale:"install.app_name"` - RepoRootPath string `binding:"Required"` - LFSRootPath string - RunUser string `binding:"Required"` - Domain string `binding:"Required"` + AppName string `binding:"TrimSpace;Required" locale:"install.app_name"` + RepoRootPath string `binding:"TrimSpace;Required"` + LFSRootPath string `binding:"TrimSpace"` + RunUser string `binding:"TrimSpace;Required"` + Domain string `binding:"TrimSpace;Required"` SSHPort int - HTTPPort string `binding:"Required"` - AppURL string `binding:"Required"` - LogRootPath string `binding:"Required"` + HTTPPort string `binding:"TrimSpace;Required"` + AppURL string `binding:"TrimSpace;Required"` + LogRootPath string `binding:"TrimSpace;Required"` - SMTPAddr string - SMTPPort string - SMTPFrom string - SMTPUser string `binding:"OmitEmpty;MaxSize(254)" locale:"install.mailer_user"` + SMTPAddr string `binding:"TrimSpace"` + SMTPPort string `binding:"TrimSpace"` + SMTPFrom string `binding:"TrimSpace"` + SMTPUser string `binding:"TrimSpace;OmitEmpty;MaxSize(254)" locale:"install.mailer_user"` SMTPPasswd string RegisterConfirm bool MailNotify bool @@ -58,14 +56,14 @@ type InstallForm struct { DefaultAllowCreateOrganization bool DefaultEnableTimetracking bool EnableUpdateChecker bool - NoReplyAddress string + NoReplyAddress string `binding:"TrimSpace"` - PasswordAlgorithm string + PasswordAlgorithm string `binding:"TrimSpace"` - AdminName string `binding:"OmitEmpty;Username;MaxSize(30)" locale:"install.admin_name"` + AdminName string `binding:"TrimSpace;OmitEmpty;Username;MaxSize(30)" locale:"install.admin_name"` AdminPasswd string `binding:"OmitEmpty;MaxSize(255)" locale:"install.admin_password"` AdminConfirmPasswd string - AdminEmail string `binding:"OmitEmpty;MinSize(3);MaxSize(254);Include(@)" locale:"install.admin_email"` + AdminEmail string `binding:"TrimSpace;OmitEmpty;MinSize(3);MaxSize(254);Include(@)" locale:"install.admin_email"` // ReinstallConfirmFirst we can not use 1/2/3 or A/B/C here, there is a framework bug, can not parse "reinstall_confirm_1" or "reinstall_confirm_a" ReinstallConfirmFirst bool @@ -275,7 +273,7 @@ func DetectInvalidOAuth2ApplicationRedirectURI(uris []string) (invalidURL string return "" } -func (f *EditOAuth2ApplicationForm) Validate(ctx *middleware.ValidateContext, errs binding.Errors) binding.Errors { +func (f *EditOAuth2ApplicationForm) Validate(ctx *middleware.ValidateContext, errs validation.BindingErrors) validation.BindingErrors { invalidURI := DetectInvalidOAuth2ApplicationRedirectURI(util.SplitTrimSpace(f.RedirectURIs, "\n")) if invalidURI != "" { errs = middleware.AddValidationError(errs, "RedirectURIs", "RedirectURIs: "+ctx.Locale.TrString("form.url_error", `"`+invalidURI+`"`))