Skip to content

Fix IIS outofprocess to remove WebSocket compression handshake #58846

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 2 commits into from
Nov 13, 2024

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
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