diff --git a/.github/workflows/gin.yml b/.github/workflows/gin.yml index 8857bebc..2d633f52 100644 --- a/.github/workflows/gin.yml +++ b/.github/workflows/gin.yml @@ -41,6 +41,7 @@ jobs: '--ldflags="-checklinkname=0" -tags sonic', "-tags go_json", "-race", + "-tags gin_bind_encoding", ] include: - os: ubuntu-latest diff --git a/binding/form_mapping.go b/binding/form_mapping.go index 6982fd4f..bda7da5e 100644 --- a/binding/form_mapping.go +++ b/binding/form_mapping.go @@ -15,6 +15,7 @@ import ( "strings" "time" + bindingcodec "github.com/gin-gonic/gin/codec/binding" "github.com/gin-gonic/gin/codec/json" "github.com/gin-gonic/gin/internal/bytesconv" ) @@ -185,17 +186,6 @@ type BindUnmarshaler interface { UnmarshalParam(param string) error } -// trySetCustom tries to set a custom type value -// If the value implements the BindUnmarshaler interface, it will be used to set the value, we will return `true` -// to skip the default value setting. -func trySetCustom(val string, value reflect.Value) (isSet bool, err error) { - switch v := value.Addr().Interface().(type) { - case BindUnmarshaler: - return true, v.UnmarshalParam(val) - } - return false, nil -} - // trySetUsingParser tries to set a custom type value based on the presence of the "parser" tag on the field. // If the parser tag does not exist or does not match any of the supported parsers, gin will skip over this. func trySetUsingParser(val string, value reflect.Value, parser string) (isSet bool, err error) { @@ -265,7 +255,7 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][ if ok, err = trySetUsingParser(vs[0], value, opt.parser); ok { return ok, err - } else if ok, err = trySetCustom(vs[0], value); ok { + } else if ok, err = bindingcodec.API.TrySetByInterface(vs[0], value); ok { return ok, err } @@ -290,7 +280,7 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][ if ok, err = trySetUsingParser(vs[0], value, opt.parser); ok { return ok, err - } else if ok, err = trySetCustom(vs[0], value); ok { + } else if ok, err = bindingcodec.API.TrySetByInterface(vs[0], value); ok { return ok, err } @@ -313,7 +303,7 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][ if ok, err = trySetUsingParser(val, value, opt.parser); ok { return ok, err - } else if ok, err = trySetCustom(val, value); ok { + } else if ok, err = bindingcodec.API.TrySetByInterface(val, value); ok { return ok, err } return true, setWithProperType(val, value, field, opt) @@ -324,7 +314,7 @@ func setWithProperType(val string, value reflect.Value, field reflect.StructFiel // this if-check is required for parsing nested types like []MyId, where MyId is [12]byte if ok, err := trySetUsingParser(val, value, opt.parser); ok { return err - } else if ok, err = trySetCustom(val, value); ok { + } else if ok, err = bindingcodec.API.TrySetByInterface(val, value); ok { return err } diff --git a/binding/form_mapping_default_test.go b/binding/form_mapping_default_test.go new file mode 100644 index 00000000..0f458cd2 --- /dev/null +++ b/binding/form_mapping_default_test.go @@ -0,0 +1,31 @@ +//go:build !gin_bind_encoding + +package binding + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// If someone does not specify parser=TextUnmarshaler even when it's defined for the type, gin should ignore the +// UnmarshalText logic and continue using its default binding logic. (This ensures gin does not break backwards +// compatibility) +// +// Note: TestMappingUsingBindUnmarshalerAndTextUnmarshalerWhenOnlyTextUnmarshalerDefined works differently when: +// - form_mapping_encoding_test.go (with gin_bind_encoding build tag enabled) +func TestMappingUsingBindUnmarshalerAndTextUnmarshalerWhenOnlyTextUnmarshalerDefined(t *testing.T) { + var s struct { + Hex customUnmarshalTextHex `form:"hex"` + HexByUnmarshalText customUnmarshalTextHex `form:"hex2,parser=encoding.TextUnmarshaler"` + } + err := mappingByPtr(&s, formSource{ + "hex": {`11`}, + "hex2": {`11`}, + }, "form") + require.NoError(t, err) + + assert.EqualValues(t, 11, s.Hex) // this is using default int binding, not our custom hex binding. 0x11 should be 17 in decimal + assert.EqualValues(t, 0x11, s.HexByUnmarshalText) // correct expected value for normal hex binding +} diff --git a/binding/form_mapping_encoding_test.go b/binding/form_mapping_encoding_test.go new file mode 100644 index 00000000..9337a3da --- /dev/null +++ b/binding/form_mapping_encoding_test.go @@ -0,0 +1,121 @@ +//go:build gin_bind_encoding + +package binding + +import ( + "encoding" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// In gin_bind_encoding mode, TextUnmarshaler is used automatically when present, even without an +// explicit parser tag. +func TestMappingUsingBindUnmarshalerAndTextUnmarshalerWhenOnlyTextUnmarshalerDefined_DefaultEncodingUnmarshalText(t *testing.T) { + var s struct { + Hex customUnmarshalTextHex `form:"hex"` + HexByUnmarshalText customUnmarshalTextHex `form:"hex2,parser=encoding.TextUnmarshaler"` + } + err := mappingByPtr(&s, formSource{ + "hex": {`11`}, + "hex2": {`11`}, + }, "form") + require.NoError(t, err) + + assert.EqualValues(t, 0x11, s.Hex) + assert.EqualValues(t, 0x11, s.HexByUnmarshalText) +} + +// ==== Automatic TextUnmarshaler binding tests (no parser tag required) ==== + +func TestMappingTextUnmarshalerAutoBindForm(t *testing.T) { + var s struct { + ID objectIDUnmarshalText `form:"id"` + } + err := mappingByPtr(&s, formSource{"id": {"664a062ac74a8ad104e0e80f"}}, "form") + require.NoError(t, err) + expected, _ := convertToOidUnmarshalText("664a062ac74a8ad104e0e80f") + assert.Equal(t, expected, s.ID) +} + +func TestMappingTextUnmarshalerAutoBindURI(t *testing.T) { + var s struct { + ID objectIDUnmarshalText `uri:"id"` + } + err := mappingByPtr(&s, formSource{"id": {"664a062ac74a8ad104e0e80f"}}, "uri") + require.NoError(t, err) + expected, _ := convertToOidUnmarshalText("664a062ac74a8ad104e0e80f") + assert.Equal(t, expected, s.ID) +} + +func TestMappingTextUnmarshalerAutoBindSlice(t *testing.T) { + var s struct { + IDs []objectIDUnmarshalText `form:"ids" collection_format:"csv"` + } + err := mappingByPtr(&s, formSource{"ids": {"664a062ac74a8ad104e0e80e,664a062ac74a8ad104e0e80f"}}, "form") + require.NoError(t, err) + id1, _ := convertToOidUnmarshalText("664a062ac74a8ad104e0e80e") + id2, _ := convertToOidUnmarshalText("664a062ac74a8ad104e0e80f") + expected := []objectIDUnmarshalText{id1, id2} + assert.Equal(t, expected, s.IDs) +} + +func TestMappingTextUnmarshalerAutoBindMultipleValues(t *testing.T) { + var s struct { + IDs []objectIDUnmarshalText `form:"ids"` + } + err := mappingByPtr(&s, formSource{"ids": { + "664a062ac74a8ad104e0e80e", + "664a062ac74a8ad104e0e80f", + }}, "form") + require.NoError(t, err) + id1, _ := convertToOidUnmarshalText("664a062ac74a8ad104e0e80e") + id2, _ := convertToOidUnmarshalText("664a062ac74a8ad104e0e80f") + assert.Equal(t, []objectIDUnmarshalText{id1, id2}, s.IDs) +} + +func TestMappingTextUnmarshalerAutoBindDefault(t *testing.T) { + var s struct { + ID objectIDUnmarshalText `form:"id,default=664a062ac74a8ad104e0e80f"` + } + err := mappingByPtr(&s, formSource{}, "form") + require.NoError(t, err) + expected, _ := convertToOidUnmarshalText("664a062ac74a8ad104e0e80f") + assert.Equal(t, expected, s.ID) +} + +func TestMappingTextUnmarshalerAutoBindInvalidValue(t *testing.T) { + var s struct { + ID objectIDUnmarshalText `form:"id"` + } + err := mappingByPtr(&s, formSource{"id": {"not-a-valid-objectid"}}, "form") + require.Error(t, err) +} + +// BindUnmarshaler should take precedence over TextUnmarshaler +type testDualUnmarshaler struct { + Value string +} + +func (d *testDualUnmarshaler) UnmarshalParam(param string) error { + d.Value = "param:" + param + return nil +} + +func (d *testDualUnmarshaler) UnmarshalText(text []byte) error { + d.Value = "text:" + string(text) + return nil +} + +var _ BindUnmarshaler = (*testDualUnmarshaler)(nil) +var _ encoding.TextUnmarshaler = (*testDualUnmarshaler)(nil) + +func TestMappingBindUnmarshalerTakesPrecedenceOverTextUnmarshaler(t *testing.T) { + var s struct { + Field testDualUnmarshaler `form:"field"` + } + err := mappingByPtr(&s, formSource{"field": {"hello"}}, "form") + require.NoError(t, err) + assert.Equal(t, "param:hello", s.Field.Value) // BindUnmarshaler wins +} diff --git a/binding/form_mapping_test.go b/binding/form_mapping_test.go index c78f7398..9c7502e4 100644 --- a/binding/form_mapping_test.go +++ b/binding/form_mapping_test.go @@ -1016,24 +1016,6 @@ func TestMappingUsingBindUnmarshalerAndTextUnmarshalerWhenOnlyBindUnmarshalerDef assert.EqualValues(t, 0xf5, s.HexByUnmarshalText) // reverts to BindUnmarshaler binding } -// If someone does not specify parser=TextUnmarshaler even when it's defined for the type, gin should ignore the -// UnmarshalText logic and continue using its default binding logic. (This ensures gin does not break backwards -// compatibility) -func TestMappingUsingBindUnmarshalerAndTextUnmarshalerWhenOnlyTextUnmarshalerDefined(t *testing.T) { - var s struct { - Hex customUnmarshalTextHex `form:"hex"` - HexByUnmarshalText customUnmarshalTextHex `form:"hex2,parser=encoding.TextUnmarshaler"` - } - err := mappingByPtr(&s, formSource{ - "hex": {`11`}, - "hex2": {`11`}, - }, "form") - require.NoError(t, err) - - assert.EqualValues(t, 11, s.Hex) // this is using default int binding, not our custom hex binding. 0x11 should be 17 in decimal - assert.EqualValues(t, 0x11, s.HexByUnmarshalText) // correct expected value for normal hex binding -} - type customHexUnmarshalParamAndUnmarshalText int func (f *customHexUnmarshalParamAndUnmarshalText) UnmarshalParam(param string) error { diff --git a/codec/binding/api.go b/codec/binding/api.go new file mode 100644 index 00000000..213365a9 --- /dev/null +++ b/codec/binding/api.go @@ -0,0 +1,28 @@ +package bindingcodec + +import "reflect" + +func init() { + API = bindingApi{} +} + +type bindingApi struct{} + +// API the binding codec in use +var API Core + +// Core the api for binding codec +type Core interface { + // TrySetByInterface tries to set valueToSet from inputVal using one of the optional interfaces that gin supports + // + // Returns: + // - isSet: whether the value was set successfully + // - err: any error that occurred during the setting process + TrySetByInterface(inputVal string, valueToSet reflect.Value) (isSet bool, err error) +} + +// bindUnmarshaler duplicates binding.BindUnmarshaler to avoid an import cycle +// This must match binding.BindUnmarshaler exactly to maintain consistent behavior +type bindUnmarshaler interface { + UnmarshalParam(param string) error +} diff --git a/codec/binding/api_test.go b/codec/binding/api_test.go new file mode 100644 index 00000000..f5d0f2b4 --- /dev/null +++ b/codec/binding/api_test.go @@ -0,0 +1,12 @@ +package bindingcodec + +import ( + "testing" +) + +// TestAPIInitialization tests that the API is properly initialized +func TestAPIInitialization(t *testing.T) { + if API == nil { + t.Fatal("API should not be nil after initialization") + } +} diff --git a/codec/binding/default.go b/codec/binding/default.go new file mode 100644 index 00000000..4d0f345b --- /dev/null +++ b/codec/binding/default.go @@ -0,0 +1,20 @@ +//go:build !gin_bind_encoding + +package bindingcodec + +import ( + "reflect" +) + +// Description summarizes what this binding codec does. This variable helps +// ensure that build tags are configured to be mutually exclusive +const Description = "Gin default binding api" + +// TrySetByInterface uses bindUnmarshaler if implemented, otherwise returns false to revert to gin's default binding logic +func (d bindingApi) TrySetByInterface(inputVal string, valueToSet reflect.Value) (isSet bool, err error) { + switch v := valueToSet.Addr().Interface().(type) { + case bindUnmarshaler: + return true, v.UnmarshalParam(inputVal) + } + return false, nil +} diff --git a/codec/binding/default_test.go b/codec/binding/default_test.go new file mode 100644 index 00000000..b3ef0016 --- /dev/null +++ b/codec/binding/default_test.go @@ -0,0 +1,89 @@ +//go:build !gin_bind_encoding + +package bindingcodec + +import ( + "errors" + "reflect" + "testing" + + "github.com/stretchr/testify/require" +) + +// mockBindUnmarshaler implements the bindUnmarshaler interface for testing +type mockBindUnmarshaler struct { + receivedParam string + returnError error +} + +func (m *mockBindUnmarshaler) UnmarshalParam(param string) error { + m.receivedParam = param + return m.returnError +} + +// TestTrySetByInterface_WithBindUnmarshaler tests successful binding with bindUnmarshaler +func TestTrySetByInterface_WithBindUnmarshaler(t *testing.T) { + api := bindingApi{} + mock := &mockBindUnmarshaler{} + value := reflect.ValueOf(mock).Elem() + + inputVal := "test-value" + isSet, err := api.TrySetByInterface(inputVal, value) + require.True(t, isSet) + require.NoError(t, err) + require.Equal(t, "test-value", mock.receivedParam) +} + +// TestTrySetByInterface_WithBindUnmarshalerError tests error handling from bindUnmarshaler +func TestTrySetByInterface_WithBindUnmarshalerError(t *testing.T) { + api := bindingApi{} + expectedErr := errors.New("unmarshal error") + mock := &mockBindUnmarshaler{returnError: expectedErr} + value := reflect.ValueOf(mock).Elem() + + inputVal := "test-value" + isSet, err := api.TrySetByInterface(inputVal, value) + require.True(t, isSet) + require.Error(t, err) +} + +// TestTrySetByInterface_WithoutInterface tests behavior with regular types +func TestTrySetByInterface_WithoutInterface(t *testing.T) { + api := bindingApi{} + + testCases := []struct { + name string + value any + }{ + {"string", "test"}, + {"int", 42}, + {"bool", true}, + {"struct", struct{ Field string }{"value"}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create a pointer to make it addressable + ptr := reflect.New(reflect.TypeOf(tc.value)) + ptr.Elem().Set(reflect.ValueOf(tc.value)) + value := ptr.Elem() + + isSet, err := api.TrySetByInterface("input", value) + require.False(t, isSet) + require.NoError(t, err) + }) + } +} + +// TestTrySetByInterface_WithPointer tests that the method works with pointer values +func TestTrySetByInterface_WithPointer(t *testing.T) { + api := bindingApi{} + mock := &mockBindUnmarshaler{} + value := reflect.ValueOf(mock).Elem() + + inputVal := "pointer-test" + isSet, err := api.TrySetByInterface(inputVal, value) + require.True(t, isSet) + require.NoError(t, err) + require.Equal(t, inputVal, mock.receivedParam) +} diff --git a/codec/binding/encoding.go b/codec/binding/encoding.go new file mode 100644 index 00000000..8f0003f0 --- /dev/null +++ b/codec/binding/encoding.go @@ -0,0 +1,37 @@ +//go:build gin_bind_encoding + +package bindingcodec + +import ( + "encoding" + "log" + "reflect" + "time" +) + +// Description summarizes what this binding codec does. This variable helps +// ensure that build tags are configured to be mutually exclusive +const Description = "Gin binding api using encoding.TextUnmarshaler" + +func init() { + log.Println("[GIN-debug] gin_bind_encoding flag active. Gin will bind using encoding.TextUnmarshaler by default") +} + +// TrySetByInterface checks for bindUnmarshaler first, then falls back to encoding.TextUnmarshaler. +// This allows types which implement TextUnmarshaler (like uuid.UUID) to be bound +// automatically without requiring an explicit parser tag. +// +// Note: time.Time is excluded from automatic TextUnmarshaler handling because gin +// provides dedicated time parsing via time_format, time_utc, and time_location tags. +func (d bindingApi) TrySetByInterface(inputVal string, valueToSet reflect.Value) (isSet bool, err error) { + switch v := valueToSet.Addr().Interface().(type) { + case bindUnmarshaler: + return true, v.UnmarshalParam(inputVal) + case encoding.TextUnmarshaler: + // Skip time.Time — it has dedicated handling in setWithProperType via setTimeField + if _, isTime := valueToSet.Interface().(time.Time); !isTime { + return true, v.UnmarshalText([]byte(inputVal)) + } + } + return false, nil +} diff --git a/codec/binding/encoding_test.go b/codec/binding/encoding_test.go new file mode 100644 index 00000000..c7cd76c4 --- /dev/null +++ b/codec/binding/encoding_test.go @@ -0,0 +1,152 @@ +//go:build gin_bind_encoding + +package bindingcodec + +import ( + "encoding" + "errors" + "reflect" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// mockBindUnmarshaler implements the bindUnmarshaler interface for testing +type mockBindUnmarshaler struct { + receivedParam string + returnError error +} + +func (m *mockBindUnmarshaler) UnmarshalParam(param string) error { + m.receivedParam = param + return m.returnError +} + +// mockTextUnmarshaler implements encoding.TextUnmarshaler for testing +type mockTextUnmarshaler struct { + receivedText []byte + returnError error +} + +func (m *mockTextUnmarshaler) UnmarshalText(text []byte) error { + m.receivedText = text + return m.returnError +} + +func TestTrySetByInterface_WithBindUnmarshaler(t *testing.T) { + api := bindingApi{} + mock := &mockBindUnmarshaler{} + value := reflect.ValueOf(mock).Elem() + + inputVal := "test-value" + isSet, err := api.TrySetByInterface(inputVal, value) + require.True(t, isSet) + require.NoError(t, err) + require.Equal(t, inputVal, mock.receivedParam) +} + +func TestTrySetByInterface_WithBindUnmarshalerError(t *testing.T) { + api := bindingApi{} + expectedErr := errors.New("unmarshal error") + mock := &mockBindUnmarshaler{returnError: expectedErr} + value := reflect.ValueOf(mock).Elem() + + inputVal := "test-value" + isSet, err := api.TrySetByInterface(inputVal, value) + require.True(t, isSet) + require.Error(t, err) +} + +func TestTrySetByInterface_WithTextUnmarshaler(t *testing.T) { + api := bindingApi{} + mock := &mockTextUnmarshaler{} + value := reflect.ValueOf(mock).Elem() + + inputVal := "text-value" + isSet, err := api.TrySetByInterface(inputVal, value) + require.True(t, isSet) + require.NoError(t, err) + require.Equal(t, []byte(inputVal), mock.receivedText) +} + +func TestTrySetByInterface_WithTextUnmarshalerError(t *testing.T) { + api := bindingApi{} + expectedErr := errors.New("text unmarshal error") + mock := &mockTextUnmarshaler{returnError: expectedErr} + value := reflect.ValueOf(mock).Elem() + + inputVal := "text-value" + isSet, err := api.TrySetByInterface(inputVal, value) + require.True(t, isSet) + require.Error(t, err) +} + +func TestTrySetByInterface_WithTimeTime(t *testing.T) { + api := bindingApi{} + now := time.Now() + value := reflect.ValueOf(&now).Elem() + + inputVal := "2023-01-01T00:00:00Z" + isSet, err := api.TrySetByInterface(inputVal, value) + require.False(t, isSet) + require.NoError(t, err) +} + +// TestTrySetByInterface_WithoutInterface tests behavior with regular types +func TestTrySetByInterface_WithoutInterface(t *testing.T) { + api := bindingApi{} + + testCases := []struct { + name string + value any + }{ + {"string", "test"}, + {"int", 42}, + {"bool", true}, + {"struct", struct{ Field string }{"value"}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ptr := reflect.New(reflect.TypeOf(tc.value)) + ptr.Elem().Set(reflect.ValueOf(tc.value)) + value := ptr.Elem() + + isSet, err := api.TrySetByInterface("input", value) + require.False(t, isSet) + require.NoError(t, err) + }) + } +} + +// mockBothInterfaces implements both bindUnmarshaler and encoding.TextUnmarshaler +type mockBothInterfaces struct { + bindCalled bool + textCalled bool +} + +func (m *mockBothInterfaces) UnmarshalParam(param string) error { + m.bindCalled = true + return nil +} + +func (m *mockBothInterfaces) UnmarshalText(text []byte) error { + m.textCalled = true + return nil +} + +var _ encoding.TextUnmarshaler = (*mockBothInterfaces)(nil) + +func TestTrySetByInterface_PriorityBindUnmarshaler(t *testing.T) { + api := bindingApi{} + mock := &mockBothInterfaces{} + value := reflect.ValueOf(mock).Elem() + + inputVal := "test" + isSet, err := api.TrySetByInterface(inputVal, value) + require.True(t, isSet) + require.NoError(t, err) + require.True(t, mock.bindCalled) + require.False(t, mock.textCalled) +} diff --git a/docs/doc.md b/docs/doc.md index d1c33b87..8b6fa9e7 100644 --- a/docs/doc.md +++ b/docs/doc.md @@ -39,6 +39,7 @@ - [Bind default value if none provided](#bind-default-value-if-none-provided) - [Collection format for arrays](#collection-format-for-arrays) - [Bind Uri](#bind-uri) + - [Bind with encoding.TextUnmarshaler by default](#bind-with-encodingTextUnmarshaler-by-default) - [Bind custom unmarshaler](#bind-custom-unmarshaler) - [Bind Header](#bind-header) - [Bind HTML checkboxes](#bind-html-checkboxes) @@ -1228,6 +1229,48 @@ curl -v localhost:8088/thinkerou/987fbc97-4bed-5078-9f07-9141ba07c9f3 curl -v localhost:8088/thinkerou/not-uuid ``` +### Bind with encoding.TextUnmarshaler by default + +By default, Gin binds using its own logic that does not account for `encoding.TextUnmarshaler`. +To bind using `encoding.TextUnmarshaler` by default, add the build tag `gin_bind_encoding` during the build process. + +```go +package main + +import ( + "github.com/gin-gonic/gin" + "github.com/gofrs/uuid" +) + +func main() { + route := gin.Default() + var request struct { + Uuid uuid.UUID `form:"uuid" binding:"uuid"` + } + + route.GET("/test", func(ctx *gin.Context) { + _ = ctx.BindQuery(&request) + ctx.JSON(200, request) + }) + _ = route.Run(":8088") +} +``` + +Test it with: + +```sh +go run -tags=gin_bind_encoding main.go +# Then in another terminal +curl 'localhost:8088/test?uuid=70d9b500-fa26-11dd-876f-322495cdf0f7' +``` +Result +```sh +{"Uuid":"70d9b500-fa26-11dd-876f-322495cdf0f7"} +``` + +Note: +- This feature is not active by default to avoid breaking changes for existing users dependent on gin's default binding logic + ### Bind custom unmarshaler To override gin's default binding logic, define a function on your type that satisfies the `encoding.TextUnmarshaler` interface from the Golang standard library. Then specify `parser=encoding.TextUnmarshaler` in the `uri`/`form` tag of the field being bound.