-
Notifications
You must be signed in to change notification settings - Fork 10k
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
[release/7.0] Fix HTTP/3 and HTTP/2 header decoder buffer allocation #47949
Conversation
Hi @ManickaP. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document. |
Hi @ManickaP. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge. To learn more about how to prepare a servicing PR click here. |
FYI you need to manually make the change in main too. There is no automated forward porting. |
This is already in main #47793 |
Approved by Tactics (@SteveMCarroll) on 4/28 via email. Marking as Servicing-approved. |
Hi @ManickaP. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed. |
Need changes I added to the code sync PR to fix warnings - #47793 |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Just noticed this is still red. I'll fix the CI issues here. |
Er, looks like I don't have permission to push to your fork @ManickaP. Opened a PR here instead to update this branch: ManickaP#123 |
Merged and the pipelines are running, thank you for the fix! |
Looks like we've still got failures, do we need another fix? @JamesNK @ManickaP @adityamandaleeka |
Manual backport of #47793
Backport of runtime PR in main dotnet/runtime#78862
Port of runtime 7.0 PR dotnet/runtime#85337
Fixes dotnet/runtime#78516
/cc @Tratcher @JamesNK
Fix HTTP/3 and HTTTP/2 header decoder buffer allocation
Description
We resize an internal buffer by an incorrect amount, and subsequent copies to that buffer will throw. The problem occurs in HPack since 5.0 and in QPack since 7.0.
Customer Impact
Reliability problem in HTTP/2 and HTTP/3, where some requests/responses with large headers that should be accepted might end up throwing exception.
This is a shared code with runtime so this affects both client and server - existing PR in runtime: dotnet/runtime#85337.
Regression?
Regressed in 7.0 for QPack.
Risk
Low, as this affects only QPack (H/3 is still not as wide-spread as other HTTP versions) and HPack in (rare) case of 4KB+ sized headers data buffers.
Verification
Added tests for the root cause and similar scenarios, increasing test coverage. All of those are ran in CI.
Packaging changes reviewed?