Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encodes response trailers when gRPC-Web client ask for text based response. #1013

Closed
wants to merge 0 commits into from

Conversation

fengli79
Copy link
Contributor

Encodes response trailers when gRPC-Web client ask for text based response.

@@ -119,6 +119,11 @@ Http::FilterTrailersStatus GrpcWebFilter::encodeTrailers(Http::HeaderMap& traile
const uint32_t length = htonl(temp.length());
buffer.add(&length, 4);
buffer.move(temp);
if (is_text_response_) {
std::string encoded = Base64::encode(buffer, buffer.length());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const

@@ -119,6 +119,11 @@ Http::FilterTrailersStatus GrpcWebFilter::encodeTrailers(Http::HeaderMap& traile
const uint32_t length = htonl(temp.length());
buffer.add(&length, 4);
buffer.move(temp);
if (is_text_response_) {
std::string encoded = Base64::encode(buffer, buffer.length());
buffer.drain(buffer.length());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be clearer to use a second buffer rather than draining here, it shows the logical flow. Overhead is the same, it's all just slices I believe.

@fengli79 fengli79 closed this May 25, 2017
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
Automatic merge from submit-queue.

Update OWNERS

**What this PR does / why we need it**:

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
```
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Since we essentially just want to enforce that each filter has a unique name in the registry, we can move this burden into the library and avoid making consumers specify filter names directly.

Signed-off-by: Michael Rebello <[email protected]>
Signed-off-by: JP Simard <[email protected]>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Since we essentially just want to enforce that each filter has a unique name in the registry, we can move this burden into the library and avoid making consumers specify filter names directly.

Signed-off-by: Michael Rebello <[email protected]>
Signed-off-by: JP Simard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants