Skip to content

Commit ec9515a

Browse files
vishrclaude
andauthored
fix(binder): include field name in form/struct bind conversion errors (#2629) (#3005)
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) <noreply@anthropic.com>
1 parent dba8ff6 commit ec9515a

3 files changed

Lines changed: 41 additions & 8 deletions

File tree

bind.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"encoding"
88
"encoding/xml"
99
"errors"
10+
"fmt"
1011
"mime/multipart"
1112
"net/http"
1213
"reflect"
@@ -249,15 +250,15 @@ func bindData(destination any, data map[string][]string, tag string, dataFiles m
249250
// try unmarshalling first, in case we're dealing with an alias to an array type
250251
if ok, err := unmarshalInputsToField(typeField.Type.Kind(), inputValue, structField); ok {
251252
if err != nil {
252-
return err
253+
return fmt.Errorf("%s: %w", inputFieldName, err)
253254
}
254255
continue
255256
}
256257

257258
formatTag := typeField.Tag.Get("format")
258259
if ok, err := unmarshalInputToField(typeField.Type.Kind(), inputValue[0], structField, formatTag); ok {
259260
if err != nil {
260-
return err
261+
return fmt.Errorf("%s: %w", inputFieldName, err)
261262
}
262263
continue
263264
}
@@ -275,15 +276,15 @@ func bindData(destination any, data map[string][]string, tag string, dataFiles m
275276
slice := reflect.MakeSlice(structField.Type(), numElems, numElems)
276277
for j := range numElems {
277278
if err := setWithProperType(sliceOf, inputValue[j], slice.Index(j)); err != nil {
278-
return err
279+
return fmt.Errorf("%s: %w", inputFieldName, err)
279280
}
280281
}
281282
structField.Set(slice)
282283
continue
283284
}
284285

285286
if err := setWithProperType(structFieldKind, inputValue[0], structField); err != nil {
286-
return err
287+
return fmt.Errorf("%s: %w", inputFieldName, err)
287288
}
288289
}
289290
return nil

bind_field_error_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// SPDX-License-Identifier: MIT
2+
// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors
3+
4+
package echo
5+
6+
import (
7+
"net/http"
8+
"net/http/httptest"
9+
"strings"
10+
"testing"
11+
12+
"github.com/stretchr/testify/assert"
13+
)
14+
15+
// Regression test for #2629: when binding form data fails a type conversion, the
16+
// returned error must identify which field failed (so applications can render a
17+
// useful message), instead of a bare strconv error with no field context.
18+
func TestBind_formConversionErrorIncludesFieldName(t *testing.T) {
19+
e := New()
20+
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("number=10a"))
21+
req.Header.Set(HeaderContentType, MIMEApplicationForm)
22+
c := e.NewContext(req, httptest.NewRecorder())
23+
24+
type DTO struct {
25+
Number int `form:"number"`
26+
}
27+
var dto DTO
28+
err := c.Bind(&dto)
29+
30+
assert.Error(t, err)
31+
assert.ErrorContains(t, err, "number", "bind error must identify the failing field")
32+
}

bind_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
842842
givenURL: "/api/real_node/endpoint?id=nope",
843843
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
844844
expect: &Opts{ID: 0, Node: "node_from_path"}, // path params binding has already modified bind target
845-
expectError: `code=400, message=Bad Request, err=strconv.ParseInt: parsing "nope": invalid syntax`,
845+
expectError: `code=400, message=Bad Request, err=id: strconv.ParseInt: parsing "nope": invalid syntax`,
846846
},
847847
{
848848
name: "nok, GET body bind failure - trying to bind json array to struct",
@@ -1196,7 +1196,7 @@ func TestBindUnmarshalParamExtras(t *testing.T) {
11961196
}{}
11971197
err := testBindURL("/?t=xxxx", &result)
11981198

1199-
assert.EqualError(t, err, `code=400, message=Bad Request, err='xxxx' is not an integer`)
1199+
assert.EqualError(t, err, `code=400, message=Bad Request, err=t: 'xxxx' is not an integer`)
12001200
})
12011201

12021202
t.Run("ok, target is struct", func(t *testing.T) {
@@ -1301,7 +1301,7 @@ func TestBindUnmarshalParams(t *testing.T) {
13011301
}{}
13021302
err := testBindURL("/?t=xxxx", &result)
13031303

1304-
assert.EqualError(t, err, "code=400, message=Bad Request, err='xxxx' is not an integer")
1304+
assert.EqualError(t, err, "code=400, message=Bad Request, err=t: 'xxxx' is not an integer")
13051305
})
13061306

13071307
t.Run("ok, target is struct", func(t *testing.T) {
@@ -1368,7 +1368,7 @@ func TestBindInt8(t *testing.T) {
13681368
}
13691369
p := target{}
13701370
err := testBindURL("/?v=x&v=2", &p)
1371-
assert.EqualError(t, err, `code=400, message=Bad Request, err=strconv.ParseInt: parsing "x": invalid syntax`)
1371+
assert.EqualError(t, err, `code=400, message=Bad Request, err=v: strconv.ParseInt: parsing "x": invalid syntax`)
13721372
})
13731373

13741374
t.Run("nok, int8 embedded in struct", func(t *testing.T) {

0 commit comments

Comments
 (0)