Javiercn/js interop improvements part 1 (#11809)

* Adds exception sanitization to RemoteJSRuntime
* Adds default async timeouts of 1 minute for JS async calls to server-side blazor

src/Components/Server/ref/Microsoft.AspNetCore.Components.Server.netcoreapp3.0.cs

@@ -27,6 +27,8 @@ namespace Microsoft.AspNetCore.Components.Server
     {
         public CircuitOptions() { }
         public System.TimeSpan DisconnectedCircuitRetentionPeriod { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
+        public System.TimeSpan JSInteropDefaultCallTimeout { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
+        public bool JSInteropDetailedErrors { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public int MaxRetainedDisconnectedCircuits { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
     }
     public sealed partial class ComponentEndpointConventionBuilder : Microsoft.AspNetCore.Builder.IEndpointConventionBuilder, Microsoft.AspNetCore.SignalR.IHubEndpointConventionBuilder

src/Components/Server/src/CircuitOptions.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using Microsoft.AspNetCore.DataProtection;
 
 namespace Microsoft.AspNetCore.Components.Server
 {
@@ -46,5 +45,24 @@ namespace Microsoft.AspNetCore.Components.Server
         /// Defaults to <c>3 minutes</c>.
         /// </value>
         public TimeSpan DisconnectedCircuitRetentionPeriod { get; set; } = TimeSpan.FromMinutes(3);
+
+        /// <summary>
+        /// Gets or sets a value that determines whether or not to send detailed exception messages from .NET interop method invocation
+        /// exceptions to JavaScript.
+        /// </summary>
+        /// <remarks>
+        /// This value should only be turned on in development scenarios as turning it on in production might result in the leak of
+        /// sensitive information to untrusted parties.
+        /// </remarks>
+        /// <value>Defaults to <c>false</c>.</value>
+        public bool JSInteropDetailedErrors { get; set; }
+
+        /// <summary>
+        /// Gets or sets a value that indicates how long the server will wait before timing out an asynchronous JavaScript function invocation.
+        /// </summary>
+        /// <value>
+        /// Defaults to <c>1 minute</c>.
+        /// </value>
+        public TimeSpan JSInteropDefaultCallTimeout { get; set; } = TimeSpan.FromMinutes(1);
     }
 }

src/Components/Server/src/Circuits/CircuitOptionsJSInteropDetailedErrorsConfiguration.cs

@@ -0,0 +1,24 @@
+// 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 Microsoft.AspNetCore.Hosting;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.Options;
+
+namespace Microsoft.AspNetCore.Components.Server
+{
+    internal class CircuitOptionsJSInteropDetailedErrorsConfiguration : IConfigureOptions<CircuitOptions>
+    {
+        public CircuitOptionsJSInteropDetailedErrorsConfiguration(IConfiguration configuration)
+        {
+            Configuration = configuration;
+        }
+
+        public IConfiguration Configuration { get; }
+
+        public void Configure(CircuitOptions options)
+        {
+            options.JSInteropDetailedErrors = Configuration.GetValue<bool>(WebHostDefaults.DetailedErrorsKey);
+        }
+    }
+}

src/Components/Server/src/Circuits/RemoteJSRuntime.cs

@@ -3,19 +3,40 @@
 
 using System;
 using Microsoft.AspNetCore.SignalR;
+using Microsoft.Extensions.Options;
 using Microsoft.JSInterop;
 
 namespace Microsoft.AspNetCore.Components.Server.Circuits
 {
     internal class RemoteJSRuntime : JSRuntimeBase
     {
+        private readonly CircuitOptions _options;
         private CircuitClientProxy _clientProxy;
 
+        public RemoteJSRuntime(IOptions<CircuitOptions> options)
+        {
+            _options = options.Value;
+            DefaultAsyncTimeout = _options.JSInteropDefaultCallTimeout;
+        }
+
         internal void Initialize(CircuitClientProxy clientProxy)
         {
             _clientProxy = clientProxy ?? throw new ArgumentNullException(nameof(clientProxy));
         }
 
+        protected override object OnDotNetInvocationException(Exception exception, string assemblyName, string methodIdentifier)
+        {
+            if (_options.JSInteropDetailedErrors)
+            {
+                return base.OnDotNetInvocationException(exception, assemblyName, methodIdentifier);
+            }
+
+            var message = $"There was an exception invoking '{methodIdentifier}' on assembly '{assemblyName}'. For more details turn on " +
+                $"detailed exceptions in '{typeof(CircuitOptions).Name}.{nameof(CircuitOptions.JSInteropDetailedErrors)}'";
+
+            return message;
+        }
+
         protected override void BeginInvokeJS(long asyncHandle, string identifier, string argsJson)
         {
             if (!_clientProxy.Connected)

src/Components/Server/src/DependencyInjection/ComponentServiceCollectionExtensions.cs

@@ -78,9 +78,11 @@ namespace Microsoft.Extensions.DependencyInjection
             services.AddScoped<IComponentContext, RemoteComponentContext>();
             services.AddScoped<AuthenticationStateProvider, FixedAuthenticationStateProvider>();
 
+            services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<CircuitOptions>, CircuitOptionsJSInteropDetailedErrorsConfiguration>());
+
             if (configure != null)
             {
-                services.Configure<CircuitOptions>(configure);
+                services.Configure(configure);
             }
 
             return builder;

src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFramework>netcoreapp3.0</TargetFramework>

src/Components/Server/test/Circuits/TestCircuitHost.cs

@@ -13,6 +13,7 @@ using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using Moq;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Components.Server.Circuits
 {
@@ -38,7 +39,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits
             serviceScope = serviceScope ?? Mock.Of<IServiceScope>();
             clientProxy = clientProxy ?? new CircuitClientProxy(Mock.Of<IClientProxy>(), Guid.NewGuid().ToString());
             var renderRegistry = new RendererRegistry();
-            var jsRuntime = new RemoteJSRuntime();
+            var jsRuntime = new RemoteJSRuntime(Options.Create(new CircuitOptions()));
             var dispatcher = Rendering.Renderer.CreateDefaultDispatcher();
 
             if (remoteRenderer == null)

src/Components/test/E2ETest/Infrastructure/ServerFixtures/AspNetSiteServerFixture.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.IO;
+using System.Linq;
 using System.Reflection;
 using Microsoft.AspNetCore.Hosting;
 
@@ -18,6 +20,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures
 
         public AspNetEnvironment Environment { get; set; } = AspNetEnvironment.Production;
 
+        public List<string> AdditionalArguments { get; set; } = new List<string>();
+
         protected override IWebHost CreateWebHost()
         {
             if (BuildWebHostMethod == null)
@@ -34,7 +38,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures
                 "--urls", "http://127.0.0.1:0",
                 "--contentroot", sampleSitePath,
                 "--environment", Environment.ToString(),
-            });
+            }.Concat(AdditionalArguments).ToArray());
         }
     }
 }

src/Components/test/E2ETest/Infrastructure/ServerFixtures/ToggleExecutionModeServerFixture.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 
 namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures
 {
@@ -15,6 +16,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures
         private AspNetSiteServerFixture.BuildWebHost _buildWebHostMethod;
         private IDisposable _serverToDispose;
 
+        public List<string> AspNetFixtureAdditionalArguments { get; set; } = new List<string>();
+
         public void UseAspNetHost(AspNetSiteServerFixture.BuildWebHost buildWebHostMethod)
         {
             _buildWebHostMethod = buildWebHostMethod
@@ -35,6 +38,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures
             {
                 // Use specified ASP.NET host server
                 var underlying = new AspNetSiteServerFixture();
+                underlying.AdditionalArguments.AddRange(AspNetFixtureAdditionalArguments);
                 underlying.BuildWebHostMethod = _buildWebHostMethod;
                 _serverToDispose = underlying;
                 return underlying.RootUri.AbsoluteUri;
@@ -45,6 +49,12 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures
         {
             _serverToDispose?.Dispose();
         }
+
+        internal ToggleExecutionModeServerFixture<TClientProgram> WithAdditionalArguments(string [] additionalArguments)
+        {
+            AspNetFixtureAdditionalArguments.AddRange(additionalArguments);
+            return this;
+        }
     }
 
     public enum ExecutionMode { Client, Server }

src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj

@@ -7,7 +7,7 @@
     <TargetFramework>netcoreapp3.0</TargetFramework>
     <TestGroupName>Components.E2ETests</TestGroupName>
 
-    <SkipTests>true</SkipTests>
+    <SkipTests Condition="$(ContinuousIntegrationBuild) == 'true'" >true</SkipTests>
     <!-- https://github.com/aspnet/AspNetCore/issues/6857 -->
     <BuildHelixPayload>false</BuildHelixPayload>
 

src/Components/test/E2ETest/ServerExecutionTests/ServerInteropTestDefaultExceptionsBehavior.cs

@@ -0,0 +1,70 @@
+// 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 BasicTestApp;
+using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
+using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
+using Microsoft.AspNetCore.Components.Server;
+using Microsoft.AspNetCore.E2ETesting;
+using OpenQA.Selenium;
+using OpenQA.Selenium.Support.UI;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests
+{
+    public class ServerInteropTestDefaultExceptionsBehavior : BasicTestAppTestBase
+    {
+        public ServerInteropTestDefaultExceptionsBehavior(
+            BrowserFixture browserFixture,
+            ToggleExecutionModeServerFixture<Program> serverFixture,
+            ITestOutputHelper output)
+            : base(browserFixture, serverFixture.WithServerExecution(), output)
+        {
+        }
+
+        protected override void InitializeAsyncCore()
+        {
+            Navigate(ServerPathBase, noReload: true);
+            MountTestComponent<InteropComponent>();
+        }
+
+        [Fact]
+        public void DotNetExceptionDetailsAreNotLoggedByDefault()
+        {
+            // Arrange
+            var expectedValues = new Dictionary<string, string>
+            {
+                ["AsyncThrowSyncException"] = GetExpectedMessage("AsyncThrowSyncException"),
+                ["AsyncThrowAsyncException"] = GetExpectedMessage("AsyncThrowAsyncException"),
+            };
+
+            var actualValues = new Dictionary<string, string>();
+
+            // Act
+            var interopButton = Browser.FindElement(By.Id("btn-interop"));
+            interopButton.Click();
+
+            var wait = new WebDriverWait(Browser, TimeSpan.FromSeconds(10))
+                .Until(d => d.FindElement(By.Id("done-with-interop")));
+
+            foreach (var expectedValue in expectedValues)
+            {
+                var currentValue = Browser.FindElement(By.Id(expectedValue.Key));
+                actualValues.Add(expectedValue.Key, currentValue.Text);
+            }
+
+            // Assert
+            foreach (var expectedValue in expectedValues)
+            {
+                Assert.Equal(expectedValue.Value, actualValues[expectedValue.Key]);
+            }
+
+            string GetExpectedMessage(string method) =>
+                $"\"There was an exception invoking '{method}' on assembly 'BasicTestApp'. For more details turn on " +
+                $"detailed exceptions in '{typeof(CircuitOptions).Name}.{nameof(CircuitOptions.JSInteropDetailedErrors)}'\"";
+        }
+    }
+}

src/Components/test/E2ETest/ServerExecutionTests/ServerInteropTestJsInvocationsTimeoutsBehavior.cs

@@ -0,0 +1,51 @@
+// 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.Threading.Tasks;
+using BasicTestApp;
+using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
+using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
+using Microsoft.AspNetCore.E2ETesting;
+using OpenQA.Selenium;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests
+{
+    public class ServerInteropTestJsInvocationsTimeoutsBehavior : BasicTestAppTestBase
+    {
+        public ServerInteropTestJsInvocationsTimeoutsBehavior(
+            BrowserFixture browserFixture,
+            ToggleExecutionModeServerFixture<Program> serverFixture,
+            ITestOutputHelper output)
+            : base(browserFixture, serverFixture.WithServerExecution(), output)
+        {
+        }
+
+        protected override void InitializeAsyncCore()
+        {
+            Navigate(ServerPathBase, noReload: true);
+            MountTestComponent<LongRunningInterop>();
+        }
+
+        [Fact]
+        public async Task LongRunningJavaScriptFunctionsResultInCancellationAndWorkingAppAfterFunctionCompletion()
+        {
+            // Act & Assert
+            var interopButton = Browser.FindElement(By.Id("btn-interop"));
+            interopButton.Click();
+
+            Browser.Exists(By.Id("done-with-interop"));
+
+            Browser.Exists(By.Id("task-was-cancelled"));
+
+            // wait 10 seconds, js method completes in 5 seconds, after this point it would have triggered a completion for sure.
+            await Task.Delay(10000);
+
+            var circuitFunctional = Browser.FindElement(By.Id("circuit-functional"));
+            circuitFunctional.Click();
+
+            Browser.Exists(By.Id("done-circuit-functional"));
+        }
+    }
+}

src/Components/test/E2ETest/ServerExecutionTests/TestSubclasses.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using BasicTestApp;
-using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
 using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
 using Microsoft.AspNetCore.Components.E2ETest.Tests;
 using Microsoft.AspNetCore.E2ETesting;
@@ -37,9 +36,12 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests
     public class ServerInteropTest : InteropTest
     {
         public ServerInteropTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture<Program> serverFixture, ITestOutputHelper output)
-            : base(browserFixture, serverFixture.WithServerExecution(), output)
+            : base(browserFixture, serverFixture.WithServerExecution().WithAdditionalArguments(GetAdditionalArguments()), output)
         {
         }
+
+        private static string[] GetAdditionalArguments() =>
+            new string[] { "--circuit-detailed-errors", "true" };
     }
 
     public class ServerRoutingTest : RoutingTest

src/Components/test/testassets/BasicTestApp/Index.razor

@@ -4,6 +4,7 @@
     <select @bind=SelectedComponentTypeName>
         <option value="none">Choose...</option>
         <option value="BasicTestApp.InteropComponent">Interop component</option>
+        <option value="BasicTestApp.LongRunningInterop">Long running interop</option>
         <option value="BasicTestApp.AsyncEventHandlerComponent">Async event handlers</option>
         <option value="BasicTestApp.AddRemoveChildComponents">Add/remove child components</option>
         <option value="BasicTestApp.CounterComponent">Counter</option>

src/Components/test/testassets/BasicTestApp/LongRunningInterop.razor

@@ -0,0 +1,53 @@
+@using Microsoft.JSInterop
+@using BasicTestApp.InteropTest
+@using System.Runtime.InteropServices
+@using System.Text.Json
+@inject IJSRuntime JSRuntime
+
+<button id="btn-interop" @onclick="@InvokeInteropAsync">Invoke interop!</button>
+
+@if (DoneWithInterop)
+{
+    <button id="circuit-functional" @onclick="@DoWork">Circuit still functional</button>
+    if (TaskWasCancelled)
+    {
+        <p id="task-was-cancelled">Task was cancelled.</p>
+    }
+    <p id="done-with-interop">Done with interop.</p>
+}
+
+@if (WorkDone)
+{
+    <p id="done-circuit-functional">Circuit is functional after client notification completion.</p>
+}
+
+@code {
+    public bool TaskWasCancelled { get; set; }
+
+    public bool WorkDone { get; set; }
+
+    public bool DoneWithInterop { get; set; }
+
+    public async Task InvokeInteropAsync()
+    {
+        try
+        {
+            using var cancellationTokenSource = new System.Threading.CancellationTokenSource(TimeSpan.FromSeconds(1));
+            await JSRuntime.InvokeAsync<object>(
+                "asyncFunctionTakesLongerThanDefaultTimeoutToResolve",
+                Array.Empty<object>(),
+                cancellationTokenSource.Token);
+        }
+        catch (TaskCanceledException)
+        {
+            TaskWasCancelled = true;
+        }
+
+        DoneWithInterop = true;
+    }
+
+    public void DoWork()
+    {
+        WorkDone = true;
+    }
+}

src/Components/test/testassets/BasicTestApp/wwwroot/js/jsinteroptests.js

@@ -210,6 +210,12 @@ function asyncFunctionThrowsAsyncException() {
   });
 }
 
+function asyncFunctionTakesLongerThanDefaultTimeoutToResolve() {
+    return new Promise((resolve, reject) => {
+        setTimeout(() => resolve(undefined), 5000);
+    });
+}
+
 function collectInteropResults() {
   let result = {};
   let properties = Object.getOwnPropertyNames(results);

src/Components/test/testassets/Ignitor/Ignitor.csproj

@@ -10,7 +10,6 @@
     <Reference Include="Microsoft.AspNetCore.SignalR.Protocols.MessagePack" />
     <Reference Include="Microsoft.Extensions.Logging.Console" />
     <Reference Include="Microsoft.AspNetCore.Components.Server" />
-    <Reference Include="System.Text.Json" />
   </ItemGroup>
 
 </Project>

src/Components/test/testassets/TestServer/Startup.cs

@@ -25,7 +25,12 @@ namespace TestServer
             {
                 options.AddPolicy("AllowAll", _ => { /* Controlled below */ });
             });
-            services.AddServerSideBlazor();
+            services.AddServerSideBlazor()
+                .AddCircuitOptions(o =>
+                {
+                    var detailedErrors = Configuration.GetValue<bool>("circuit-detailed-errors");
+                    o.JSInteropDetailedErrors = detailedErrors;
+                });
             services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme).AddCookie();
 
             services.AddAuthorization(options =>

src/Mvc/Mvc.Components.Prerendering/test/HtmlHelperComponentPrerenderingExtensionsTests.cs

@@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Html;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Mvc.Rendering;
+using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.JSInterop;
 using Microsoft.Net.Http.Headers;
@@ -484,6 +485,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
         private static IHtmlHelper CreateHelper(HttpContext ctx = null, Action<IServiceCollection> configureServices = null)
         {
             var services = new ServiceCollection();
+            services.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build());
             services.AddLogging();
             services.AddDataProtection();
             services.AddSingleton(HtmlEncoder.Default);