-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
HTTP/3 codec does not set end_stream
properly
#28053
Comments
Can you confirm whether this is a bug or not @danzh2010? |
In your test, did FIN arrive in OnBodyAvailable() with empty payload? |
I have been working on something else now but I will check it soon and probably share the test through a PR soon so that you can take a look at it. |
This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions. |
@RyanTheOptimist could you add "no stalebot" tag to this issue please? (or let me know how I can do it myself if there's a way) |
Done! |
Title: HTTP/3 codec does not set
end_stream
properlyDescription:
HTTP/3 codec does not set
end_stream
properly fordecodeHeaders
, and perhaps for other methods as well.I think the issue is that
OnInitialHeadersComplete
ofEnvoyQuicServerStream
andEnvoyQuicClientStream
just passesfin
argument todecodeHeaders
. However, thefin
argument ofOnInitialHeadersComplete
doesn't seem to contain the right information based on the comments in the QUICHE code.Repro steps:
I wrote an integration test using
IntegrationCodecClient::startRequest
, which internally callsencodeHeaders
. I set the secondheader_only_request
totrue
on the client side, and found that thefin
argument ofEnvoyQuicServerStream::OnInitialHeadersComplete
is still set tofalse
.I think
EnvoyQuicServerStream::OnInitialHeadersComplete
should pass something different from thefin
argument, perhapsfin_received()
derived fromQuicStream
.The text was updated successfully, but these errors were encountered: