Skip to content

[release/6.0-rc1] HTTP/3: Response drain timeout #35492

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 24, 2021
Merged

[release/6.0-rc1] HTTP/3: Response drain timeout #35492

merged 1 commit into from
Aug 24, 2021
+302 −97

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Aug 19, 2021 •
edited by halter73
Loading

Backport of #35322 to release/6.0-rc1

Customer Impact

Response drain timeout is a security feature. It hardens Kestrel against certain attacks. This feature exists in HTTP/1.1 and HTTP/2. This PR adds it to HTTP/3.

Testing

Unit tests and functional tests.

Risk

Low. This feature only impacts HTTP/3. HTTP/3 is a preview feature in .NET 6.

@JamesNK JamesNK added area-runtime ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. labels Aug 19, 2021
@JamesNK JamesNK added this to the 6.0-rc1 milestone Aug 19, 2021
adityamandaleeka
adityamandaleeka approved these changes Aug 19, 2021
View reviewed changes
Copy link
Member

@adityamandaleeka adityamandaleeka left a comment

Choose a reason for hiding this comment

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

@davidfowl

@JamesNK
Copy link
Member Author

JamesNK commented Aug 19, 2021 •
edited
Loading

Contact me before merging this. One of the other PRs has a unit test that needs to be updated. Will make changes here after it is merged.

@davidfowl
Copy link
Member

20 files changed... Let me take a deeper look.

@JamesNK
Merge

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
1efd312
@JamesNK JamesNK force-pushed the jamesnk/rc1-http3-responsedrain branch from 357c30d to 1efd312 Compare August 20, 2021 22:21
@JamesNK
Copy link
Member Author

JamesNK commented Aug 20, 2021

Contact me before merging this. One of the other PRs has a unit test that needs to be updated. Will make changes here after it is merged.

Done

@davidfowl davidfowl added the Servicing-approved Shiproom has approved the issue label Aug 21, 2021
@JamesNK
Copy link
Member Author

JamesNK commented Aug 23, 2021

I don't have The Power to click merge.

@dougbu, or someone else with the right permissions, could you merge it?

@dougbu
Copy link
Contributor

dougbu commented Aug 23, 2021

@davidfowl where / when was this servicing-approved❔ Just double-checking because this isn't in the Servicing Status project.

@adityamandaleeka
Copy link
Member

@dougbu This is for 6.0 RC1. I don't believe those are being tracked in that project.

@dougbu
Copy link
Contributor

dougbu commented Aug 23, 2021

I don't believe those are being tracked in that project.

The project is one of the ways we track all conversations w/ Tactics about issues / PRs that require approval. If it's not necessary for the current RC1 state, 🆗 but we'll definitely need to use the project in RC2. @Pilchie is my understanding correct❔

@Pilchie
Copy link
Member

Pilchie commented Aug 23, 2021

I'll admit that I haven't been paying as much attention to that project since https://tacticsview.azurewebsites.net/ was made.

@dougbu
Copy link
Contributor

dougbu commented Aug 23, 2021

I'll admit that I haven't been paying as much attention to that project since https://tacticsview.azurewebsites.net/ was made.

Should we stop using the project, just delete it, …❔

@dougbu
Copy link
Contributor

dougbu commented Aug 23, 2021

@davidfowl where / when was this servicing-approved❔

I don't think my question was answered. I'm here to merge if this had the right 👀 on it.

@dougbu
Copy link
Contributor

dougbu commented Aug 24, 2021

Should we stop using the project, just delete it, …❔

/fyi we have Fabric bot rules that do "stuff" with the Servicing Status project but no rules for 'release/6.0-rc1'. I lean toward reducing our rule complexity and ignoring or deleting the project if that's allowed for existing servicing branches.

@Pilchie
Copy link
Member

Pilchie commented Aug 24, 2021

We can go ahead and merge this based on David's approval on Friday (he had delegated M2 approval then). In the future we should try to get things merged within 24-hours of approval though, as the bar does go up.

Regarding the servicing project, I'm fine with deleting it and simplifying the bot's config if no one is getting value from the project. Is anyone? Maybe we should ask at a DoI meeting before just deleting?

@dougbu dougbu merged commit c3c9732 into release/6.0-rc1 Aug 24, 2021
@dougbu dougbu deleted the jamesnk/rc1-http3-responsedrain branch August 24, 2021 02:14
@dougbu
Copy link
Contributor

dougbu commented Aug 24, 2021

Maybe we should ask at a DoI meeting before just deleting?

Good place to ask but that might miss the people who care. How 'bout an email❔

wtgodbe added a commit that referenced this pull request Aug 24, 2021
@wtgodbe @dotnet-maestro
@github-actions @BrennanConroy @JamesNK @SteveSandersonMS @TanayParikh @halter73 @HaoK @captainsafia @dougbu
[automated] Merge branch 'release/6.0-rc1' => 'release/6.0' (#35646)
8da7b81 
* Update dependencies from https://github.com/dotnet/efcore build 20210821.6 (#35617)

Microsoft.EntityFrameworkCore.Tools , dotnet-ef , Microsoft.EntityFrameworkCore , Microsoft.EntityFrameworkCore.SqlServer , Microsoft.EntityFrameworkCore.InMemory , Microsoft.EntityFrameworkCore.Relational , Microsoft.EntityFrameworkCore.Sqlite , Microsoft.EntityFrameworkCore.Design
 From Version 6.0.0-rc.1.21420.45 -> To Version 6.0.0-rc.1.21421.6

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update docker container to mcr (#35630)

Co-authored-by: Brennan <[email protected]>

* [release/6.0-rc1] Update dependencies from dotnet/runtime dotnet/efcore (#35634)

[release/6.0-rc1] Update dependencies from dotnet/runtime dotnet/efcore

* [release/6.0-rc1] HTTP/3: Response drain timeout (#35492)

- backport of #35322 to release/6.0-rc1

* [release/6.0-rc1] Reliability improvement for input date E2E tests (#35616)

* Reliability improvement for input date E2E tests

* Avoid "collection was modified" error in CircuitGracefulTerminationTests

* Avoid timing issues in CanFocusDuringOnAfterRenderAsyncWithFocusInEvent

* Update src/Components/test/E2ETest/Tests/FormsTest.cs

Co-authored-by: Tanay Parikh <[email protected]>

* Remove notes from earlier

Co-authored-by: Steve Sanderson <[email protected]>
Co-authored-by: Tanay Parikh <[email protected]>

* Minimal APIs naming cleanup

* Add Support for `DateOnly` & `TimeOnly` for `SupplyParameterFromQuery`

Fixes: #35525
API Proposal: #35567

* Add FailureReasons (#35425)

* Add support for Results extension point

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Brennan <[email protected]>
Co-authored-by: James Newton-King <[email protected]>
Co-authored-by: Steve Sanderson <[email protected]>
Co-authored-by: Tanay Parikh <[email protected]>
Co-authored-by: Stephen Halter <[email protected]>
Co-authored-by: Tanay Parikh <[email protected]>
Co-authored-by: Hao Kung <[email protected]>
Co-authored-by: Safia Abdalla <[email protected]>
Co-authored-by: Doug Bunting <[email protected]>
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@halter73 halter73

@adityamandaleeka adityamandaleeka

@Tratcher Tratcher

@BrennanConroy BrennanConroy

Assignees
No one assigned
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. Servicing-approved Shiproom has approved the issue
Projects
None yet
Milestone
6.0-rc1
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@JamesNK @davidfowl @dougbu @adityamandaleeka @Pilchie @halter73 @Tratcher @amcasey