From 1f90a64f8063cf61c25effeed005b5eefc364d73 Mon Sep 17 00:00:00 2001 From: Kenneth Pouncey Date: Thu, 3 Dec 2020 10:25:13 +0100 Subject: [PATCH 1/5] [browser][websocket] Fix for Close Description validation - Issue https://github.com/dotnet/runtime/issues/45531 - The description validation exception was not being percolated through to the unit test --- .../BrowserWebSockets/BrowserWebSocket.cs | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index b767c3149bee43..78319744a66239 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -355,7 +355,7 @@ public override Task SendAsync(ArraySegment buffer, WebSocketMessageType m } WebSocketValidate.ValidateArraySegment(buffer, nameof(buffer)); - + //System.Diagnostics.Debug.WriteLine($"we are here with a buffer {_writeBuffer is null}"); if (!endOfMessage) { _writeBuffer ??= new MemoryStream(); @@ -379,6 +379,21 @@ public override Task SendAsync(ArraySegment buffer, WebSocketMessageType m } } + // System.Runtime.InteropServices.JavaScript.Array writtenBufferArray = new System.Runtime.InteropServices.JavaScript.Array(); + // string strBufferss = buffer.Array == null ? string.Empty : Encoding.UTF8.GetString(buffer.Array, buffer.Offset, buffer.Count); + // writtenBufferArray.Push(strBufferss); + // writtenBufferArray.Push(strBufferss); + // JSObject textPlain = new JSObject(); + + // textPlain.SetObjectProperty("type", "text/plain"); + // var args = new System.Runtime.InteropServices.JavaScript.Array(); + // args.Push(strBufferss); + // HostObject writtenBufferBlob1 = new HostObject("Blob", args, textPlain ); + // args.Push(writtenBufferBlob1); + // HostObject writtenBufferBlob = new HostObject("Blob", args, textPlain ); + + // System.Diagnostics.Debug.WriteLine($"we are here with a blob {writtenBufferArray}"); + try { switch (messageType) @@ -391,7 +406,9 @@ public override Task SendAsync(ArraySegment buffer, WebSocketMessageType m break; default: string strBuffer = buffer.Array == null ? string.Empty : Encoding.UTF8.GetString(buffer.Array, buffer.Offset, buffer.Count); + //_innerWebSocket!.Invoke("send", writtenBufferArray); _innerWebSocket!.Invoke("send", strBuffer); + //_innerWebSocket!.Invoke("send", writtenBufferBlob); break; } } @@ -446,24 +463,32 @@ public override void Abort() Dispose(); } - public override async Task CloseAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) + public override Task CloseAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) { _writeBuffer = null; ThrowIfNotConnected(); - await CloseAsyncCore(closeStatus, statusDescription, cancellationToken).ConfigureAwait(continueOnCapturedContext: true); - } - - private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) - { - ThrowOnInvalidState(State, WebSocketState.Open, WebSocketState.CloseReceived, WebSocketState.CloseSent); WebSocketValidate.ValidateCloseStatus(closeStatus, statusDescription); + try + { + ThrowOnInvalidState(State, WebSocketState.Open, WebSocketState.CloseReceived, WebSocketState.CloseSent); + } + catch (Exception exc) + { + return Task.FromException(exc); + } + + return CloseAsyncCore(closeStatus, statusDescription, cancellationToken); + } + + private Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) + { _tcsClose = new TaskCompletionSource(); _innerWebSocketCloseStatus = closeStatus; _innerWebSocketCloseStatusDescription = statusDescription; _innerWebSocket!.Invoke("close", (int)closeStatus, statusDescription); - await _tcsClose.Task.ConfigureAwait(continueOnCapturedContext: true); + return _tcsClose.Task; } public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) => throw new PlatformNotSupportedException(); From 64970cccb319d33b8f0cde5796231ef23a61ddff Mon Sep 17 00:00:00 2001 From: Kenneth Pouncey Date: Thu, 3 Dec 2020 10:32:43 +0100 Subject: [PATCH 2/5] Remove test code --- .../BrowserWebSockets/BrowserWebSocket.cs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index 78319744a66239..2725edd030d526 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -355,7 +355,7 @@ public override Task SendAsync(ArraySegment buffer, WebSocketMessageType m } WebSocketValidate.ValidateArraySegment(buffer, nameof(buffer)); - //System.Diagnostics.Debug.WriteLine($"we are here with a buffer {_writeBuffer is null}"); + if (!endOfMessage) { _writeBuffer ??= new MemoryStream(); @@ -379,21 +379,6 @@ public override Task SendAsync(ArraySegment buffer, WebSocketMessageType m } } - // System.Runtime.InteropServices.JavaScript.Array writtenBufferArray = new System.Runtime.InteropServices.JavaScript.Array(); - // string strBufferss = buffer.Array == null ? string.Empty : Encoding.UTF8.GetString(buffer.Array, buffer.Offset, buffer.Count); - // writtenBufferArray.Push(strBufferss); - // writtenBufferArray.Push(strBufferss); - // JSObject textPlain = new JSObject(); - - // textPlain.SetObjectProperty("type", "text/plain"); - // var args = new System.Runtime.InteropServices.JavaScript.Array(); - // args.Push(strBufferss); - // HostObject writtenBufferBlob1 = new HostObject("Blob", args, textPlain ); - // args.Push(writtenBufferBlob1); - // HostObject writtenBufferBlob = new HostObject("Blob", args, textPlain ); - - // System.Diagnostics.Debug.WriteLine($"we are here with a blob {writtenBufferArray}"); - try { switch (messageType) @@ -406,9 +391,7 @@ public override Task SendAsync(ArraySegment buffer, WebSocketMessageType m break; default: string strBuffer = buffer.Array == null ? string.Empty : Encoding.UTF8.GetString(buffer.Array, buffer.Offset, buffer.Count); - //_innerWebSocket!.Invoke("send", writtenBufferArray); _innerWebSocket!.Invoke("send", strBuffer); - //_innerWebSocket!.Invoke("send", writtenBufferBlob); break; } } From 58e45978a4d3db29f78b2595764842878c390696 Mon Sep 17 00:00:00 2001 From: Kenneth Pouncey Date: Fri, 4 Dec 2020 11:46:38 +0100 Subject: [PATCH 3/5] Address review comments. Wrap in a try/catch that will catch that exception and store it into a task to be returned. --- .../BrowserWebSockets/BrowserWebSocket.cs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index 2725edd030d526..d09ad4fa0f6757 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -467,11 +467,18 @@ public override Task CloseAsync(WebSocketCloseStatus closeStatus, string? status private Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) { - _tcsClose = new TaskCompletionSource(); - _innerWebSocketCloseStatus = closeStatus; - _innerWebSocketCloseStatusDescription = statusDescription; - _innerWebSocket!.Invoke("close", (int)closeStatus, statusDescription); - return _tcsClose.Task; + try + { + _tcsClose = new TaskCompletionSource(); + _innerWebSocketCloseStatus = closeStatus; + _innerWebSocketCloseStatusDescription = statusDescription; + _innerWebSocket!.Invoke("close", (int)closeStatus, statusDescription); + return _tcsClose.Task; + } + catch (Exception exc) + { + return Task.FromException(exc); + } } public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) => throw new PlatformNotSupportedException(); From 2b9d8629d957d12850c22a6b05326ec49835a715 Mon Sep 17 00:00:00 2001 From: Kenneth Pouncey Date: Wed, 9 Dec 2020 10:43:18 +0100 Subject: [PATCH 4/5] Fix merge conflict --- .../System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index 8cf32a3d095baa..4288444f2de37b 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -482,7 +482,7 @@ public override Task CloseAsync(WebSocketCloseStatus closeStatus, string? status return CloseAsyncCore(closeStatus, statusDescription, cancellationToken); } - private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) + private Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) { try { From c78e2f79d15f627451427b610e4ee6a2d0f3dbd9 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Thu, 10 Dec 2020 22:54:34 -0600 Subject: [PATCH 5/5] Remove active issue --- src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs index 0acdbf73cca10e..0e50e787d58732 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs @@ -103,7 +103,6 @@ public async Task CloseAsync_CloseDescriptionIsMaxLength_Success(Uri server) [OuterLoop("Uses external server")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45531", TestPlatforms.Browser)] public async Task CloseAsync_CloseDescriptionIsMaxLengthPlusOne_ThrowsArgumentException(Uri server) { string closeDescription = new string('C', CloseDescriptionMaxLength + 1);