grpc-json: no buffering at end_stream or non-gRPC response#2363
grpc-json: no buffering at end_stream or non-gRPC response#2363htuch merged 9 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Lizan Zhou <zlizan@google.com>
mattklein123
left a comment
There was a problem hiding this comment.
LGTM, small nits. Thanks for fixing!
| }; | ||
|
|
||
| bool isGrpcResponse(const Http::HeaderMap& headers) { | ||
| if (strcmp("200", headers.Status()->value().c_str()) != 0) { |
There was a problem hiding this comment.
Please use getResponseStatus here to match similar code elsewhere.
| return false; | ||
| } | ||
| return headers.ContentType() && | ||
| Http::Headers::get().ContentTypeValues.Grpc == headers.ContentType()->value().c_str(); |
There was a problem hiding this comment.
I think this will cause a string temporary to be created. If you invert it won't:
headers.ContentType()->value() == Http::Headers::get().ContentTypeValues.Grpc.c_str()
There was a problem hiding this comment.
Done.
I'm fine either though I don't think it will make a string temporary, as <string> defines:
bool operator== (const string& lhs, const char* rhs);
There was a problem hiding this comment.
Ah, good to know. Thanks for pointing that out.
Signed-off-by: Lizan Zhou <zlizan@google.com>
| if (Http::Utility::getResponseStatus(headers) != 200) { | ||
| return false; | ||
| } | ||
| return headers.ContentType() && |
There was a problem hiding this comment.
Should this use https://github.com/envoyproxy/envoy/blob/master/source/common/grpc/common.cc#L25? Maybe this util belongs in Grpc::Common as well.
Signed-off-by: Lizan Zhou <zlizan@google.com>
Signed-off-by: Lizan Zhou <zlizan@google.com>
source/common/grpc/common.h
Outdated
| public: | ||
| /** | ||
| * @param headers the headers to parse. | ||
| * @param bool indicating wether the header is at end_stream. |
There was a problem hiding this comment.
? (I think you meant to put this below)
mattklein123
left a comment
There was a problem hiding this comment.
LGTM other than small comment.
Signed-off-by: Lizan Zhou <zlizan@google.com>
Signed-off-by: Lizan Zhou <zlizan@google.com>
Signed-off-by: Lizan Zhou <zlizan@google.com>
Description: Improves security with respect to third-party actions. See: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions I also read through the recent commits and updated to the latest release, since it contained a few bugfixes. Risk: Low Testing: CI Signed-off-by: Mike Schore <mike.schore@gmail.com> Signed-off-by: JP Simard <jp@jpsim.com>
Description: Improves security with respect to third-party actions. See: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions I also read through the recent commits and updated to the latest release, since it contained a few bugfixes. Risk: Low Testing: CI Signed-off-by: Mike Schore <mike.schore@gmail.com> Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: Lizan Zhou zlizan@google.com
Description:
Make gRPC-JSON transcoder not buffering response at end_stream or non-gRPC response.
Risk Level: Low
Testing:
unit test
Docs Changes:
N/A
Release Notes:
N/A
Fixes #2275