Allow nonstandard close statuses in ManagedWebSocket

[IDisposable]

Add ServiceRestart (1012), TryAgainLater (1013), and BadGateway (1014) to the list of WebSocketCloseStatus values and allow them to be used as valid WebSocket close statuses so we don't reject the close and discard the status description by adding them to the private IsValueCloseStatus method switch statement declaring them as valid true.

Fixes Issue #82602

Description

The current implementation of ManagedWebSocket explicitly declares some WebSocketCloseStatus values as "acceptable" reasons to close the socket. For those, the information status description is extracted and made available. For all other values, the close status is rejected and the closing information is discarded.

This means that things like BadGateway (1014), or ServiceRestart (1012), or TryAgainLater (1013) will be declared invalid by ManagedWebSocket.IsValidCloseStatus and thus not handled cleanly even though they could happen after a web socket is completely setup. These codes are documented here as valid server-initiated close reasons.

Customer Impact

Lost information and ragged closing of a web socket when a server-side or proxy closes because of any of the previously rejected values:

• ServiceRestart = 1012 // indicates that the server / service is restarting.
• TryAgainLater = 1013 // indicates that a temporary server condition forced blocking the client's request.
• BadGateway = 1014 // indicates that the server acting as gateway received an invalid response

This codes are documented here and here as IANA registered and in the Mozilla docs

Regression

No

Testing

Added test cases for all three to new WebSocketCloseTests.cs file with no regressions in current tests.

Risk

This does not change the public enum of WebSocketCloseStatus as we don't want to invoke public review and the values are not mentioned in the RFC.

Package authoring signed off?

N/A

[ghost]

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

Issue Details

---

Add ServiceRestart (1012), TryAgainLater (1013), and BadGateway (1014) to the list of WebSocketCloseStatus values and allow them to be used as valid WebSocket close statuses so we don't reject the close and discard the status description by adding them to the private IsValueCloseStatus method switch statement declaring them as valid true.

Fixes Issue #82602

Description

The current implementation of ManagedWebSocket explicitly declares some WebSocketCloseStatus values as "acceptable" reasons to close the socket. For those, the information status decsription is extracted and made available. For all other values, the close status is rejected and the closing information is discarded.

This means that things like BadGateway (1014), or ServiceRestart (1012), or TryAgainLater (1013) will be declared invalid by ManagedWebSocket.IsValidCloseStatus and thus not handled cleanly even though they could happen after a web socket is completely setup.

Customer Impact

Lost information and ragged closing of a web socket when a server-side or proxy closes because of any of the previously rejected values:

• ServiceRestart = 1012 // indicates that the server / service is restarting.
• TryAgainLater = 1013 // indicates that a temporary server condition forced blocking the client's request.
• BadGateway = 1014 // indicates that the server acting as gateway received an invalid response

Regression

No

Testing

Added test cases for all three to the HttpListenerWebSocketTests.cs file with no regressions found.

Risk

Changes the public exposed enum of WebSocketCloseStatus.cs adding three new values that were previously not documented.

Package authoring signed off?

IMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:	IDisposable
Assignees:	-
Labels:	area-System.Net, community-contribution
Milestone:	-

[CarnaViire]

Thank you for your contribution @IDisposable!

However, you cannot add public API without going through the API review process first.

I strongly believe the issue can be fixed without adding new enum values, just by casting ints to WebSocketCloseStatus in the checks and tests. Please let me know what kinds of problems you are experiencing with this -- I would be happy to help.

Also, when adding the tests, could you please add them to the library that you were changing, i.e. System.Net.WebSockets, not to HttpListener?
You can test receiving the close status by supplying a MemoryStream with the close frame bits written into it, in the same way this test is constructed:

runtime/src/libraries/System.Net.WebSockets/tests/WebSocketCreateTest.cs

Lines 153 to 160 in 8fe12bc

	public async Task ReceiveAsync_InvalidPayloadLength_AbortsAndThrowsException(byte[] lenBytes, bool shouldFail)
	{
	var frame = new byte[11];
	frame[0] = 0b1_000_0010; // FIN, RSV, OPCODE
	frame[1] = 0b0_1111111; // MASK, PAYLOAD_LEN
	Assert.Equal(8, lenBytes.Length);
	Array.Copy(lenBytes, 0, frame, 2, lenBytes.Length); // EXTENDED_PAYLOAD_LEN
	frame[10] = (byte)'a';

[IDisposable]

Reworking this to not use public API changes... PR will follow in 12 36 hours or so

[IDisposable]

Replaced by #83827

[IDisposable]

@CarnaViire I reimplemented this PR without changing the public enum.