Implement HTTP/2 WebSockets

[greenEkatherine]

Resolves #69669

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[ghost]

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

Issue Details

---

Resolves #69669

Author:	greenEkatherine
Assignees:	-
Labels:	area-System.Net.Http, new-api-needs-documentation
Milestone:	-

[stephentoub]

I only quickly skimmed it and left a few comments... I'll review in more depth once it's out of draft and TODOs are addressed.

[stephentoub]

I don't see any tests for the new APIs or functionality. Where are they?

[greenEkatherine]

I don't see any tests for the new APIs or functionality. Where are they?

Our external server doesn't support web sockets over H/2, so I can add only loopback. And interop tests with Kestrel after this will be merged

[stephentoub]

so I can add only loopback

Right, that's what I was expecting.

[CarnaViire]

@stephentoub is there a reason why we never cached the whole invoker, only the handler? We'd create it and just throw away for finalization

runtime/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs

Line 143 in 9768606

response = await new HttpMessageInvoker(handler).SendAsync(request, externalAndAbortCancellation.Token).ConfigureAwait(false);

[stephentoub]

Not to my knowledge.

[CarnaViire]

Please correct me if I'm wrong, but the exception seems to contain SETTINGS_ENABLE_CONNECT_PROTOCOL only when H2 connection is established but settings did not contain enabling extended connect? What will happen if H2 connection was not established at all (if server is H1 only)?

Also, I am a bit concerned about the whole while(true) approach. The conditions on when we exit or continue looping are a bit obscured.
We only have 3 options: try only H2, try H2 and then H1, try only H1. Meaning, we only have 2 steps, and the second will only be called if the first was not allowed or not succeeded in a special way. Can we unwind the loop? Something like

res = null
fail if unexpected versions
if (can try H2)
{
    try
    {
        res = do H2 stuff
    }
    catch when can downgrade
}
if (res == null) // either H2 not requested or it failed with downgrade enabled
{
    res = do H1 stuff
}
do stuff with res

Please let me know if I miss something

[greenEkatherine]

What will happen if H2 connection was not established at all (if server is H1 only)?

In that case we get HttpRequestException that I cannot separate from other reasons. I'm not sure if I can add Data to the exception that is already in use?

[stephentoub]

Do we have tests that validate:

• sending/receiving with HTTP/2 after connecting?
• downgrading?

[greenEkatherine]

Send/receive test for web socket stream is added and now I'm struggling to emulate downgrading, not sure if I can make it today.

[CarnaViire]

This is not needed anywhere, is it?

[greenEkatherine]

oops, I'll remove it