From 3f386c71edc437d86302533cef5208052f1249e0 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sat, 13 Jun 2026 13:09:58 -0700 Subject: [PATCH] fix(binder): include field name in form/struct bind conversion errors (#2629) bindData had the field name (inputFieldName) in scope but returned bare conversion errors, so c.Bind() failures gave no indication of which field failed (e.g. "strconv.ParseInt: parsing \"10a\"..."). Wrap the conversion errors with the field name using %w so errors.Is/As still work. Matches the wrap proposed by the reporter in the issue thread. Existing tests that asserted the bare message are updated to the field-prefixed form. Co-Authored-By: Claude Opus 4.8 (1M context) --- bind.go | 9 +++++---- bind_field_error_test.go | 32 ++++++++++++++++++++++++++++++++ bind_test.go | 8 ++++---- 3 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 bind_field_error_test.go diff --git a/bind.go b/bind.go index dd01d3203..d320e2ba4 100644 --- a/bind.go +++ b/bind.go @@ -7,6 +7,7 @@ import ( "encoding" "encoding/xml" "errors" + "fmt" "mime/multipart" "net/http" "reflect" @@ -249,7 +250,7 @@ func bindData(destination any, data map[string][]string, tag string, dataFiles m // try unmarshalling first, in case we're dealing with an alias to an array type if ok, err := unmarshalInputsToField(typeField.Type.Kind(), inputValue, structField); ok { if err != nil { - return err + return fmt.Errorf("%s: %w", inputFieldName, err) } continue } @@ -257,7 +258,7 @@ func bindData(destination any, data map[string][]string, tag string, dataFiles m formatTag := typeField.Tag.Get("format") if ok, err := unmarshalInputToField(typeField.Type.Kind(), inputValue[0], structField, formatTag); ok { if err != nil { - return err + return fmt.Errorf("%s: %w", inputFieldName, err) } continue } @@ -275,7 +276,7 @@ func bindData(destination any, data map[string][]string, tag string, dataFiles m slice := reflect.MakeSlice(structField.Type(), numElems, numElems) for j := range numElems { if err := setWithProperType(sliceOf, inputValue[j], slice.Index(j)); err != nil { - return err + return fmt.Errorf("%s: %w", inputFieldName, err) } } structField.Set(slice) @@ -283,7 +284,7 @@ func bindData(destination any, data map[string][]string, tag string, dataFiles m } if err := setWithProperType(structFieldKind, inputValue[0], structField); err != nil { - return err + return fmt.Errorf("%s: %w", inputFieldName, err) } } return nil diff --git a/bind_field_error_test.go b/bind_field_error_test.go new file mode 100644 index 000000000..8bf63b58d --- /dev/null +++ b/bind_field_error_test.go @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors + +package echo + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// Regression test for #2629: when binding form data fails a type conversion, the +// returned error must identify which field failed (so applications can render a +// useful message), instead of a bare strconv error with no field context. +func TestBind_formConversionErrorIncludesFieldName(t *testing.T) { + e := New() + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("number=10a")) + req.Header.Set(HeaderContentType, MIMEApplicationForm) + c := e.NewContext(req, httptest.NewRecorder()) + + type DTO struct { + Number int `form:"number"` + } + var dto DTO + err := c.Bind(&dto) + + assert.Error(t, err) + assert.ErrorContains(t, err, "number", "bind error must identify the failing field") +} diff --git a/bind_test.go b/bind_test.go index b23c638f9..e33298b39 100644 --- a/bind_test.go +++ b/bind_test.go @@ -842,7 +842,7 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) { givenURL: "/api/real_node/endpoint?id=nope", givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`), expect: &Opts{ID: 0, Node: "node_from_path"}, // path params binding has already modified bind target - expectError: `code=400, message=Bad Request, err=strconv.ParseInt: parsing "nope": invalid syntax`, + expectError: `code=400, message=Bad Request, err=id: strconv.ParseInt: parsing "nope": invalid syntax`, }, { name: "nok, GET body bind failure - trying to bind json array to struct", @@ -1196,7 +1196,7 @@ func TestBindUnmarshalParamExtras(t *testing.T) { }{} err := testBindURL("/?t=xxxx", &result) - assert.EqualError(t, err, `code=400, message=Bad Request, err='xxxx' is not an integer`) + assert.EqualError(t, err, `code=400, message=Bad Request, err=t: 'xxxx' is not an integer`) }) t.Run("ok, target is struct", func(t *testing.T) { @@ -1301,7 +1301,7 @@ func TestBindUnmarshalParams(t *testing.T) { }{} err := testBindURL("/?t=xxxx", &result) - assert.EqualError(t, err, "code=400, message=Bad Request, err='xxxx' is not an integer") + assert.EqualError(t, err, "code=400, message=Bad Request, err=t: 'xxxx' is not an integer") }) t.Run("ok, target is struct", func(t *testing.T) { @@ -1368,7 +1368,7 @@ func TestBindInt8(t *testing.T) { } p := target{} err := testBindURL("/?v=x&v=2", &p) - assert.EqualError(t, err, `code=400, message=Bad Request, err=strconv.ParseInt: parsing "x": invalid syntax`) + assert.EqualError(t, err, `code=400, message=Bad Request, err=v: strconv.ParseInt: parsing "x": invalid syntax`) }) t.Run("nok, int8 embedded in struct", func(t *testing.T) {