Browse Source

Stop enforcing JSON read till end if `EndInvokeJS` returns early (#39060)

* mono_crash.*.blob

* Stop enforcing read till end if EndInvoke returns early

* Tests

* Test name change

* Fix tests

* PR Feedback @pranavkm
Tanay Parikh 4 years ago
parent
commit
fa368788c2

+ 1 - 0
.gitignore

@@ -43,3 +43,4 @@ UpgradeLog.htm
 .idea
 *.svclog
 mono_crash.*.json
+mono_crash.*.blob

+ 4 - 1
src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs

@@ -306,7 +306,10 @@ public static class DotNetDispatcher
         var success = reader.GetBoolean();
 
         reader.Read();
-        jsRuntime.EndInvokeJS(taskId, success, ref reader);
+        if (!jsRuntime.EndInvokeJS(taskId, success, ref reader))
+        {
+            return;
+        }
 
         if (!reader.Read() || reader.TokenType != JsonTokenType.EndArray)
         {

+ 5 - 2
src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs

@@ -225,13 +225,13 @@ public abstract partial class JSRuntime : IJSRuntime, IDisposable
     }
 
     [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2072:RequiresUnreferencedCode", Justification = "We enforce trimmer attributes for JSON deserialized types on InvokeAsync.")]
-    internal void EndInvokeJS(long taskId, bool succeeded, ref Utf8JsonReader jsonReader)
+    internal bool EndInvokeJS(long taskId, bool succeeded, ref Utf8JsonReader jsonReader)
     {
         if (!_pendingTasks.TryRemove(taskId, out var tcs))
         {
             // We should simply return if we can't find an id for the invocation.
             // This likely means that the method that initiated the call defined a timeout and stopped waiting.
-            return;
+            return false;
         }
 
         CleanupTasksAndRegistrations(taskId);
@@ -251,11 +251,14 @@ public abstract partial class JSRuntime : IJSRuntime, IDisposable
                 var exceptionText = jsonReader.GetString() ?? string.Empty;
                 TaskGenericsUtil.SetTaskCompletionSourceException(tcs, new JSException(exceptionText));
             }
+
+            return true;
         }
         catch (Exception exception)
         {
             var message = $"An exception occurred executing JS interop: {exception.Message}. See InnerException for more details.";
             TaskGenericsUtil.SetTaskCompletionSourceException(tcs, new JSException(message, exception));
+            return false;
         }
     }
 

+ 126 - 63
src/JSInterop/Microsoft.JSInterop/test/Infrastructure/DotNetDispatcherTest.cs

@@ -263,7 +263,7 @@ public class DotNetDispatcherTest
     }
 
     [Fact]
-    public void EndInvoke_WithSuccessValue()
+    public void EndInvokeJS_WithSuccessValue()
     {
         // Arrange
         var jsRuntime = new TestJSRuntime();
@@ -282,7 +282,7 @@ public class DotNetDispatcherTest
     }
 
     [Fact]
-    public async Task EndInvoke_WithErrorString()
+    public async Task EndInvokeJS_WithErrorString()
     {
         // Arrange
         var jsRuntime = new TestJSRuntime();
@@ -299,7 +299,7 @@ public class DotNetDispatcherTest
     }
 
     [Fact]
-    public async Task EndInvoke_WithNullError()
+    public async Task EndInvokeJS_WithNullError()
     {
         // Arrange
         var jsRuntime = new TestJSRuntime();
@@ -314,6 +314,129 @@ public class DotNetDispatcherTest
         Assert.Empty(ex.Message);
     }
 
+    [Fact]
+    public void EndInvokeJS_DoesNotThrowJSONExceptionIfTaskCancelled()
+    {
+        // Arrange
+        var jsRuntime = new TestJSRuntime();
+        var testDTO = new TestDTO { StringVal = "Hello", IntVal = 4 };
+        var cts = new CancellationTokenSource();
+        var argsJson = JsonSerializer.Serialize(new object[] { jsRuntime.LastInvocationAsyncHandle, true, testDTO }, jsRuntime.JsonSerializerOptions);
+
+        // Act
+        var task = jsRuntime.InvokeAsync<TestDTO>("unimportant", cts.Token);
+
+        cts.Cancel();
+
+        DotNetDispatcher.EndInvokeJS(jsRuntime, argsJson);
+
+        // Assert
+        Assert.False(task.IsCompletedSuccessfully);
+        Assert.True(task.IsCanceled);
+    }
+
+    [Fact]
+    public void EndInvokeJS_ThrowsIfJsonIsEmptyString()
+    {
+        // Arrange
+        var jsRuntime = new TestJSRuntime();
+        var task = jsRuntime.InvokeAsync<TestDTO>("somemethod");
+
+        // Act & Assert
+        Assert.ThrowsAny<JsonException>(() => DotNetDispatcher.EndInvokeJS(jsRuntime, ""));
+    }
+
+    [Fact]
+    public void EndInvokeJS_ThrowsIfJsonIsNotArray()
+    {
+        // Arrange
+        var jsRuntime = new TestJSRuntime();
+        var task = jsRuntime.InvokeAsync<TestDTO>("somemethod");
+
+        // Act & Assert
+        Assert.ThrowsAny<JsonException>(() => DotNetDispatcher.EndInvokeJS(jsRuntime, $"{{\"key\": \"{jsRuntime.LastInvocationAsyncHandle}\"}}"));
+    }
+
+    [Fact]
+    public void EndInvokeJS_ThrowsIfJsonArrayIsInComplete()
+    {
+        // Arrange
+        var jsRuntime = new TestJSRuntime();
+        var task = jsRuntime.InvokeAsync<TestDTO>("somemethod");
+
+        // Act & Assert
+        Assert.ThrowsAny<JsonException>(() => DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, false"));
+    }
+
+    [Fact]
+    public void EndInvokeJS_ThrowsIfJsonArrayHasMoreThan3Arguments()
+    {
+        // Arrange
+        var jsRuntime = new TestJSRuntime();
+        var task = jsRuntime.InvokeAsync<TestDTO>("somemethod");
+
+        // Act & Assert
+        Assert.ThrowsAny<JsonException>(() => DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, false, \"Hello\", 5]"));
+    }
+
+    [Fact]
+    public void EndInvokeJS_DoesNotThrowJSONExceptionIfTaskCancelled_WithMoreThan3Arguments()
+    {
+        // Arrange
+        var jsRuntime = new TestJSRuntime();
+        var cts = new CancellationTokenSource();
+
+        // Act
+        var task = jsRuntime.InvokeAsync<TestDTO>("unimportant", cts.Token);
+
+        cts.Cancel();
+
+        DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, false, \"Hello\", 5]");
+
+        // Assert
+        Assert.False(task.IsCompletedSuccessfully);
+        Assert.True(task.IsCanceled);
+    }
+
+    [Fact]
+    public void EndInvokeJS_Works()
+    {
+        // Arrange
+        var jsRuntime = new TestJSRuntime();
+        var task = jsRuntime.InvokeAsync<TestDTO>("somemethod");
+
+        // Act
+        DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, true, {{\"intVal\": 7}}]");
+
+        // Assert
+        Assert.True(task.IsCompletedSuccessfully);
+        Assert.Equal(7, task.Result.IntVal);
+    }
+
+    [Fact]
+    public void EndInvokeJS_WithArrayValue()
+    {
+        var jsRuntime = new TestJSRuntime();
+        var task = jsRuntime.InvokeAsync<int[]>("somemethod");
+
+        DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, true, [1, 2, 3]]");
+
+        Assert.True(task.IsCompletedSuccessfully);
+        Assert.Equal(new[] { 1, 2, 3 }, task.Result);
+    }
+
+    [Fact]
+    public void EndInvokeJS_WithNullValue()
+    {
+        var jsRuntime = new TestJSRuntime();
+        var task = jsRuntime.InvokeAsync<TestDTO>("somemethod");
+
+        DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, true, null]");
+
+        Assert.True(task.IsCompletedSuccessfully);
+        Assert.Null(task.Result);
+    }
+
     [Fact]
     public void CanInvokeInstanceMethodWithParams()
     {
@@ -652,66 +775,6 @@ public class DotNetDispatcherTest
         Assert.Equal($"In call to '{method}', parameter of type '{nameof(TestDTO)}' at index 2 must be declared as type 'DotNetObjectRef<TestDTO>' to receive the incoming value.", ex.Message);
     }
 
-    [Fact]
-    public void EndInvokeJS_ThrowsIfJsonIsEmptyString()
-    {
-        Assert.ThrowsAny<JsonException>(() => DotNetDispatcher.EndInvokeJS(new TestJSRuntime(), ""));
-    }
-
-    [Fact]
-    public void EndInvokeJS_ThrowsIfJsonIsNotArray()
-    {
-        Assert.ThrowsAny<JsonException>(() => DotNetDispatcher.EndInvokeJS(new TestJSRuntime(), "{\"key\": \"value\"}"));
-    }
-
-    [Fact]
-    public void EndInvokeJS_ThrowsIfJsonArrayIsInComplete()
-    {
-        Assert.ThrowsAny<JsonException>(() => DotNetDispatcher.EndInvokeJS(new TestJSRuntime(), "[7, false"));
-    }
-
-    [Fact]
-    public void EndInvokeJS_ThrowsIfJsonArrayHasMoreThan3Arguments()
-    {
-        Assert.ThrowsAny<JsonException>(() => DotNetDispatcher.EndInvokeJS(new TestJSRuntime(), "[7, false, \"Hello\", 5]"));
-    }
-
-    [Fact]
-    public void EndInvokeJS_Works()
-    {
-        var jsRuntime = new TestJSRuntime();
-        var task = jsRuntime.InvokeAsync<TestDTO>("somemethod");
-
-        DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, true, {{\"intVal\": 7}}]");
-
-        Assert.True(task.IsCompletedSuccessfully);
-        Assert.Equal(7, task.Result.IntVal);
-    }
-
-    [Fact]
-    public void EndInvokeJS_WithArrayValue()
-    {
-        var jsRuntime = new TestJSRuntime();
-        var task = jsRuntime.InvokeAsync<int[]>("somemethod");
-
-        DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, true, [1, 2, 3]]");
-
-        Assert.True(task.IsCompletedSuccessfully);
-        Assert.Equal(new[] { 1, 2, 3 }, task.Result);
-    }
-
-    [Fact]
-    public void EndInvokeJS_WithNullValue()
-    {
-        var jsRuntime = new TestJSRuntime();
-        var task = jsRuntime.InvokeAsync<TestDTO>("somemethod");
-
-        DotNetDispatcher.EndInvokeJS(jsRuntime, $"[{jsRuntime.LastInvocationAsyncHandle}, true, null]");
-
-        Assert.True(task.IsCompletedSuccessfully);
-        Assert.Null(task.Result);
-    }
-
     [Fact]
     public void ReceiveByteArray_Works()
     {