grpc_http1_bridge: prevent excessive data removal#24418
grpc_http1_bridge: prevent excessive data removal#24418snowp merged 3 commits intoenvoyproxy:mainfrom kmcbride:grpc-http1-bridge-data-fix
Conversation
|
Hi @kmcbride, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
|
/retest |
|
Retrying Azure Pipelines: |
snowp
left a comment
snowp
left a comment
There was a problem hiding this comment.
Thanks for the fix! This makes sense to me
Do you think you could update the unit tests to cover this case?
A release note would also be warranted (https://github.com/envoyproxy/envoy/blob/main/changelogs/current.yaml)
/wait
|
/retest |
|
Retrying Azure Pipelines: |
|
@snowp thanks for the review. The latest commit should hopefully address the comments. |
Signed-off-by: kmcbride <kmcbride@users.noreply.github.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
@snowp CI seems stuck 🙁 |
|
Can you try pushing an empty commit? I don't think we have a way to force these to re-run |
Signed-off-by: kmcbride <kmcbride@users.noreply.github.com>
Head branch was pushed to by a user without write access
|
/retest |
|
Retrying Azure Pipelines: |
|
@snowp I think this PR needs to be re-approved because I had to push a commit that fixed a merge conflict in the changelog. |
Signed-off-by: kmcbride <kmcbride@users.noreply.github.com>
Head branch was pushed to by a user without write access
Signed-off-by: kmcbride <kmcbride@users.noreply.github.com> Signed-off-by: VishalDamgude <vishal.damgude@freshworks.com>
2 participants
When leveraging the Protobuf upgrade functionality there is the possibility of data loss for responses that are processed in multiple chunks, as the frame header to be removed is seemingly only present in the first chunk.
This is a fix for an issue that we've seen in practice: clients fail to decode responses >16KB because 5 bytes of data is missing every subsequent ~16KB.