refactor: http request binding (#38971)

Better than before, still not good enough (more work can be done in the
future)

And add the missing error handling in the PrivateContext "bind"
middleware.

By the way, picked some "TrimSpace" changes from "fix: trim whitespace
from SMTP address and port - #38934" (fix #38926)

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
 	}

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)

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