Skip to content

Conversation

BrennanConroy
Copy link
Member

Fixes https://github.com/dotnet/AspNetCore-ManualTests/issues/2688

Blazor enabled websocket compression by default and it turns out our forwarder in IIS OutOfProcess doesn't support websocket compression. Both the WinHttp and websocket.dll layers (which we use) do not support the compression framing.

The WinHttp layer hard codes sending 0 Sec-WebSocket-Extensions settings to websocket.dll but since we are just forwarding the client request which might contain a Sec-WebSocket-Extensions header, the server might then return a Sec-WebSocket-Extensions header in the response which will be rejected by websocket.dll due to it expecting 0 extensions from the earlier WinHttp call.

So the fix is to just remove the Sec-WebSocket-Extensions header from the request (which is effectively what WinHttp did without actually modifying the headers) so the server won't potentially respond with any Sec-WebSocket-Extensions header in the response.

Also, did a bunch of test work to move the websocket tests out of just IISExpress and to enable OutOfProcess testing as well.

@BrennanConroy BrennanConroy added feature-iis Includes: IIS, ANCM area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions labels Nov 8, 2024

// WinHttp does not support any extensions being returned by the server, so we remove the request header to avoid the server
// responding with any accepted extensions.
pRequest->DeleteHeader("Sec-WebSocket-Extensions");
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming this is fine even if the header doesn't exist?

Copy link
Member Author

Choose a reason for hiding this comment

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

The doc comments don't discuss it, but looking at the code it seems to check for existence internally. Also, the non-compression websocket tests should cover that scenario.

@BrennanConroy BrennanConroy marked this pull request as ready for review November 9, 2024 03:11
@BrennanConroy BrennanConroy merged commit f526f0c into main Nov 13, 2024
27 checks passed
@BrennanConroy BrennanConroy deleted the brecon/wscomp branch November 13, 2024 22:05
@BrennanConroy
Copy link
Member Author

/backport to release/9.0

@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Nov 13, 2024
@github-actions
Copy link
Contributor

Started backporting to release/9.0: https://github.com/dotnet/aspnetcore/actions/runs/11826616953

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-iis Includes: IIS, ANCM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants