Skip to content

Commit 61d6ca5

Browse files
authored
Return error on context value type mismatch in bind (#62)
Previously, when a value retrieved from the request context via a `context:"key"` tag could not be converted to the destination field type, the field was silently skipped. Now it returns a new ErrBindContextTypeMismatch error with details about the key, source type, field name, and target type. - Extract context-field binding into a dedicated bindContextField helper. - Missing keys, nil values, and unexported fields remain silent no-ops. - Add test for type mismatch error and test for missing/nil context values.
1 parent 5eb2dcf commit 61d6ca5

2 files changed

Lines changed: 55 additions & 10 deletions

File tree

binding.go

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package fox
33
import (
44
"bytes"
55
"errors"
6+
"fmt"
67
"io"
78
"net/http"
89
"reflect"
@@ -13,6 +14,11 @@ import (
1314
// ErrBindNonPointerValue is required bind pointer
1415
var ErrBindNonPointerValue = errors.New("can not bind to non-pointer value")
1516

17+
// ErrBindContextTypeMismatch is returned when a value retrieved from the
18+
// request context via a `context:"key"` tag cannot be converted to the
19+
// destination field type.
20+
var ErrBindContextTypeMismatch = errors.New("context value type mismatch")
21+
1622
// DefaultBinder default binder
1723
var DefaultBinder binding.Binding = binding.JSON
1824

@@ -33,7 +39,8 @@ var bodyBinders = map[string]binding.BindingBody{
3339
binding.MIMETOML: binding.TOML, // toml
3440
}
3541

36-
// bind request arguments
42+
// bind populates obj from the request: body (per Content-Type), then any
43+
// `context`, `query`, `uri`, and `header` tagged fields, in that order.
3744
func bind(ctx *Context, obj any) error {
3845
vPtr := reflect.ValueOf(obj)
3946

@@ -110,13 +117,8 @@ func bind(ctx *Context, obj any) error {
110117
hasHeaderField = true
111118
}
112119
if tag := field.Tag.Get("context"); tag != "" && tag != "-" {
113-
if value, exists := ctx.Get(tag); exists {
114-
if fieldValue := vPtr.Field(i); fieldValue.CanSet() {
115-
// convert context value to field type and set
116-
if val := reflect.ValueOf(value); val.Type().ConvertibleTo(fieldValue.Type()) {
117-
fieldValue.Set(val.Convert(fieldValue.Type()))
118-
}
119-
}
120+
if err := bindContextField(ctx, vPtr.Field(i), field.Name, tag); err != nil {
121+
return err
120122
}
121123
}
122124
}
@@ -153,6 +155,26 @@ func bind(ctx *Context, obj any) error {
153155
return nil
154156
}
155157

158+
// bindContextField copies a value stored on ctx into a struct field tagged
159+
// with `context:"key"`. Missing keys, unexported fields and nil values are
160+
// no-ops; an unconvertible stored type returns ErrBindContextTypeMismatch.
161+
func bindContextField(ctx *Context, fieldValue reflect.Value, fieldName, key string) error {
162+
value, exists := ctx.Get(key)
163+
if !exists || value == nil {
164+
return nil
165+
}
166+
if !fieldValue.CanSet() {
167+
return nil
168+
}
169+
val := reflect.ValueOf(value)
170+
if !val.Type().ConvertibleTo(fieldValue.Type()) {
171+
return fmt.Errorf("%w: key %q (%T) -> field %s (%s)",
172+
ErrBindContextTypeMismatch, key, value, fieldName, fieldValue.Type())
173+
}
174+
fieldValue.Set(val.Convert(fieldValue.Type()))
175+
return nil
176+
}
177+
156178
type queryBinding struct{}
157179

158180
func (queryBinding) Name() string {

binding_test.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,9 +548,32 @@ func TestBind_ContextFieldNotConvertible(t *testing.T) {
548548

549549
var obj TestStruct
550550
err := bind(ctx, &obj)
551-
// Should not error, just skip the field
551+
require.Error(t, err)
552+
require.ErrorIs(t, err, ErrBindContextTypeMismatch)
553+
assert.Contains(t, err.Error(), "int_value")
554+
assert.Equal(t, 0, obj.IntValue)
555+
}
556+
557+
// TestBind_ContextFieldMissing verifies that a missing or nil context value
558+
// leaves the destination field at its zero value without raising an error.
559+
func TestBind_ContextFieldMissing(t *testing.T) {
560+
type TestStruct struct {
561+
Missing string `context:"missing_key"`
562+
Nilled string `context:"nilled_key"`
563+
}
564+
565+
req, _ := http.NewRequest(http.MethodGet, "/", nil)
566+
ctx := &Context{
567+
Context: &gin.Context{Request: req},
568+
Request: req,
569+
}
570+
ctx.Set("nilled_key", nil)
571+
572+
var obj TestStruct
573+
err := bind(ctx, &obj)
552574
require.NoError(t, err)
553-
assert.Equal(t, 0, obj.IntValue) // Should remain zero value
575+
assert.Empty(t, obj.Missing)
576+
assert.Empty(t, obj.Nilled)
554577
}
555578

556579
// TestBind_WithBindingError tests bind with body binding error

0 commit comments

Comments
 (0)