Add additional close statuses in ManagedWebSocket (#83827)

* Allow non RFC  close statuses in ManagedWebSocket

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`.
These codes are documented [here as IANA registered codes](https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code) as valid server-initiated close reasons.

Fixes Issue https://github.com/dotnet/runtime/issues/82602

* Cleanup comment format

Co-authored-by: MartyIX <203266+MartyIX@users.noreply.github.com>
* Rename test data to CloseStatuses

Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>
* Rename test method to follow naming convention.

Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>
* Addressed PR feedback

Finished rename of CloseStatuses test data
Renamed `closeStatusDescription` to `serverMessage`
Send hello message and close status then await both responses and
check they are as expected. This necessitated switching to the `ReceiveAsync` that accepts an `ArraySegment`.
Explicitly typed `var`s
Inlined helper methods (for clarity)

* Rename local for per PR feedback

Swapping back to a distinct and more appropriately named variable for the `closeStatusDescription`

Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>
* Label the boolean for isServer flag

Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>
* Use better local variable name

Renamed local `serverMessage` back to `closeStatusDescription` per PR feedback.
Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>
* Address PR and rebase to main

Rebased to current main, updated the commit messages and added the remaining changes to address the PR comments.

---------

Co-authored-by: MartyIX <203266+MartyIX@users.noreply.github.com>
Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>

diff --git a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs
index 48a55b4..114cbbe 100644 (file)
--- a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs
+++ b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs
@@ -1059,6 +1059,9 @@ namespace System.Net.WebSockets
                 case WebSocketCloseStatus.NormalClosure:
                 case WebSocketCloseStatus.PolicyViolation:
                 case WebSocketCloseStatus.ProtocolError:
+                case (WebSocketCloseStatus)1012: // ServiceRestart
+                case (WebSocketCloseStatus)1013: // TryAgainLater
+                case (WebSocketCloseStatus)1014: // BadGateway
                     return true;
 
                 default:

diff --git a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/WebSocketCloseStatus.cs b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/WebSocketCloseStatus.cs
index 235dc86..0608a83 100644 (file)
--- a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/WebSocketCloseStatus.cs
+++ b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/WebSocketCloseStatus.cs
@@ -18,6 +18,10 @@ namespace System.Net.WebSockets
         MessageTooBig = 1009,
         MandatoryExtension = 1010,
         InternalServerError = 1011
+        // non-RFC IANA registered status codes that we allow as valid closing status
+        // 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
         // TLSHandshakeFailed = 1015, // 1015 is reserved and should never be used by user
 
         // 0 - 999 Status codes in the range 0-999 are not used.

diff --git a/src/libraries/System.Net.WebSockets/tests/System.Net.WebSockets.Tests.csproj b/src/libraries/System.Net.WebSockets/tests/System.Net.WebSockets.Tests.csproj
index 8af7c5b..f49bdcb 100644 (file)
--- a/src/libraries/System.Net.WebSockets/tests/System.Net.WebSockets.Tests.csproj
+++ b/src/libraries/System.Net.WebSockets/tests/System.Net.WebSockets.Tests.csproj
@@ -6,6 +6,7 @@
     <RdXmlFile Include="default.rd.xml" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="WebSocketCloseTests.cs" />
     <Compile Include="WebSocketTests.cs" />
     <Compile Include="WebSocketExceptionTests.cs" />
     <Compile Include="WebSocketReceiveResultTests.cs" />

diff --git a/src/libraries/System.Net.WebSockets/tests/WebSocketCloseTests.cs b/src/libraries/System.Net.WebSockets/tests/WebSocketCloseTests.cs
new file mode 100644 (file)
index 0000000..86d1dfb
--- /dev/null
+++ b/src/libraries/System.Net.WebSockets/tests/WebSocketCloseTests.cs
@@ -0,0 +1,89 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace System.Net.WebSockets.Tests
+{
+    public class WebSocketCloseTests
+    {
+        private readonly CancellationTokenSource? _cancellation;
+
+        public WebSocketCloseTests()
+        {
+            if (!Debugger.IsAttached)
+            {
+                _cancellation = new CancellationTokenSource(TimeSpan.FromSeconds(5));
+            }
+        }
+
+        public CancellationToken CancellationToken => _cancellation?.Token ?? default;
+
+        public static object[][] CloseStatuses = {
+            new object[] { WebSocketCloseStatus.EndpointUnavailable },
+            new object[] { WebSocketCloseStatus.InternalServerError },
+            new object[] { WebSocketCloseStatus.InvalidMessageType},
+            new object[] { WebSocketCloseStatus.InvalidPayloadData },
+            new object[] { WebSocketCloseStatus.MandatoryExtension },
+            new object[] { WebSocketCloseStatus.MessageTooBig },
+            new object[] { WebSocketCloseStatus.NormalClosure },
+            new object[] { WebSocketCloseStatus.PolicyViolation },
+            new object[] { WebSocketCloseStatus.ProtocolError },
+            new object[] { (WebSocketCloseStatus)1012 },  // ServiceRestart indicates that the server / service is restarting.
+            new object[] { (WebSocketCloseStatus)1013 },  // TryAgainLater indicates that a temporary server condition forced blocking the client's request.
+            new object[] { (WebSocketCloseStatus)1014 },  // BadGateway indicates that the server acting as gateway received an invalid response
+        };
+
+        [Theory]
+        [MemberData(nameof(CloseStatuses))]
+        public void WebSocketReceiveResult_WebSocketCloseStatus_Roundtrip(WebSocketCloseStatus closeStatus)
+        {
+            string closeStatusDescription = "closeStatus " + closeStatus.ToString();
+            WebSocketReceiveResult wsrr = new WebSocketReceiveResult(42, WebSocketMessageType.Close, endOfMessage: true, closeStatus, closeStatusDescription);
+            Assert.Equal(42, wsrr.Count);
+            Assert.Equal(closeStatus, wsrr.CloseStatus);
+            Assert.Equal(closeStatusDescription, wsrr.CloseStatusDescription);
+        }
+
+        [Theory]
+        [MemberData(nameof(CloseStatuses))]
+        public async Task ReceiveAsync_ValidCloseStatus_Success(WebSocketCloseStatus closeStatus)
+        {
+            byte[] receiveBuffer = new byte[1024];
+            WebSocketTestStream stream = new();
+            Encoding encoding = Encoding.UTF8;
+
+            using (WebSocket server = WebSocket.CreateFromStream(stream, isServer: true, subProtocol: null, TimeSpan.FromSeconds(3)))
+            using (WebSocket client = WebSocket.CreateFromStream(stream.Remote, isServer: false, subProtocol: null, TimeSpan.FromSeconds(3)))
+            {
+                Assert.NotNull(server);
+                Assert.NotNull(client);
+
+                // send something
+                string hello = "Testing " + closeStatus.ToString();
+                byte[] sendBytes = encoding.GetBytes(hello);
+                await server.SendAsync(sendBytes.AsMemory(), WebSocketMessageType.Text, WebSocketMessageFlags.None, CancellationToken);
+
+                // and then server-side close with the test status
+                string closeStatusDescription = "CloseStatus " + closeStatus.ToString();
+                await server.CloseOutputAsync(closeStatus, closeStatusDescription, CancellationToken);
+
+                // get the hello from the client (after the close message was sent)
+                WebSocketReceiveResult result = await client.ReceiveAsync(new ArraySegment<byte>(receiveBuffer), CancellationToken);
+                Assert.Equal(WebSocketMessageType.Text, result.MessageType);
+                string response = encoding.GetString(receiveBuffer.AsSpan(0, result.Count));
+                Assert.Equal(hello, response);
+
+                // now look for the expected close status
+                WebSocketReceiveResult closing = await client.ReceiveAsync(new ArraySegment<byte>(receiveBuffer), CancellationToken);
+                Assert.Equal(WebSocketMessageType.Close, closing.MessageType);
+                Assert.Equal(closeStatus, closing.CloseStatus);
+                Assert.Equal(closeStatusDescription, closing.CloseStatusDescription);
+            }
+        }
+    }
+}