skip parsing HttpBody field and pass query params as-is #13679
Conversation
|
/wait |
| @@ -860,6 +860,20 @@ TEST_F(GrpcJsonTranscoderFilterTest, TranscodingUnaryPostWithHttpBody) { | |||
| EXPECT_THAT(request, ProtoEq(expected_request)); | |||
| } | |||
|
|
|||
| TEST_F(GrpcJsonTranscoderFilterTest, TranscodingUnaryGetWithHttpBody) { | |||
| const std::string path = "/echoRawBody?data=oops%7b"; | |||
There was a problem hiding this comment.
Not quite understand the intention of setting data parameter here? Does it want transcoder to set its value to the HttpBody.data field?
I don't see the code to copy it to the HttpBody.data field.
There was a problem hiding this comment.
The code shouldn't write data field in to HttpBody.data because HttpBody field shouldn't be override by query param. This test is to test when there is a query param is named with data, and endpoint wants a raw httpbody request info, transcoder should handle this correctly.
|
/wait Sorry for the back and forth, but is working on adding several test scenarios to make it more understandable. :p |
|
Ughh, apparently dumb me follow the wrong sign-off process, reopen this pr. @qiwzhang I will need one more time of approval, really appreciate your help. :D |
Signed-off-by: Curry Xu <[email protected]>
|
You may need to fix the format first. You can run tools/code_format/check_format.py fix modified-cc-files |
Signed-off-by: Curry Xu <[email protected]>
Signed-off-by: Curry Xu <[email protected]>
|
@lizan Hi! Could you take a look at this and let me know if this can be merged or if I need to do anything else? Thank you! :) |
Commit Message: skip parsing HttpBody field and pass query params as-is
Additional Description: Check the request url, if there is any HttpBody, and
method_info->request_body_field_pathis not a prefix ofresolved_binding.field_path, skip parsing HttpBody field. This avoid we may skip some HttpBody objects which were intended to be configurable by query arguments.Testing: Add a new unit test to cover the changes.
Docs Changes: n/a
Fixes #13634