filters/on_demand: fix first request dropped when body is present#42158
filters/on_demand: fix first request dropped when body is present#42158botengyao merged 2 commits intoenvoyproxy:mainfrom
Conversation
5590f59 to
bcd1197
Compare
Track downstream_end_stream_ to support stream recreation with fully read request bodies. Previously, the filter rejected all requests with bodies by checking !callbacks_->decodingBuffer(), even when the body was complete. Now properly distinguishes incomplete vs complete requests by tracking end_stream state through decoder methods, following the same pattern as the router filter's internal redirect support. Fixes envoyproxy#17891 Signed-off-by: William Dauchy <william.dauchy@datadoghq.com> Signed-off-by: William Dauchy <william.dauchy@datadoghq.com>
d870f25 to
b7de6c8
Compare
runtime flag to protect the behaviour change Signed-off-by: William Dauchy <william.dauchy@datadoghq.com>
botengyao
left a comment
There was a problem hiding this comment.
One small improvement otherwise lgtm
/wait
|
Does this addresses the comment mentioned #17891 (comment) for both H/1 and H/2? |
I believe the protocol level I will double check this in practice. |
Will it make sense to add integration tests? |
good point, I will see if I can test this scenario |
I tried something in #42248 |
…42248) Commit Message: Add integration tests verifying VHDS on-demand updates work correctly with request bodies for both HTTP/1.1 and HTTP/2. The tests use HttpProtocolIntegrationTest parameterization to automatically run for both downstream protocols (HTTP/1.1 and HTTP/2), ensuring the fix for #17891 works correctly across different HTTP protocol versions. Additional Description: Tests verify that: - Request bodies are preserved when triggering on-demand VHDS updates - Stream recreation works correctly after VHDS updates complete - Both HTTP/1.1 and HTTP/2 protocols handle bodies correctly The test instantiation uses getProtocolTestParamsWithoutHTTP3() which returns parameter combinations for both HTTP/1.1 and HTTP/2 downstream protocols, so each test runs multiple times (once per protocol combination). Complements the unit tests in d39afea and provides end-to-end verification for #17891. followup of #42158 Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] Signed-off-by: William Dauchy <william.dauchy@datadoghq.com>
…voyproxy#42158) Commit Message: Track downstream_end_stream_ to support stream recreation with fully read request bodies. Previously, the filter rejected all requests with bodies by checking !callbacks_->decodingBuffer(), even when the body was complete. Now properly distinguishes incomplete vs complete requests by tracking end_stream state through decoder methods, following the same pattern as the router filter's internal redirect support. Additional Description: Risk Level: Testing: updated tests Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] Fixes envoyproxy#17891 [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: William Dauchy <william.dauchy@datadoghq.com> Signed-off-by: Gustavo <grnmeira@gmail.com>
…nvoyproxy#42248) Commit Message: Add integration tests verifying VHDS on-demand updates work correctly with request bodies for both HTTP/1.1 and HTTP/2. The tests use HttpProtocolIntegrationTest parameterization to automatically run for both downstream protocols (HTTP/1.1 and HTTP/2), ensuring the fix for envoyproxy#17891 works correctly across different HTTP protocol versions. Additional Description: Tests verify that: - Request bodies are preserved when triggering on-demand VHDS updates - Stream recreation works correctly after VHDS updates complete - Both HTTP/1.1 and HTTP/2 protocols handle bodies correctly The test instantiation uses getProtocolTestParamsWithoutHTTP3() which returns parameter combinations for both HTTP/1.1 and HTTP/2 downstream protocols, so each test runs multiple times (once per protocol combination). Complements the unit tests in d39afea and provides end-to-end verification for envoyproxy#17891. followup of envoyproxy#42158 Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] Signed-off-by: William Dauchy <william.dauchy@datadoghq.com> Signed-off-by: Gustavo <grnmeira@gmail.com>
Commit Message:
Track downstream_end_stream_ to support stream recreation with fully read request bodies. Previously, the filter rejected all requests with bodies by checking !callbacks_->decodingBuffer(), even when the body was complete.
Now properly distinguishes incomplete vs complete requests by tracking end_stream state through decoder methods, following the same pattern as the router filter's internal redirect support.
Additional Description:
Risk Level:
Testing: updated tests
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
Fixes #17891
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]