From 244ed547644047a0696eba8955f6a407fb30a338 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Tue, 2 Jul 2019 12:40:50 +0200 Subject: [PATCH 1/3] Allow sanitization of dotnet interop exceptions (dotnet/extensions#1879) Adds the ability to sanitize exceptions during JS to .NET interop calls by overriding OnDotNetInvocationException method and returning an object to be returned to JavaScript that describes the exception.\n\nCommit migrated from https://github.com/dotnet/extensions/commit/d7eab7c0830382d435aef2cc22faf90c39b8fb54 --- .../ref/Microsoft.JSInterop.netstandard2.0.cs | 1 + .../src/DotNetDispatcher.cs | 9 ++-- .../Microsoft.JSInterop/src/JSRuntimeBase.cs | 21 +++++++-- .../test/JSRuntimeBaseTest.cs | 44 +++++++++++++++++++ 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs b/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs index c2884dc64a..cd383df0df 100644 --- a/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs +++ b/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs @@ -61,6 +61,7 @@ namespace Microsoft.JSInterop protected JSRuntimeBase() { } protected abstract void BeginInvokeJS(long asyncHandle, string identifier, string argsJson); public System.Threading.Tasks.Task InvokeAsync(string identifier, params object[] args) { throw null; } + protected virtual object OnDotNetInvocationException(System.Exception exception, string assemblyName, string methodIdentifier) { throw null; } } } namespace Microsoft.JSInterop.Internal diff --git a/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs b/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs index 360efae5d1..4359cd76c7 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs @@ -103,7 +103,7 @@ namespace Microsoft.JSInterop else if (syncException != null) { // Threw synchronously, let's respond. - jsRuntimeBaseInstance.EndInvokeDotNet(callId, false, syncException); + jsRuntimeBaseInstance.EndInvokeDotNet(callId, false, syncException, assemblyName, methodIdentifier); } else if (syncResult is Task task) { @@ -114,16 +114,17 @@ namespace Microsoft.JSInterop if (t.Exception != null) { var exception = t.Exception.GetBaseException(); - jsRuntimeBaseInstance.EndInvokeDotNet(callId, false, ExceptionDispatchInfo.Capture(exception)); + + jsRuntimeBaseInstance.EndInvokeDotNet(callId, false, ExceptionDispatchInfo.Capture(exception), assemblyName, methodIdentifier); } var result = TaskGenericsUtil.GetTaskResult(task); - jsRuntimeBaseInstance.EndInvokeDotNet(callId, true, result); + jsRuntimeBaseInstance.EndInvokeDotNet(callId, true, result, assemblyName, methodIdentifier); }, TaskScheduler.Current); } else { - jsRuntimeBaseInstance.EndInvokeDotNet(callId, true, syncResult); + jsRuntimeBaseInstance.EndInvokeDotNet(callId, true, syncResult, assemblyName, methodIdentifier); } } diff --git a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs index 50fd7e2839..694b0cb625 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs @@ -62,18 +62,31 @@ namespace Microsoft.JSInterop /// A JSON representation of the arguments. protected abstract void BeginInvokeJS(long asyncHandle, string identifier, string argsJson); - internal void EndInvokeDotNet(string callId, bool success, object resultOrException) + /// + /// Allows derived classes to configure the information about an exception in a JS interop call that gets sent to JavaScript. + /// + /// + /// This callback can be used in remote JS interop scenarios to sanitize exceptions that happen on the server to avoid disclosing + /// sensitive information to remote browser clients. + /// + /// The exception that occurred. + /// The assembly for the invoked .NET method. + /// The identifier for the invoked .NET method. + /// An object containing information about the exception. + protected virtual object OnDotNetInvocationException(Exception exception, string assemblyName, string methodIdentifier) => exception.ToString(); + + internal void EndInvokeDotNet(string callId, bool success, object resultOrException, string assemblyName, string methodIdentifier) { // For failures, the common case is to call EndInvokeDotNet with the Exception object. // For these we'll serialize as something that's useful to receive on the JS side. // If the value is not an Exception, we'll just rely on it being directly JSON-serializable. - if (!success && resultOrException is Exception) + if (!success && resultOrException is Exception ex) { - resultOrException = resultOrException.ToString(); + resultOrException = OnDotNetInvocationException(ex, assemblyName, methodIdentifier); } else if (!success && resultOrException is ExceptionDispatchInfo edi) { - resultOrException = edi.SourceException.ToString(); + resultOrException = OnDotNetInvocationException(edi.SourceException, assemblyName, methodIdentifier); } // We pass 0 as the async handle because we don't want the JS-side code to diff --git a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs index afbe5d0595..b01ef59998 100644 --- a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs +++ b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs @@ -168,6 +168,38 @@ namespace Microsoft.JSInterop.Tests Assert.Same(obj3, runtime.ObjectRefManager.FindDotNetObject(4)); } + [Fact] + public void CanSanitizeDotNetInteropExceptions() + { + // Arrange + var expectedMessage = "An error ocurred while invoking '[Assembly]::Method'. Swapping to 'Development' environment will " + + "display more detailed information about the error that occurred."; + + string GetMessage(string assembly, string method) => $"An error ocurred while invoking '[{assembly}]::{method}'. Swapping to 'Development' environment will " + + "display more detailed information about the error that occurred."; + + var runtime = new TestJSRuntime() + { + OnDotNetException = (e, a, m) => new JSError { Message = GetMessage(a, m) } + }; + + var exception = new Exception("Some really sensitive data in here"); + + // Act + runtime.EndInvokeDotNet("0", false, exception, "Assembly", "Method"); + + // Assert + var call = runtime.BeginInvokeCalls.Single(); + Assert.Equal(0, call.AsyncHandle); + Assert.Equal("DotNet.jsCallDispatcher.endInvokeDotNetFromJS", call.Identifier); + Assert.Equal($"[\"0\",false,{{\"message\":\"{expectedMessage.Replace("'", "\\u0027")}\"}}]", call.ArgsJson); + } + + private class JSError + { + public string Message { get; set; } + } + class TestJSRuntime : JSRuntimeBase { public List BeginInvokeCalls = new List(); @@ -179,6 +211,18 @@ namespace Microsoft.JSInterop.Tests public string ArgsJson { get; set; } } + public Func OnDotNetException { get; set; } + + protected override object OnDotNetInvocationException(Exception exception, string assemblyName, string methodName) + { + if (OnDotNetException != null) + { + return OnDotNetException(exception, assemblyName, methodName); + } + + return base.OnDotNetInvocationException(exception, assemblyName, methodName); + } + protected override void BeginInvokeJS(long asyncHandle, string identifier, string argsJson) { BeginInvokeCalls.Add(new BeginInvokeAsyncArgs From 5e4f1f5fb2e70651aef0dee4068eac7df9504c31 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Tue, 2 Jul 2019 13:24:29 +0200 Subject: [PATCH 2/3] Adds an opt-in default timeout for async JS calls (dotnet/extensions#1880) * Adds the ability to configure a default timeout for JavaScript interop calls. * Adds the ability to pass in a cancellation token to InvokeAsync that will be used instead of the default timeout. * A default cancellation token can be passed to signal no cancellation, but it is not recommended in remote interop scenarios.\n\nCommit migrated from https://github.com/dotnet/extensions/commit/e04faac7e4a71ae03e31bcbe923f8743eefad9f7 --- .../ref/Microsoft.JSInterop.netstandard2.0.cs | 4 +- .../Microsoft.JSInterop/src/JSRuntimeBase.cs | 109 ++++++++++++++---- .../test/JSRuntimeBaseTest.cs | 88 ++++++++++++-- 3 files changed, 169 insertions(+), 32 deletions(-) diff --git a/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs b/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs index cd383df0df..b0f187bcd1 100644 --- a/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs +++ b/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs @@ -59,7 +59,9 @@ namespace Microsoft.JSInterop public abstract partial class JSRuntimeBase : Microsoft.JSInterop.IJSRuntime { protected JSRuntimeBase() { } - protected abstract void BeginInvokeJS(long asyncHandle, string identifier, string argsJson); + protected System.TimeSpan? DefaultAsyncTimeout { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + protected abstract void BeginInvokeJS(long taskId, string identifier, string argsJson); + public System.Threading.Tasks.Task InvokeAsync(string identifier, System.Collections.Generic.IEnumerable args, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public System.Threading.Tasks.Task InvokeAsync(string identifier, params object[] args) { throw null; } protected virtual object OnDotNetInvocationException(System.Exception exception, string assemblyName, string methodIdentifier) { throw null; } } diff --git a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs index 694b0cb625..562a6a27ba 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs @@ -3,6 +3,8 @@ using System; using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; using System.Runtime.ExceptionServices; using System.Text.Json; using System.Threading; @@ -20,8 +22,71 @@ namespace Microsoft.JSInterop private readonly ConcurrentDictionary _pendingTasks = new ConcurrentDictionary(); + private readonly ConcurrentDictionary _cancellationRegistrations = + new ConcurrentDictionary(); + internal DotNetObjectRefManager ObjectRefManager { get; } = new DotNetObjectRefManager(); + /// + /// Gets or sets the default timeout for asynchronous JavaScript calls. + /// + protected TimeSpan? DefaultAsyncTimeout { get; set; } + + /// + /// Invokes the specified JavaScript function asynchronously. + /// + /// The JSON-serializable return type. + /// An identifier for the function to invoke. For example, the value "someScope.someFunction" will invoke the function window.someScope.someFunction. + /// JSON-serializable arguments. + /// A cancellation token to signal the cancellation of the operation. + /// An instance of obtained by JSON-deserializing the return value. + public Task InvokeAsync(string identifier, IEnumerable args, CancellationToken cancellationToken = default) + { + var taskId = Interlocked.Increment(ref _nextPendingTaskId); + var tcs = new TaskCompletionSource(TaskContinuationOptions.RunContinuationsAsynchronously); + if (cancellationToken != default) + { + _cancellationRegistrations[taskId] = cancellationToken.Register(() => + { + tcs.TrySetCanceled(cancellationToken); + CleanupTasksAndRegistrations(taskId); + }); + } + _pendingTasks[taskId] = tcs; + + try + { + if (cancellationToken.IsCancellationRequested) + { + tcs.TrySetCanceled(cancellationToken); + CleanupTasksAndRegistrations(taskId); + + return tcs.Task; + } + + var argsJson = args?.Any() == true ? + JsonSerializer.Serialize(args, JsonSerializerOptionsProvider.Options) : + null; + BeginInvokeJS(taskId, identifier, argsJson); + + return tcs.Task; + } + catch + { + CleanupTasksAndRegistrations(taskId); + throw; + } + } + + private void CleanupTasksAndRegistrations(long taskId) + { + _pendingTasks.TryRemove(taskId, out _); + if (_cancellationRegistrations.TryRemove(taskId, out var registration)) + { + registration.Dispose(); + } + } + /// /// Invokes the specified JavaScript function asynchronously. /// @@ -31,36 +96,32 @@ namespace Microsoft.JSInterop /// An instance of obtained by JSON-deserializing the return value. public Task InvokeAsync(string identifier, params object[] args) { - // We might consider also adding a default timeout here in case we don't want to - // risk a memory leak in the scenario where the JS-side code is failing to complete - // the operation. - - var taskId = Interlocked.Increment(ref _nextPendingTaskId); - var tcs = new TaskCompletionSource(); - _pendingTasks[taskId] = tcs; - - try + if (!DefaultAsyncTimeout.HasValue) { - var argsJson = args?.Length > 0 ? - JsonSerializer.Serialize(args, JsonSerializerOptionsProvider.Options) : - null; - BeginInvokeJS(taskId, identifier, argsJson); - return tcs.Task; + return InvokeAsync(identifier, args, default); } - catch + else { - _pendingTasks.TryRemove(taskId, out _); - throw; + return InvokeWithDefaultCancellation(identifier, args); + } + } + + private async Task InvokeWithDefaultCancellation(string identifier, IEnumerable args) + { + using (var cts = new CancellationTokenSource(DefaultAsyncTimeout.Value)) + { + // We need to await here due to the using + return await InvokeAsync(identifier, args, cts.Token); } } /// /// Begins an asynchronous function invocation. /// - /// The identifier for the function invocation, or zero if no async callback is required. + /// The identifier for the function invocation, or zero if no async callback is required. /// The identifier for the function to invoke. /// A JSON representation of the arguments. - protected abstract void BeginInvokeJS(long asyncHandle, string identifier, string argsJson); + protected abstract void BeginInvokeJS(long taskId, string identifier, string argsJson); /// /// Allows derived classes to configure the information about an exception in a JS interop call that gets sent to JavaScript. @@ -95,15 +156,19 @@ namespace Microsoft.JSInterop BeginInvokeJS(0, "DotNet.jsCallDispatcher.endInvokeDotNetFromJS", args); } - internal void EndInvokeJS(long asyncHandle, bool succeeded, JSAsyncCallResult asyncCallResult) + internal void EndInvokeJS(long taskId, bool succeeded, JSAsyncCallResult asyncCallResult) { using (asyncCallResult?.JsonDocument) { - if (!_pendingTasks.TryRemove(asyncHandle, out var tcs)) + if (!_pendingTasks.TryRemove(taskId, out var tcs)) { - throw new ArgumentException($"There is no pending task with handle '{asyncHandle}'."); + // 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; } + CleanupTasksAndRegistrations(taskId); + if (succeeded) { var resultType = TaskGenericsUtil.GetTaskCompletionSourceResultType(tcs); diff --git a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs index b01ef59998..3ad78c303f 100644 --- a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs +++ b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Text.Json; +using System.Threading; using System.Threading.Tasks; using Microsoft.JSInterop.Internal; using Xunit; @@ -38,6 +39,69 @@ namespace Microsoft.JSInterop.Tests }); } + [Fact] + public async Task InvokeAsync_CancelsAsyncTask_AfterDefaultTimeout() + { + // Arrange + var runtime = new TestJSRuntime(); + runtime.DefaultTimeout = TimeSpan.FromSeconds(1); + + // Act + var task = runtime.InvokeAsync("test identifier 1", "arg1", 123, true); + + // Assert + await Assert.ThrowsAsync(async () => await task); + } + + [Fact] + public async Task InvokeAsync_CompletesSuccessfullyBeforeTimeout() + { + // Arrange + var runtime = new TestJSRuntime(); + runtime.DefaultTimeout = TimeSpan.FromSeconds(10); + + // Act + var task = runtime.InvokeAsync("test identifier 1", "arg1", 123, true); + runtime.EndInvokeJS(2, succeeded: true, null); + + // Assert + await task; + } + + [Fact] + public async Task InvokeAsync_CancelsAsyncTasksWhenCancellationTokenFires() + { + // Arrange + using var cts = new CancellationTokenSource(); + var runtime = new TestJSRuntime(); + + // Act + var task = runtime.InvokeAsync("test identifier 1", new object[] { "arg1", 123, true }, cts.Token); + + cts.Cancel(); + + // Assert + await Assert.ThrowsAsync(async () => await task); + } + + [Fact] + public async Task InvokeAsync_DoesNotStartWorkWhenCancellationHasBeenRequested() + { + // Arrange + using var cts = new CancellationTokenSource(); + cts.Cancel(); + var runtime = new TestJSRuntime(); + + // Act + var task = runtime.InvokeAsync("test identifier 1", new object[] { "arg1", 123, true }, cts.Token); + + cts.Cancel(); + + // Assert + await Assert.ThrowsAsync(async () => await task); + Assert.Empty(runtime.BeginInvokeCalls); + } + [Fact] public void CanCompleteAsyncCallsAsSuccess() { @@ -115,21 +179,19 @@ namespace Microsoft.JSInterop.Tests } [Fact] - public void CannotCompleteSameAsyncCallMoreThanOnce() + public async Task CompletingSameAsyncCallMoreThanOnce_IgnoresSecondResultAsync() { // Arrange var runtime = new TestJSRuntime(); // Act/Assert - runtime.InvokeAsync("test identifier", Array.Empty()); + var task = runtime.InvokeAsync("test identifier", Array.Empty()); var asyncHandle = runtime.BeginInvokeCalls[0].AsyncHandle; - runtime.OnEndInvoke(asyncHandle, true, null); - var ex = Assert.Throws(() => - { - // Second "end invoke" will fail - runtime.OnEndInvoke(asyncHandle, true, null); - }); - Assert.Equal($"There is no pending task with handle '{asyncHandle}'.", ex.Message); + runtime.OnEndInvoke(asyncHandle, true, new JSAsyncCallResult(JsonDocument.Parse("{}"), JsonDocument.Parse("{\"Message\": \"Some data\"}").RootElement.GetProperty("Message"))); + runtime.OnEndInvoke(asyncHandle, false, new JSAsyncCallResult(null, JsonDocument.Parse("{\"Message\": \"Exception\"}").RootElement.GetProperty("Message"))); + + var result = await task; + Assert.Equal("Some data", result); } [Fact] @@ -204,6 +266,14 @@ namespace Microsoft.JSInterop.Tests { public List BeginInvokeCalls = new List(); + public TimeSpan? DefaultTimeout + { + set + { + base.DefaultAsyncTimeout = value; + } + } + public class BeginInvokeAsyncArgs { public long AsyncHandle { get; set; } From 6069a897588812da5f895e7c4a982f14732c1c48 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Tue, 2 Jul 2019 19:21:16 +0200 Subject: [PATCH 3/3] Add InvokeAsync with cancellation token overload to IJSRuntime (dotnet/extensions#1915) Adds the InvokeAsync with cancellation token overload to IJSRuntime\n\nCommit migrated from https://github.com/dotnet/extensions/commit/55f0b77464408036d99c4fbdd2465ea3e38a4e9b --- .../ref/Microsoft.JSInterop.netstandard2.0.cs | 1 + src/JSInterop/Microsoft.JSInterop/src/IJSRuntime.cs | 13 ++++++++++++- .../Microsoft.JSInterop/test/JSRuntimeTest.cs | 5 +++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs b/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs index b0f187bcd1..ad95b7342b 100644 --- a/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs +++ b/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs @@ -32,6 +32,7 @@ namespace Microsoft.JSInterop } public partial interface IJSRuntime { + System.Threading.Tasks.Task InvokeAsync(string identifier, System.Collections.Generic.IEnumerable args, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)); System.Threading.Tasks.Task InvokeAsync(string identifier, params object[] args); } public partial class JSException : System.Exception diff --git a/src/JSInterop/Microsoft.JSInterop/src/IJSRuntime.cs b/src/JSInterop/Microsoft.JSInterop/src/IJSRuntime.cs index 97713bb3c1..d7ee372a73 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/IJSRuntime.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/IJSRuntime.cs @@ -1,7 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; +using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; namespace Microsoft.JSInterop @@ -19,5 +20,15 @@ namespace Microsoft.JSInterop /// JSON-serializable arguments. /// An instance of obtained by JSON-deserializing the return value. Task InvokeAsync(string identifier, params object[] args); + + /// + /// Invokes the specified JavaScript function asynchronously. + /// + /// The JSON-serializable return type. + /// An identifier for the function to invoke. For example, the value "someScope.someFunction" will invoke the function window.someScope.someFunction. + /// JSON-serializable arguments. + /// A cancellation token to signal the cancellation of the operation. + /// An instance of obtained by JSON-deserializing the return value. + Task InvokeAsync(string identifier, IEnumerable args, CancellationToken cancellationToken = default); } } diff --git a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeTest.cs b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeTest.cs index ca8c96df60..f2fab5d741 100644 --- a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeTest.cs +++ b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeTest.cs @@ -2,7 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Xunit; @@ -29,6 +31,9 @@ namespace Microsoft.JSInterop.Tests { public Task InvokeAsync(string identifier, params object[] args) => throw new NotImplementedException(); + + public Task InvokeAsync(string identifier, IEnumerable args, CancellationToken cancellationToken = default) => + throw new NotImplementedException(); } } }