Skip to content

[HTTP3] Correctly handle server closing a control stream #73680

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

Merged
merged 3 commits into from
Aug 11, 2022

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Aug 10, 2022

This PR makes sure we behave according to the spec when server closes/aborts either its or clients control stream (i.e. close the connection with H3_CLOSED_CRITICAL_STREAM error)

@ghost ghost added the area-System.Net.Http label Aug 10, 2022
@rzikm rzikm requested a review from CarnaViire August 10, 2022 08:21
@ghost ghost assigned rzikm Aug 10, 2022
@rzikm rzikm requested a review from ManickaP August 10, 2022 08:21
@ghost
Copy link

ghost commented Aug 10, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR makes sure we behave according to the spec when server closes/aborts either its or clients control stream (i.e. close the connection with H3_CLOSED_CRITICAL_STREAM error)

Author: rzikm
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@rzikm
Copy link
Member Author

rzikm commented Aug 10, 2022

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

…andler/Http3Connection.cs

Co-authored-by: Marie Píchová <11718369+ManickaP@users.noreply.github.com>
@@ -7,6 +7,7 @@
using System.Runtime.Versioning;
using System.Net.Quic;
using System.IO;
using System.Linq;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
using System.Linq;

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, I missed this one and merged it, I will remove it when I next touch HTTP3

@rzikm rzikm merged commit 80640d4 into dotnet:main Aug 11, 2022
@karelz karelz added this to the 7.0.0 milestone Aug 15, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants