Discussion: How to handle path variables with different text names and json_name tags in EncodeURL

[ruinshe]

Background

I've added a failing test case in @transport/http/binding/encode_test.go that highlights an issue with our current URL encoding implementation.

diff --git a/transport/http/binding/encode_test.go b/transport/http/binding/encode_test.go
index 227c2eaa..62500ab5 100644
--- a/transport/http/binding/encode_test.go
+++ b/transport/http/binding/encode_test.go
@@ -15,6 +15,12 @@ func TestEncodeURL(t *testing.T) {
                needQuery    bool
                want         string
        }{
+               {
+                       pathTemplate: "https://apis.example.com/api/v1/users/{user_id}/actions:updateUser",
+                       request:      &binding.UpdateUserRequest{UserId: "jack"},
+                       needQuery:    false,
+                       want:         "https://apis.example.com/api/v1/users/jack/actions:updateUser",
+               },
                {
                        pathTemplate: "http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}",
                        request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "2233!!!!"}},

As I haven't committed it yet, here is the test case for context:

service UserService {
  rpc UpdateUser(UpdateUserRequest) returns (User) {
    option (google.api.hage) = {
      post : "/api/v1/users/{user_id}/actions:updateUser"
      body : "user"
    };
  }
}

message UpdateUserRequest {
  // The id of the user.
  string user_id = 1 [ (google.api.field_behavior) = REQUIRED ];;

  // The user to be updated.
  User user = 2 [ (google.api.field_behavior) = REQUIRED ];
}

The root cause is that transport/http/binding/encode.go:EncodeURL uses encoding/form/proto_encode.go:EncodeValues to prepare the values for both path parameters and query parameters. However, EncodeValues uses the json_name of a protobuf field to create the key-value map, which is correct for query parameters according to protobuf's JSON mapping specification.

The problem arises when a URL path template uses the protobuf field's text name (the actual field name in the .proto file, which is typically snake_case), but the key generated by EncodeValues is the camelCase json_name.

In the example above, the path template expects a variable named {user_id}. However, EncodeValues processes the UpdateUserRequest and generates a key userId for the user_id field. When EncodeURL attempts to substitute {user_id}, it can't find a matching key in the parameter map, causing the substitution to fail and resulting in a malformed URL.

A key constraint is that we cannot simply change EncodeValues to use text names, as it's correctly implemented for query parameters. Furthermore, the latter half of the EncodeURL function relies on this json_name-based map to correctly drop path parameters and formulate the final query string.

Proposed Solutions

I have two potential solutions in mind, each with its own trade-offs. I'd like to get the community's input on the best approach.

Option 1: Runtime Mapping in EncodeURL

We could implement logic within EncodeURL itself (or a helper function) that performs a similar operation to encodeByField from proto_encode.go. This new logic would iterate over the fields of the protobuf message, creating a mapping from the text name (e.g., "name") to the JSON name (e.g., "naming").

When EncodeURL processes a path variable like {sub.name}, it would use this map to look up the correct key ("sub.naming") in the queryParams map generated by EncodeValues.

• Pros:
  • Self-contained fix within the framework.
  • No breaking changes or compatibility issues with the protoc-gen-go-http plugin.
• Cons:
  • Introduces runtime reflection overhead for each EncodeURL call that has path parameters.
  • Duplicates some of the logic already present in proto_encode.go.

Option 2: Pre-computation in protoc-gen-go-http

Another approach is to modify the protoc-gen-go-http plugin. The plugin could analyze the protobuf definitions and pre-generate the necessary text-name-to-json-name mappings. This map could then be passed as an argument to EncodeURL from the generated code.

• Pros:
  • Higher performance at runtime as the mapping is pre-computed.
• Cons:
  • Breaking Change: This would create a strong dependency between the framework version and the plugin version. Users would have to upgrade both simultaneously, which could cause significant friction.

Request for Feedback

Which of these two options seems more viable? Or, is there a third, better solution we haven't considered? I'm looking forward to hearing everyone's thoughts on how to best resolve this issue while considering performance and backward compatibility.

A crucial point to clarify is what the official stance should be: Should HTTP path variables use text_name or json_name? The current tests in @transport/http/binding/encode_test.go for cases like {sub.naming} imply that json_name is expected. However, the common convention for API path parameters is to use the snake_case field name (text_name).

Furthermore, even if we were to decide that json_name is the correct approach for path variables, the protoc-gen-go-http plugin currently does not support generating this. This reveals a fundamental inconsistency between the framework's behavior and the code generation tooling. We need to align these.

Related Issues and PRs

This problem has been discussed in several other issues and pull requests:

• EncodeURL will ignore some fields if request message contains repeated custom message #3141
• Address bar parameter splicing failed #2761
• [Question] I modified it based on examples/helloworld and found that client/main.go reported an error when executing. I am not sure if it is a BUG. #3331
• When generating the URL, the path parameter {xxx} is not replaced correctly, but instead xxx is added as a query parameter incorrectly #3560
• 修复地址栏参数 #2774
• feat: add option to control using proto text name as key during URL encoding #3311

[ruinshe]

@shenqidebaozi @demoManito could you help to take a look?