fix(http/binding): fix http encode url (#2392)

* fix(http/binding): fix http encode url

encoding/form/form.go

@@ -17,10 +17,13 @@ const (
 	nullStr = "null"
 )
 
+var (
+	encoder = form.NewEncoder()
+	decoder = form.NewDecoder()
+)
+
 func init() {
-	decoder := form.NewDecoder()
 	decoder.SetTagName("json")
-	encoder := form.NewEncoder()
 	encoder.SetTagName("json")
 	encoding.RegisterCodec(codec{encoder: encoder, decoder: decoder})
 }

encoding/form/proto_encode.go

@@ -14,16 +14,19 @@ import (
 )
 
 // EncodeValues encode a message into url values.
-func EncodeValues(msg proto.Message) (url.Values, error) {
+func EncodeValues(msg interface{}) (url.Values, error) {
 	if msg == nil || (reflect.ValueOf(msg).Kind() == reflect.Ptr && reflect.ValueOf(msg).IsNil()) {
 		return url.Values{}, nil
 	}
-	u := make(url.Values)
-	err := encodeByField(u, "", msg.ProtoReflect())
-	if err != nil {
-		return nil, err
+	if v, ok := msg.(proto.Message); ok {
+		u := make(url.Values)
+		err := encodeByField(u, "", v.ProtoReflect())
+		if err != nil {
+			return nil, err
+		}
+		return u, nil
 	}
-	return u, nil
+	return encoder.Encode(msg)
 }
 
 func encodeByField(u url.Values, path string, m protoreflect.Message) (finalErr error) {

transport/http/binding/encode.go

@@ -1,24 +1,21 @@
 package binding
 
 import (
-	"fmt"
 	"reflect"
 	"regexp"
-	"strings"
 
 	"github.com/go-kratos/kratos/v2/encoding/form"
-
 	"google.golang.org/protobuf/proto"
-	protoreflect "google.golang.org/protobuf/reflect/protoreflect"
 )
 
 var reg = regexp.MustCompile(`/{[\\.\w]+}`)
 
 // EncodeURL encode proto message to url path.
-func EncodeURL(pathTemplate string, msg proto.Message, needQuery bool) string {
+func EncodeURL(pathTemplate string, msg interface{}, needQuery bool) string {
 	if msg == nil || (reflect.ValueOf(msg).Kind() == reflect.Ptr && reflect.ValueOf(msg).IsNil()) {
 		return pathTemplate
 	}
+	queryParams, _ := form.EncodeValues(msg)
 	pathParams := make(map[string]struct{})
 	path := reg.ReplaceAllStringFunc(pathTemplate, func(in string) string {
 		// it's unreachable because the reg means that must have more than one char in {}
@@ -26,50 +23,25 @@ func EncodeURL(pathTemplate string, msg proto.Message, needQuery bool) string {
 		//	return in
 		//}
 		key := in[2 : len(in)-1]
-		vars := strings.Split(key, ".")
-		value, err := getValueByField(msg.ProtoReflect(), vars)
-		if err != nil {
-			return in
-		}
+		value := queryParams.Get(key)
 		pathParams[key] = struct{}{}
 		return "/" + value
 	})
 	if !needQuery {
-		if query := form.EncodeFieldMask(msg.ProtoReflect()); query != "" {
-			return path + "?" + query
+		if v, ok := msg.(proto.Message); ok {
+			if query := form.EncodeFieldMask(v.ProtoReflect()); query != "" {
+				return path + "?" + query
+			}
 		}
 		return path
 	}
-	u, err := form.EncodeValues(msg)
-	if err == nil && len(u) > 0 {
+	if len(queryParams) > 0 {
 		for key := range pathParams {
-			delete(u, key)
+			delete(queryParams, key)
 		}
-		query := u.Encode()
-		if query != "" {
+		if query := queryParams.Encode(); query != "" {
 			path += "?" + query
 		}
 	}
 	return path
 }
-
-func getValueByField(v protoreflect.Message, fieldPath []string) (string, error) {
-	var fd protoreflect.FieldDescriptor
-	for i, fieldName := range fieldPath {
-		fields := v.Descriptor().Fields()
-		if fd = fields.ByJSONName(fieldName); fd == nil {
-			fd = fields.ByName(protoreflect.Name(fieldName))
-			if fd == nil {
-				return "", fmt.Errorf("field path not found: %q", fieldName)
-			}
-		}
-		if i == len(fieldPath)-1 {
-			break
-		}
-		if fd.Message() == nil || fd.Cardinality() == protoreflect.Repeated {
-			return "", fmt.Errorf("invalid path: %q is not a message", fieldName)
-		}
-		v = v.Get(fd).Message()
-	}
-	return form.EncodeField(fd, v.Get(fd))
-}

transport/http/binding/encode_test.go

@@ -9,29 +9,34 @@ import (
 )
 
 func TestProtoPath(t *testing.T) {
-	url := EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "2233!!!"}}, false)
-	fmt.Println(url)
+	url := EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "2233!!!"},
+	}, false)
 	if url != `http://helloworld.Greeter/helloworld/test/sub/2233!!!` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}", nil, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}", nil, false)
 	fmt.Println(url)
-	if url != "http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}" {
+	if url != "http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}" {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{}/sub/{sub.name}", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "hello"}}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{}/sub/{sub.naming}", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "hello"},
+	}, false)
 	fmt.Println(url)
 	if url != "http://helloworld.Greeter/helloworld/{}/sub/hello" {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{}/sub/{sub.name.cc}", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "hello"}}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{}/sub/{sub.name.cc}", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "hello"},
+	}, false)
 	fmt.Println(url)
-	if url != "http://helloworld.Greeter/helloworld/{}/sub/{sub.name.cc}" {
+	if url != "http://helloworld.Greeter/helloworld/{}/sub/" {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
 	url = EncodeURL(
-		"http://helloworld.Greeter/helloworld/{}/sub/{test_repeated.1}",
+		"http://helloworld.Greeter/helloworld/{}/sub/{test_repeated}",
 		&binding.HelloRequest{
 			Name: "test", Sub: &binding.Sub{Name: "hello"},
 			TestRepeated: []string{"123", "456"},
@@ -39,17 +44,21 @@ func TestProtoPath(t *testing.T) {
 		false,
 	)
 	fmt.Println(url)
-	if url != "http://helloworld.Greeter/helloworld/{}/sub/{test_repeated.1}" {
+	if url != "http://helloworld.Greeter/helloworld/{}/sub/123" {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "5566!!!"}}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "5566!!!"},
+	}, false)
 	fmt.Println(url)
 	if url != `http://helloworld.Greeter/helloworld/test/sub/5566!!!` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
-	url = EncodeURL("http://helloworld.Greeter/helloworld/sub", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "2233!!!"}}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/sub", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "2233!!!"},
+	}, false)
 	fmt.Println(url)
 	if url != `http://helloworld.Greeter/helloworld/sub` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
@@ -61,9 +70,9 @@ func TestProtoPath(t *testing.T) {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.name33}", &binding.HelloRequest{Name: "test"}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}", &binding.HelloRequest{Name: "test"}, false)
 	fmt.Println(url)
-	if url != `http://helloworld.Greeter/helloworld/test/sub/{sub.name33}` {
+	if url != `http://helloworld.Greeter/helloworld/test/sub/` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
@@ -76,12 +85,12 @@ func TestProtoPath(t *testing.T) {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
-	url = EncodeURL("http://helloworld.Greeter/helloworld/sub/{sub.name}", &binding.HelloRequest{
+	url = EncodeURL("http://helloworld.Greeter/helloworld/sub/{sub.naming}", &binding.HelloRequest{
 		Sub:        &binding.Sub{Name: "kratos"},
-		UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"name", "sub.name"}},
+		UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"name", "sub.naming"}},
 	}, false)
 	fmt.Println(url)
-	if url != `http://helloworld.Greeter/helloworld/sub/kratos?updateMask=name,sub.name` {
+	if url != `http://helloworld.Greeter/helloworld/sub/kratos?updateMask=name,sub.naming` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 }