Skip to content

http3: turning up upstream integration test#15841

Merged
alyssawilk merged 4 commits intoenvoyproxy:mainfrom
alyssawilk:multiplexed
Apr 6, 2021
Merged

http3: turning up upstream integration test#15841
alyssawilk merged 4 commits intoenvoyproxy:mainfrom
alyssawilk:multiplexed

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk commented Apr 5, 2021

Turning up http2 upstream tests for http3.

Risk Level: n/a (test only)
Testing: yes
Docs Changes: n/a
Release Notes: n/a
Part of #15649

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk changed the title http3: turning up more tests http3: turning up upstream integration test Apr 5, 2021
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk marked this pull request as ready for review April 6, 2021 15:04
@@ -110,8 +110,6 @@ void EnvoyQuicClientStream::resetStream(Http::StreamResetReason reason) {
}

void EnvoyQuicClientStream::switchStreamBlockState(bool should_block) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This assert was basically "we should not be flow control blocked before reading body data" which turns out not to be true once you have multiple streams on one connection.


void EnvoyQuicClientStream::maybeDecodeTrailers() {
if (sequencer()->IsClosed() && !FinishedReadingTrailers()) {
ASSERT(!received_trailers().empty());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

turns out it's fine to have empty trailers if the peer sends empty trailers :-P

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice!

@alyssawilk alyssawilk merged commit 2858cd2 into envoyproxy:main Apr 6, 2021
@alyssawilk alyssawilk deleted the multiplexed branch February 28, 2022 17:55
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