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

WebSockets over HTTP/2 issues #72301

Closed
BrennanConroy opened this issue Jul 15, 2022 · 6 comments · Fixed by #73222 or #73762
Closed

WebSockets over HTTP/2 issues #72301

BrennanConroy opened this issue Jul 15, 2022 · 6 comments · Fixed by #73222 or #73762
Assignees
Milestone

Comments

@BrennanConroy
Copy link
Member

Description

The new WebSockets over HTTP/2 feature doesn't work with ASP.NET Core 7.

I believe it is because this line

await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null), mustFlush), static (s, writeBuffer) =>

Sepcifically: endStream: (request.Content == null)
Causes the END_STREAM flag to be set when sending headers for the CONNECT request.

dbug: Microsoft.AspNetCore.Server.Kestrel.Core.Internal.LoggingConnectionMiddleware[0]
      ReadAsync[83]
      00 00 4A 01 05 00 00 00  01 02 07 43 4F 4E 4E 45   ..J..... ...CONNE
      43 54 87 01 0E 6C 6F 63  61 6C 68 6F 73 74 3A 37   CT...loc alhost:7
      32 30 34 84 00 09 3A 70  72 6F 74 6F 63 6F 6C 09   204...:p rotocol.
      77 65 62 73 6F 63 6B 65  74 00 15 73 65 63 2D 77   websocke t..sec-w
      65 62 73 6F 63 6B 65 74  2D 76 65 72 73 69 6F 6E   ebsocket -version
      02 31 33                                           .13
trce: Microsoft.AspNetCore.Server.Kestrel.Http2[37]
      Connection id "0HMJ6N3UAOPRE" received HEADERS frame for stream ID 1 with length 74 and flags END_STREAM, END_HEADERS.

cc @greenEkatherine

Additional strange behavior, if you use the ClientWebSocket without passing in an HttpClientHandler then the ConnectAsync call does not throw, but the websocket is closed on the server side.

Reproduction Steps

Client project

var webSocket = new ClientWebSocket();
webSocket.Options.HttpVersion = HttpVersion.Version20;
var httpClientHandler = new HttpClientHandler();
await webSocket.ConnectAsync(url, new HttpClient(httpClientHandler), default).ConfigureAwait(false);

Server project

var builder = WebApplication.CreateBuilder(args);
builder.Logging.SetMinimumLevel(LogLevel.Trace);
builder.WebHost.ConfigureKestrel(o =>
{
    o.ConfigureEndpointDefaults(l =>
    {
        l.UseHttps();
        l.UseConnectionLogging();
    });
});
var app = builder.Build();

app.UseWebSockets();

app.Run(async context =>
{
    var websocket = await context.WebSockets.AcceptWebSocketAsync();
    var buf = await websocket.ReceiveAsync(Memory<byte>.Empty, default);
});

app.Run();

Expected behavior

Able to connect to a WebSocket endpoint over HTTP/2.

Actual behavior

Unhandled exception. System.AggregateException: One or more errors occurred. (Unable to connect to the remote server)
 ---> System.Net.WebSockets.WebSocketException (0x80004005): Unable to connect to the remote server
 ---> System.Net.Http.HttpRequestException: Error while copying content to a stream.
 ---> System.Net.Http.HttpProtocolException: The HTTP/2 server reset the stream. HTTP/2 error code 'INTERNAL_ERROR' (0x2).
   at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
   at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
   at System.Net.Http.Http2Connection.Http2Stream.TryReadFromBuffer(Span`1 buffer, Boolean partOfSyncRead)
   at System.Net.Http.Http2Connection.Http2Stream.CopyToAsync(HttpResponseMessage responseMessage, Stream destination, Int32 bufferSize, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionResponseContent.<SerializeToStreamAsync>g__Impl|6_0(Stream stream, TransportContext context, CancellationToken cancellationToken)
   at System.Net.Http.HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
   --- End of inner exception stack trace ---
   at System.Net.Http.HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at System.Net.WebSockets.WebSocketHandle.ConnectAsync(Uri uri, HttpMessageInvoker invoker, CancellationToken cancellationToken, ClientWebSocketOptions options)
   at System.Net.WebSockets.WebSocketHandle.ConnectAsync(Uri uri, HttpMessageInvoker invoker, CancellationToken cancellationToken, ClientWebSocketOptions options)
   at System.Net.WebSockets.ClientWebSocket.ConnectAsyncCore(Uri uri, HttpMessageInvoker invoker, CancellationToken cancellationToken)

Regression?

No

Known Workarounds

None

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 15, 2022
@ghost
Copy link

ghost commented Jul 15, 2022

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

Issue Details

Description

The new WebSockets over HTTP/2 feature doesn't work with ASP.NET Core 7.

I believe it is because this line

await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null), mustFlush), static (s, writeBuffer) =>

Sepcifically: endStream: (request.Content == null)
Causes the END_STREAM flag to be set when sending headers for the CONNECT request.

dbug: Microsoft.AspNetCore.Server.Kestrel.Core.Internal.LoggingConnectionMiddleware[0]
      ReadAsync[83]
      00 00 4A 01 05 00 00 00  01 02 07 43 4F 4E 4E 45   ..J..... ...CONNE
      43 54 87 01 0E 6C 6F 63  61 6C 68 6F 73 74 3A 37   CT...loc alhost:7
      32 30 34 84 00 09 3A 70  72 6F 74 6F 63 6F 6C 09   204...:p rotocol.
      77 65 62 73 6F 63 6B 65  74 00 15 73 65 63 2D 77   websocke t..sec-w
      65 62 73 6F 63 6B 65 74  2D 76 65 72 73 69 6F 6E   ebsocket -version
      02 31 33                                           .13
trce: Microsoft.AspNetCore.Server.Kestrel.Http2[37]
      Connection id "0HMJ6N3UAOPRE" received HEADERS frame for stream ID 1 with length 74 and flags END_STREAM, END_HEADERS.

cc @greenEkatherine

Additional strange behavior, if you use the ClientWebSocket without passing in an HttpClientHandler then the ConnectAsync call does not throw, but the websocket is closed on the server side.

Reproduction Steps

Client project

var webSocket = new ClientWebSocket();
webSocket.Options.HttpVersion = HttpVersion.Version20;
var httpClientHandler = new HttpClientHandler();
await webSocket.ConnectAsync(url, new HttpClient(httpClientHandler), default).ConfigureAwait(false);

Server project

var builder = WebApplication.CreateBuilder(args);
builder.Logging.SetMinimumLevel(LogLevel.Trace);
builder.WebHost.ConfigureKestrel(o =>
{
    o.ConfigureEndpointDefaults(l =>
    {
        l.UseHttps();
        l.UseConnectionLogging();
    });
});
var app = builder.Build();

app.UseWebSockets();

app.Run(async context =>
{
    var websocket = await context.WebSockets.AcceptWebSocketAsync();
    var buf = await websocket.ReceiveAsync(Memory<byte>.Empty, default);
});

app.Run();

Expected behavior

Able to connect to a WebSocket endpoint over HTTP/2.

Actual behavior

Unhandled exception. System.AggregateException: One or more errors occurred. (Unable to connect to the remote server)
 ---> System.Net.WebSockets.WebSocketException (0x80004005): Unable to connect to the remote server
 ---> System.Net.Http.HttpRequestException: Error while copying content to a stream.
 ---> System.Net.Http.HttpProtocolException: The HTTP/2 server reset the stream. HTTP/2 error code 'INTERNAL_ERROR' (0x2).
   at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
   at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
   at System.Net.Http.Http2Connection.Http2Stream.TryReadFromBuffer(Span`1 buffer, Boolean partOfSyncRead)
   at System.Net.Http.Http2Connection.Http2Stream.CopyToAsync(HttpResponseMessage responseMessage, Stream destination, Int32 bufferSize, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionResponseContent.<SerializeToStreamAsync>g__Impl|6_0(Stream stream, TransportContext context, CancellationToken cancellationToken)
   at System.Net.Http.HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
   --- End of inner exception stack trace ---
   at System.Net.Http.HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at System.Net.WebSockets.WebSocketHandle.ConnectAsync(Uri uri, HttpMessageInvoker invoker, CancellationToken cancellationToken, ClientWebSocketOptions options)
   at System.Net.WebSockets.WebSocketHandle.ConnectAsync(Uri uri, HttpMessageInvoker invoker, CancellationToken cancellationToken, ClientWebSocketOptions options)
   at System.Net.WebSockets.ClientWebSocket.ConnectAsyncCore(Uri uri, HttpMessageInvoker invoker, CancellationToken cancellationToken)

Regression?

No

Known Workarounds

None

Configuration

No response

Other information

No response

Author: BrennanConroy
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@greenEkatherine greenEkatherine self-assigned this Jul 18, 2022
@karelz
Copy link
Member

karelz commented Jul 19, 2022

Triage: Incorrect sending of final flag. We should fix it.

@karelz karelz added this to the 7.0.0 milestone Jul 19, 2022
@karelz karelz added bug and removed untriaged New issue has not been triaged by the area owner labels Jul 19, 2022
@greenEkatherine
Copy link
Contributor

I tested the same scenario with HttpMessageInvoker and it works, it seems that using HttpClient is the root cause as in #72476

@BrennanConroy you mentioned that it failed without handler parameter, could you please provide more details? I cannot reproduce it - connect and send-receive with local server work for me. Is it the same setup, with or without TLS?

@BrennanConroy
Copy link
Member Author

BrennanConroy commented Jul 26, 2022

I tested the same scenario with HttpMessageInvoker and it works

It works because it falls back to HTTP/1.1 (i.e. it fails HTTP/2.0 which is what this issue is about).
So set webSocket.Options.HttpVersionPolicy = HttpVersionPolicy.RequestVersionExact; to see the failure.

Edit: Passing an HttpMessageInvoker has the same issue as not passing a handler, the server closes the websocket but the client "sends" perfectly fine. What local server are you using?

you mentioned that it failed without handler parameter, could you please provide more details?

Tried again and not seeing any issues in this case (besides throwing for HTTP/2.0).
Edit: Whoops, wrong server version. Tried again and see the issue.

var webSocket = new ClientWebSocket();
webSocket.Options.HttpVersion = HttpVersion.Version20;
await webSocket.ConnectAsync(new Uri("wss://localhost:7204"), default).ConfigureAwait(false);

// this call "succeeds" but the server has closed the websocket connection due to the previous END_STREAM flag
await webSocket.SendAsync(new byte[] { 35, 36, 37 }, WebSocketMessageType.Binary, true, default);
// throws
var res = await webSocket.ReceiveAsync(Array.Empty<byte>(), default);

@greenEkatherine
Copy link
Contributor

greenEkatherine commented Jul 26, 2022

What local server are you using?

Kestrel 7.0.0-rc.1.22368.6

Ah, I see it now. It is strange that switching the order first receive and than send work

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 2, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 5, 2022
@BrennanConroy
Copy link
Member Author

Reopening as the issue does not seem fixed, see #73222 (comment)

@BrennanConroy BrennanConroy reopened this Aug 10, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 11, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 12, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.