Skip to content
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

ClientWebSocket.Connect hangs indefinitely if using HttpVersionPolicy.RequestVersionOrHigher on wss connections #111977

Open
ArXen42 opened this issue Jan 29, 2025 · 2 comments

Comments

@ArXen42
Copy link

ArXen42 commented Jan 29, 2025

Description

ClientWebSocket hangs when attempting to connect to HTTP/1.1 WebSocket server using RequestVersionOrHigher policy. If this is expected behavior, it should probably be documented.

Reproduction Steps

This will hang unless HttpVersionPolicy setting is commented out:

using System.Net.WebSockets;

using var ws = new ClientWebSocket();
ws.Options.HttpVersion       = new(1, 1);
ws.Options.HttpVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher; // <--- causes problem

using var handler = new SocketsHttpHandler();
handler.ConnectTimeout = TimeSpan.FromSeconds(10);

using var invoker = new HttpMessageInvoker(handler);
await ws.ConnectAsync(new("wss://echo.websocket.org"), invoker, CancellationToken.None); // <-- hangs
Console.WriteLine(ws.State);

Expected behavior

Correctly determining that server doesn't support HTTP/2 and falling back to HTTP/1.1.

Actual behavior

Hangs indefinitely.

Regression?

Didn't test on .NET 7 yet, but seems to behave similarly on .NET 8 and .NET 9.

Known Workarounds

Using inverse options seems to work:

ws.Options.HttpVersion       = new(2, 0);
ws.Options.HttpVersionPolicy = HttpVersionPolicy.RequestVersionOrLower;

Configuration

Tested on .NET 8, 9: win-x64, linux-x64, linux-arm64.

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 29, 2025
Copy link
Contributor

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

@MihaZupan MihaZupan added the bug label Jan 29, 2025
@MihaZupan
Copy link
Member

MihaZupan commented Jan 29, 2025

I think this is due to a lack of () in our version selection code here:

if (!tryDowngrade && options.HttpVersion >= HttpVersion.Version20
|| (options.HttpVersion == HttpVersion.Version11 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && uri.Scheme == UriScheme.Wss))

Right now it's effectively
(!tryDowngrade && options.HttpVersion >= HttpVersion.Version20) || (the rest)
instead of
!tryDowngrade && (options.HttpVersion >= HttpVersion.Version20 || the rest)

So we just keep retrying H2.

Clearly we're lacking test coverage here.

@MihaZupan MihaZupan added this to the 10.0.0 milestone Jan 29, 2025
@MihaZupan MihaZupan removed the untriaged New issue has not been triaged by the area owner label Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants