fix(binding): use UnmarshalText for types like enum (#1359)

xiaost · web-flow · commit 47fff9bfc07c · 2025-05-30T11:58:49.000+08:00
diff --git a/pkg/app/server/binding/binder_test.go b/pkg/app/server/binding/binder_test.go
@@ -41,7 +41,9 @@
 package binding
 
 import (
+	"encoding"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"mime/multipart"
 	"net/url"
@@ -1665,6 +1667,41 @@ func TestBind_NormalizeContentType(t *testing.T) {
 	assert.DeepEqual(t, "version", result.Version)
 }
 
+type TestEnumType int32
+
+var _ encoding.TextUnmarshaler = (*TestEnumType)(nil)
+
+func (p *TestEnumType) UnmarshalText(v []byte) error {
+	switch string(v) {
+	case "one":
+		*p = 1
+	case "two":
+		*p = 2
+	default:
+		return errors.New("invalid")
+	}
+	return nil
+}
+
+func TestBind_TextUnmarshaler(t *testing.T) {
+	type Query struct {
+		A TestEnumType  `query:"a"`
+		B TestEnumType  `query:"b"`
+		C *TestEnumType `query:"c"`
+		D *TestEnumType `query:"d"`
+	}
+	q := &Query{}
+	req := newMockRequest().SetRequestURI("http://example.com?a=1&b=one&c=2&d=two")
+	err := DefaultBinder().BindQuery(req.Req, q)
+	assert.Nil(t, err)
+	assert.DeepEqual(t, TestEnumType(1), q.A)
+	assert.DeepEqual(t, TestEnumType(1), q.B)
+	assert.NotNil(t, q.C)
+	assert.NotNil(t, q.D)
+	assert.DeepEqual(t, TestEnumType(2), *q.C)
+	assert.DeepEqual(t, TestEnumType(2), *q.D)
+}
+
 func Benchmark_Binding(b *testing.B) {
 	type Req struct {
 		Version string `path:"v"`
diff --git a/pkg/app/server/binding/internal/decoder/base_type_decoder.go b/pkg/app/server/binding/internal/decoder/base_type_decoder.go
@@ -123,6 +123,11 @@ func (d *baseTypeFieldTextDecoder) Decode(req *protocol.Request, params param.Pa
 	}
 
 	// Non-pointer elems
+	if field.CanAddr() {
+		if tryTextUnmarshaler(field.Addr(), text) {
+			return nil
+		}
+	}
 	err = d.decoder.UnmarshalString(text, field, d.config.LooseZeroMode)
 	if err != nil {
 		return fmt.Errorf("unable to decode '%s' as %s: %w", text, d.fieldType.Name(), err)
diff --git a/pkg/app/server/binding/internal/decoder/util.go b/pkg/app/server/binding/internal/decoder/util.go
@@ -17,6 +17,7 @@
 package decoder
 
 import (
+	"encoding"
 	"fmt"
 	"reflect"
 	"strings"
@@ -45,32 +46,50 @@ func toDefaultValue(typ reflect.Type, defaultValue string) string {
 }
 
 // stringToValue is used to dynamically create reflect.Value for 'text'
-func stringToValue(elemType reflect.Type, text string, req *protocol.Request, params param.Params, config *DecodeConfig) (v reflect.Value, err error) {
-	v = reflect.New(elemType).Elem()
+func stringToValue(elemType reflect.Type, text string, req *protocol.Request, params param.Params, config *DecodeConfig) (reflect.Value, error) {
 	if customizedFunc, exist := config.TypeUnmarshalFuncs[elemType]; exist {
 		val, err := customizedFunc(req, params, text)
 		if err != nil {
 			return reflect.Value{}, err
 		}
 		return val, nil
 	}
+	v := reflect.New(elemType)
+	if tryTextUnmarshaler(v, text) {
+		return v.Elem(), nil
+	}
 	switch elemType.Kind() {
-	case reflect.Struct:
-		err = hJson.Unmarshal(bytesconv.S2b(text), v.Addr().Interface())
-	case reflect.Map:
-		err = hJson.Unmarshal(bytesconv.S2b(text), v.Addr().Interface())
+	case reflect.Struct, reflect.Map:
+		if err := hJson.Unmarshal(bytesconv.S2b(text), v.Interface()); err != nil {
+			return reflect.Value{}, err
+		}
+		return v.Elem(), nil
+
 	case reflect.Array, reflect.Slice:
 		// do nothing
+		return v.Elem(), nil
+
 	default:
 		decoder, err := SelectTextDecoder(elemType)
 		if err != nil {
-			return reflect.Value{}, fmt.Errorf("unsupported type %s for slice/array", elemType.String())
+			return reflect.Value{}, err
 		}
+		v = v.Elem()
 		err = decoder.UnmarshalString(text, v, config.LooseZeroMode)
 		if err != nil {
 			return reflect.Value{}, fmt.Errorf("unable to decode '%s' as %s: %w", text, elemType.String(), err)
 		}
+		return v, nil
 	}
 
-	return v, err
+}
+
+func tryTextUnmarshaler(v reflect.Value, s string) bool {
+	enc, ok := v.Interface().(encoding.TextUnmarshaler)
+	if ok {
+		if err := enc.UnmarshalText(bytesconv.S2b(s)); err == nil {
+			return true
+		}
+	}
+	return false
 }
diff --git a/pkg/app/server/binding/internal/decoder/util_test.go b/pkg/app/server/binding/internal/decoder/util_test.go
@@ -0,0 +1,206 @@
+/*
+ * Copyright 2024 CloudWeGo Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package decoder
+
+import (
+	"encoding"
+	"errors"
+	"reflect"
+	"testing"
+
+	"github.com/cloudwego/hertz/pkg/common/test/assert"
+	"github.com/cloudwego/hertz/pkg/protocol"
+	"github.com/cloudwego/hertz/pkg/route/param"
+)
+
+type testTextUnmarshaler struct {
+	Value string
+}
+
+func (t *testTextUnmarshaler) UnmarshalText(text []byte) error {
+	t.Value = string(text)
+	return nil
+}
+
+var _ encoding.TextUnmarshaler = (*testTextUnmarshaler)(nil)
+
+func TestStringToValue(t *testing.T) {
+	tests := []struct {
+		name        string
+		elemType    reflect.Type
+		text        string
+		config      *DecodeConfig
+		expectValue interface{}
+		expectError bool
+	}{
+		{
+			name:        "string type",
+			elemType:    reflect.TypeOf(""),
+			text:        "test string",
+			expectValue: "test string",
+		},
+		{
+			name:        "int type",
+			elemType:    reflect.TypeOf(0),
+			text:        "42",
+			expectValue: 42,
+		},
+		{
+			name:        "bool type",
+			elemType:    reflect.TypeOf(false),
+			text:        "true",
+			expectValue: true,
+		},
+		{
+			name:        "float type",
+			elemType:    reflect.TypeOf(0.0),
+			text:        "3.14",
+			expectValue: 3.14,
+		},
+		{
+			name:        "text unmarshaler",
+			elemType:    reflect.TypeOf(testTextUnmarshaler{}),
+			text:        "custom text",
+			expectValue: testTextUnmarshaler{Value: "custom text"},
+		},
+		{
+			name:        "invalid int",
+			elemType:    reflect.TypeOf(0),
+			text:        "not an int",
+			expectError: true,
+		},
+		{
+			name:        "struct type",
+			elemType:    reflect.TypeOf(struct{ Name string }{}),
+			text:        `{"Name":"test"}`,
+			expectValue: struct{ Name string }{Name: "test"},
+		},
+		{
+			name:        "struct type err",
+			elemType:    reflect.TypeOf(struct{ Name string }{}),
+			text:        `{"Name":1}`,
+			expectError: true,
+		},
+		{
+			name:        "list type",
+			elemType:    reflect.TypeOf([]int{}),
+			expectValue: *new([]int),
+		},
+		{
+			name:        "map type",
+			elemType:    reflect.TypeOf(map[string]interface{}{}),
+			text:        `{"key":"value"}`,
+			expectValue: map[string]interface{}{"key": "value"},
+		},
+		{
+			name:        "unsupported type",
+			elemType:    reflect.TypeOf(complex64(0)),
+			expectError: true,
+		},
+		{
+			name:     "custom type unmarshal func",
+			elemType: reflect.TypeOf(testTextUnmarshaler{}),
+			text:     "custom func",
+			config: &DecodeConfig{
+				TypeUnmarshalFuncs: map[reflect.Type]CustomizeDecodeFunc{
+					reflect.TypeOf(testTextUnmarshaler{}): func(req *protocol.Request, params param.Params, text string) (reflect.Value, error) {
+						return reflect.ValueOf(testTextUnmarshaler{Value: "from custom func"}), nil
+					},
+				},
+			},
+			expectValue: testTextUnmarshaler{Value: "from custom func"},
+		},
+		{
+			name:     "custom type unmarshal func err",
+			elemType: reflect.TypeOf(testTextUnmarshaler{}),
+			config: &DecodeConfig{
+				TypeUnmarshalFuncs: map[reflect.Type]CustomizeDecodeFunc{
+					reflect.TypeOf(testTextUnmarshaler{}): func(req *protocol.Request, params param.Params, text string) (reflect.Value, error) {
+						return reflect.Value{}, errors.New("err")
+					},
+				},
+			},
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := &protocol.Request{}
+			params := param.Params{}
+			config := tt.config
+			if config == nil {
+				config = &DecodeConfig{}
+			}
+			val, err := stringToValue(tt.elemType, tt.text, req, params, config)
+			if tt.expectError {
+				assert.NotNil(t, err)
+				return
+			}
+			assert.Nil(t, err)
+			assert.DeepEqual(t, tt.expectValue, val.Interface())
+		})
+	}
+}
+
+func TestTryTextUnmarshaler(t *testing.T) {
+	tests := []struct {
+		name     string
+		value    interface{}
+		text     string
+		expected bool
+	}{
+		{
+			name:     "text unmarshaler",
+			value:    &testTextUnmarshaler{},
+			text:     "test text",
+			expected: true,
+		},
+		{
+			name:     "non text unmarshaler",
+			value:    &struct{}{},
+			text:     "test text",
+			expected: false,
+		},
+		{
+			name:     "nil value",
+			value:    nil,
+			text:     "test text",
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var v reflect.Value
+			if tt.value != nil {
+				v = reflect.ValueOf(tt.value)
+			} else {
+				v = reflect.ValueOf(&tt.value).Elem()
+			}
+
+			result := tryTextUnmarshaler(v, tt.text)
+			assert.DeepEqual(t, tt.expected, result)
+
+			if tt.expected && tt.value != nil {
+				// Verify the value was actually set
+				unmarshaler := tt.value.(*testTextUnmarshaler)
+				assert.DeepEqual(t, tt.text, unmarshaler.Value)
+			}
+		})
+	}
+}
