Repository: go-kratos/kratos
Pull request #2392 (2022-09-20 14:08:19)

fix(http/binding): fix http encode url
```
/v1/users/1?key=value
```

codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/2392?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#2392](https://codecov.io/gh/go-kratos/kratos/pull/2392?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (cc298be) into [main](https://codecov.io/gh/go-kratos/kratos/commit/080165f2ee466177aeb894d82adb24874cba4e75?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (080165f) will **decrease** coverage by `0.14%`.
> The diff coverage is `82.35%`.

> :exclamation: Current head cc298be differs from pull request most recent head 64dd63a. Consider uploading reports for the commit 64dd63a to get more accurate results

```diff
@@            Coverage Diff             @@
##             main    #2392      +/-   ##
==========================================
- Coverage   85.44%   85.30%   -0.15%     
==========================================
  Files          88       88              
  Lines        3780     3764      -16     
==========================================
- Hits         3230     3211      -19     
- Misses        383      386       +3     
  Partials      167      167              
```


| [Impacted Files](https://codecov.io/gh/go-kratos/kratos/pull/2392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [encoding/form/form.go](https://codecov.io/gh/go-kratos/kratos/pull/2392/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-ZW5jb2RpbmcvZm9ybS9mb3JtLmdv) | `65.71% <ø> (-1.86%)` | :arrow_down: |
| [encoding/form/proto\_encode.go](https://codecov.io/gh/go-kratos/kratos/pull/2392/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-ZW5jb2RpbmcvZm9ybS9wcm90b19lbmNvZGUuZ28=) | `71.03% <62.50%> (-0.30%)` | :arrow_down: |
| [transport/http/binding/encode.go](https://codecov.io/gh/go-kratos/kratos/pull/2392/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-dHJhbnNwb3J0L2h0dHAvYmluZGluZy9lbmNvZGUuZ28=) | `100.00% <100.00%> (ø)` | |
| [internal/context/context.go](https://codecov.io/gh/go-kratos/kratos/pull/2392/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-aW50ZXJuYWwvY29udGV4dC9jb250ZXh0Lmdv) | `97.01% <0.00%> (-2.99%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)


Patch:
diff --git a/encoding/form/form.go b/encoding/form/form.go
index 77c4237b..823aeb8f 100644
--- a/encoding/form/form.go
+++ b/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})
 }
diff --git a/encoding/form/proto_encode.go b/encoding/form/proto_encode.go
index 449fca0d..28ebb0bc 100644
--- a/encoding/form/proto_encode.go
+++ b/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) {
diff --git a/transport/http/binding/encode.go b/transport/http/binding/encode.go
index 3129bbe5..122ec1db 100644
--- a/transport/http/binding/encode.go
+++ b/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))
-}


Test patch:
diff --git a/transport/http/binding/encode_test.go b/transport/http/binding/encode_test.go
index fdc2bb78..16c7cf0e 100644
--- a/transport/http/binding/encode_test.go
+++ b/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)
 	}
 }