From f261adb53fbebea107dbd8b85ad6a7478f0fa267 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 19 Aug 2026 20:50:06 +0800 Subject: [PATCH] chore: form binding trim space (#38978) Use "binding:TrimSpace" instead of fragile IsEmptyString And fix a bug in locale's `HasKey`: it should also try the default language if current language doesn't have the translation key, a new test is added. --- go.mod | 2 +- go.sum | 4 ++-- modules/structs/project.go | 4 ++-- modules/structs/repo_wiki.go | 2 +- modules/translation/i18n/i18n_test.go | 4 ++++ modules/translation/i18n/localestore.go | 3 +++ modules/util/util.go | 6 ------ modules/util/util_test.go | 16 ---------------- modules/validation/binding.go | 3 +-- options/locale/locale_en-US.json | 2 +- routers/api/v1/repo/wiki.go | 7 ------- routers/api/v1/shared/project.go | 8 -------- routers/web/admin/auths.go | 3 +-- routers/web/repo/issue_new.go | 5 ----- routers/web/repo/pull.go | 5 ----- services/forms/auth_form.go | 2 +- services/forms/repo_form.go | 16 ++++------------ 17 files changed, 21 insertions(+), 71 deletions(-) diff --git a/go.mod b/go.mod index 26731e6c595..0e838186816 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-20260818211407-ac8602c87be9 + gitea.com/go-chi/binding v0.0.0-20260819122636-082915a69981 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 f8cbc120b93..bb53bc7f87c 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-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/binding v0.0.0-20260819122636-082915a69981 h1:LmdlwGbzgZFZA3bK3R1q8QrNabaSz3KpnOJBLmzhN6E= +gitea.com/go-chi/binding v0.0.0-20260819122636-082915a69981/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/structs/project.go b/modules/structs/project.go index 4e85ad77971..6935315c25c 100644 --- a/modules/structs/project.go +++ b/modules/structs/project.go @@ -58,7 +58,7 @@ type CreateProjectOption struct { // EditProjectOption represents options for editing a project // swagger:model type EditProjectOption struct { - Title *string `json:"title,omitempty"` + Title *string `json:"title,omitempty" binding:"TrimSpace;Required"` Description *string `json:"description,omitempty"` // Card type: "text_only" or "images_and_text" CardType *string `json:"card_type,omitempty"` @@ -94,7 +94,7 @@ type CreateProjectColumnOption struct { // EditProjectColumnOption represents options for editing a project column // swagger:model type EditProjectColumnOption struct { - Title *string `json:"title,omitempty"` + Title *string `json:"title,omitempty" binding:"TrimSpace;Required"` // Column color in 6-digit hex format, e.g. #FF0000 Color *string `json:"color,omitempty"` // Position of the column within the project, between -128 and 127 diff --git a/modules/structs/repo_wiki.go b/modules/structs/repo_wiki.go index 1944c1a3f7a..4c5cd365a8b 100644 --- a/modules/structs/repo_wiki.go +++ b/modules/structs/repo_wiki.go @@ -43,7 +43,7 @@ type WikiPageMetaData struct { // CreateWikiPageOptions form for creating wiki type CreateWikiPageOptions struct { // page title. leave empty to keep unchanged - Title string `json:"title"` + Title string `json:"title" binding:"TrimSpace;Required"` // content must be base64 encoded ContentBase64 string `json:"content_base64"` // optional commit message summarizing the change diff --git a/modules/translation/i18n/i18n_test.go b/modules/translation/i18n/i18n_test.go index 0ef1f1b3888..871be6ac54f 100644 --- a/modules/translation/i18n/i18n_test.go +++ b/modules/translation/i18n/i18n_test.go @@ -16,6 +16,7 @@ func TestLocaleStore(t *testing.T) { { ".dot.name": "Dot Name", "fmt": "%[1]s %[2]s", +"only_in_default": "", "section.sub": "Sub String", "section.mixed": "test value; %s" @@ -60,6 +61,9 @@ func TestLocaleStore(t *testing.T) { found := lang1.HasKey("no-such") assert.False(t, found) + found = lang2.HasKey("only_in_default") + assert.True(t, found) + assert.NoError(t, ls.Close()) res := lang1.TrHTML("") diff --git a/modules/translation/i18n/localestore.go b/modules/translation/i18n/localestore.go index ef6b90b6dad..5504d2f6223 100644 --- a/modules/translation/i18n/localestore.go +++ b/modules/translation/i18n/localestore.go @@ -177,5 +177,8 @@ func (l *locale) HasKey(trKey string) bool { return false } _, ok = l.idxToMsgMap[idx] + if !ok { + _, ok = l.store.localeMap[l.store.defaultLang] + } return ok } diff --git a/modules/util/util.go b/modules/util/util.go index 19f458fb35f..7df08f40c5c 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -13,7 +13,6 @@ import ( rand2 "math/rand/v2" "slices" "strconv" - "strings" "sync" "gitea.dev/modules/container" @@ -22,11 +21,6 @@ import ( "golang.org/x/text/language" ) -// IsEmptyString checks if the provided string is empty -func IsEmptyString(s string) bool { - return len(strings.TrimSpace(s)) == 0 -} - // ParseYamlBool parses YAML 1.2 boolean values into bool func ParseYamlBool(s string) bool { return s == "true" || s == "True" || s == "TRUE" diff --git a/modules/util/util_test.go b/modules/util/util_test.go index 7dbb14e374b..50daaedaf8f 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -11,22 +11,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestIsEmptyString(t *testing.T) { - cases := []struct { - s string - expected bool - }{ - {"", true}, - {" ", true}, - {" ", true}, - {" a", false}, - } - - for _, v := range cases { - assert.Equal(t, v.expected, IsEmptyString(v.s)) - } -} - func Test_NormalizeEOL(t *testing.T) { data1 := []string{ "", diff --git a/modules/validation/binding.go b/modules/validation/binding.go index c93771dc71b..18903423e5d 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -50,7 +50,6 @@ func newFieldError(field reflect.StructField, cls, msg string) *BindingError { // AddBindingRules adds additional binding rules func AddBindingRules(b *binding.Binder) { - binding.JSONProvider = jsonProvider{} b.AddRuleNonZero("GitRefName", func(ctx context.Context, f *binding.ValidationField) *binding.Error { if !git.IsValidRefPattern(f.ValueMustString()) { return newFieldError(f.StructField, ErrGitRefName, "GitRefName") @@ -140,7 +139,7 @@ func validPort(p string) bool { } var Binder = sync.OnceValue(func() *binding.Binder { - b := binding.NewBinder().WithDefaultRules().WithNameMapper(util.ToSnakeCase) + b := binding.NewBinder().WithJSONProvider(jsonProvider{}).WithDefaultRules().WithNameMapper(util.ToSnakeCase) AddBindingRules(b) return b }) diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 11cc9ec5199..8cad6905e7c 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -532,6 +532,7 @@ "form.CommitMessage": "Commit message", "form.CommitChoice": "Commit choice", "form.TreeName": "File path", + "form.Title": "Title", "form.Content": "Content", "form.SSPISeparatorReplacement": "Separator", "form.SSPIDefaultLanguage": "Default Language", @@ -1418,7 +1419,6 @@ "repo.issues.filter_no_results": "No results", "repo.issues.filter_no_results_placeholder": "Try adjusting your search filters.", "repo.issues.new": "New Issue", - "repo.issues.new.title_empty": "Title cannot be empty", "repo.issues.new.labels": "Labels", "repo.issues.new.no_labels": "No labels", "repo.issues.new.clear_labels": "Clear labels", diff --git a/routers/api/v1/repo/wiki.go b/routers/api/v1/repo/wiki.go index 0193defaab1..6c9cc0e8cfd 100644 --- a/routers/api/v1/repo/wiki.go +++ b/routers/api/v1/repo/wiki.go @@ -13,7 +13,6 @@ import ( "gitea.dev/modules/git" "gitea.dev/modules/setting" api "gitea.dev/modules/structs" - "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/services/context" "gitea.dev/services/convert" @@ -56,12 +55,6 @@ func NewWikiPage(ctx *context.APIContext) { // "$ref": "#/responses/repoArchivedError" form := web.GetForm[*api.CreateWikiPageOptions](ctx) - - if util.IsEmptyString(form.Title) { - ctx.APIError(http.StatusBadRequest, "title is required") - return - } - wikiName := wiki_service.UserTitleToWebPath("", form.Title) if len(form.Message) == 0 { diff --git a/routers/api/v1/shared/project.go b/routers/api/v1/shared/project.go index a4e2624c159..bd196eb625d 100644 --- a/routers/api/v1/shared/project.go +++ b/routers/api/v1/shared/project.go @@ -612,10 +612,6 @@ func EditProject(ctx *context.APIContext) { } form := web.GetForm[*api.EditProjectOption](ctx) - if form.Title != nil && util.IsEmptyString(*form.Title) { - ctx.APIError(http.StatusUnprocessableEntity, "title must not be empty") - return - } opts := project_service.UpdateProjectOptions{ Title: optional.FromPtr(form.Title), Description: optional.FromPtr(form.Description), @@ -1187,10 +1183,6 @@ func EditProjectColumn(ctx *context.APIContext) { form := web.GetForm[*api.EditProjectColumnOption](ctx) if form.Title != nil { - if util.IsEmptyString(*form.Title) { - ctx.APIError(http.StatusUnprocessableEntity, "title must not be empty") - return - } column.Title = *form.Title } if form.Color != nil { diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index 5eef4a54062..e0620dfc25a 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -18,7 +18,6 @@ import ( "gitea.dev/modules/log" "gitea.dev/modules/setting" "gitea.dev/modules/templates" - "gitea.dev/modules/util" "gitea.dev/modules/web" auth_service "gitea.dev/services/auth" "gitea.dev/services/auth/source/ldap" @@ -210,7 +209,7 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source { } func parseSSPIConfig(ctx *context.Context, form forms.AuthenticationForm) (*sspi.Source, error) { - if util.IsEmptyString(form.SSPISeparatorReplacement) { + if form.SSPISeparatorReplacement == "" { ctx.Data["Err_SSPISeparatorReplacement"] = true return nil, errors.New(ctx.Locale.TrString("form.require_error", ctx.Locale.TrString("form.SSPISeparatorReplacement"))) } diff --git a/routers/web/repo/issue_new.go b/routers/web/repo/issue_new.go index bfcb73def45..efbf6354bc1 100644 --- a/routers/web/repo/issue_new.go +++ b/routers/web/repo/issue_new.go @@ -351,11 +351,6 @@ func NewIssuePost(ctx *context.Context) { return } - if util.IsEmptyString(form.Title) { - ctx.JSONError(ctx.Tr("repo.issues.new.title_empty")) - return - } - content := form.Content if filename := ctx.Req.Form.Get("template-file"); filename != "" { if template, err := issue_template.UnmarshalFromRepo(ctx, ctx.Repo.GitRepo, ctx.Repo.Repository.DefaultBranch, filename); err == nil { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index fb39d34a5cc..8ee80a8b3fe 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1353,11 +1353,6 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - if util.IsEmptyString(form.Title) { - ctx.JSONError(ctx.Tr("repo.issues.new.title_empty")) - return - } - // Check if a pull request already exists with the same head and base branch. pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, repo.ID, ci.HeadRef.ShortName(), ci.BaseRef.ShortName(), issues_model.PullRequestFlowGithub) if err != nil && !issues_model.IsErrPullRequestNotExist(err) { diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index 0875a0f81d2..da328316762 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -87,6 +87,6 @@ type AuthenticationForm struct { SSPIAutoCreateUsers bool SSPIAutoActivateUsers bool SSPIStripDomainNames bool - SSPISeparatorReplacement string `binding:"AlphaDashDot;MaxSize(5)"` + SSPISeparatorReplacement string `binding:"TrimSpace;AlphaDashDot;MaxSize(5)"` SSPIDefaultLanguage string } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index a53be1869e6..1a0722d9f14 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -336,7 +336,7 @@ type NewPackagistHookForm struct { // CreateIssueForm form for creating issue type CreateIssueForm struct { middleware.FormDefaultValidator - Title string `binding:"Required;MaxSize(255)"` + Title string `binding:"TrimSpace;Required;MaxSize(255)"` AssigneeIDs string `form:"assignee_ids"` ReviewerIDs string `form:"reviewer_ids"` Ref string `form:"ref"` @@ -550,7 +550,7 @@ type GenerateReleaseNotesForm struct { // EditReleaseForm form for changing release type EditReleaseForm struct { middleware.FormDefaultValidator - Title string `form:"title" binding:"Required;MaxSize(255)"` + Title string `form:"title" binding:"TrimSpace;Required;MaxSize(255)"` Content string `form:"content"` Draft string `form:"draft"` Prerelease bool `form:"prerelease"` @@ -558,20 +558,12 @@ type EditReleaseForm struct { } type WikiEditForm struct { - Title string + middleware.FormDefaultValidator + Title string `binding:"TrimSpace;Required"` Content string Message string } -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")) - } - f.Message = strings.TrimSpace(f.Message) - return errs -} - // AddTimeManuallyForm form that adds spent time manually. type AddTimeManuallyForm struct { middleware.FormDefaultValidator