CR feedback plus a test fix

Author: SteveSandersonMS

src/Components/Endpoints/src/Builder/OpaqueRedirection.cs

@@ -1,16 +1,19 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Security.Cryptography;
 using System.Text.Encodings.Web;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.DataProtection;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Components.Endpoints;
 
-internal static class OpaqueRedirection
+internal partial class OpaqueRedirection
 {
     // During streaming SSR, a component may try to perform a redirection. Since the response has already started
     // this can only work if we communicate the redirection back via some command that can get handled by JS,
@@ -38,7 +41,9 @@ internal static class OpaqueRedirection
     public static string CreateProtectedRedirectionUrl(HttpContext httpContext, string destinationUrl)
     {
         var protector = CreateProtector(httpContext);
-        var protectedUrl = protector.Protect(destinationUrl, TimeSpan.FromSeconds(10));
+        var options = httpContext.RequestServices.GetRequiredService<IOptions<RazorComponentsServiceOptions>>();
+        var lifetime = options.Value.TemporaryRedirectionUrlValidityDuration;
+        var protectedUrl = protector.Protect(destinationUrl, lifetime);
         return $"{RedirectionEndpointBaseRelativeUrl}?url={UrlEncoder.Default.Encode(protectedUrl)}";
     }
 
@@ -53,7 +58,23 @@ public static void AddBlazorOpaqueRedirectionEndpoint(IEndpointRouteBuilder endp
             }
 
             var protector = CreateProtector(httpContext);
-            var url = protector.Unprotect(protectedUrl[0]!);
+            string url;
+
+            try
+            {
+                url = protector.Unprotect(protectedUrl[0]!);
+            }
+            catch (CryptographicException ex)
+            {
+                if (httpContext.RequestServices.GetService<ILogger<OpaqueRedirection>>() is { } logger)
+                {
+                    Log.OpaqueUrlUnprotectionFailed(logger, ex);
+                }
+
+                httpContext.Response.StatusCode = 400;
+                return Task.CompletedTask;
+            }
+
             httpContext.Response.Redirect(url);
             return Task.CompletedTask;
         });
@@ -64,4 +85,10 @@ private static ITimeLimitedDataProtector CreateProtector(HttpContext httpContext
         var dataProtectionProvider = httpContext.RequestServices.GetRequiredService<IDataProtectionProvider>();
         return dataProtectionProvider.CreateProtector(RedirectionDataProtectionProviderPurpose).ToTimeLimitedDataProtector();
     }
+
+    public static partial class Log
+    {
+        [LoggerMessage(1, LogLevel.Information, "Opaque URL unprotection failed.", EventName = "OpaqueUrlUnprotectionFailed")]
+        public static partial void OpaqueUrlUnprotectionFailed(ILogger<OpaqueRedirection> logger, Exception exception);
+    }
 }

src/Components/Endpoints/src/DependencyInjection/RazorComponentsServiceOptions.cs

@@ -10,6 +10,9 @@ namespace Microsoft.AspNetCore.Components.Endpoints;
 /// </summary>
 public sealed class RazorComponentsServiceOptions
 {
+    // Fairly long default lifetime to allow for clock skew across servers
+    private TimeSpan _temporaryRedirectionUrlValidityDuration = TimeSpan.FromMinutes(5);
+
     internal FormDataMapperOptions _formMappingOptions = new();
 
     /// <summary>
@@ -63,4 +66,20 @@ public int MaxFormMappingKeySize
         get => _formMappingOptions.MaxKeyBufferSize;
         set => _formMappingOptions.MaxKeyBufferSize = value;
     }
+
+    /// <summary>
+    /// Gets or sets the lifetime of data protection validity for temporary redirection URLs
+    /// emitted by Blazor server-side rendering. These are only used transiently so the lifetime
+    /// only needs to be long enough for a client to receive the URL and begin navigation to it.
+    /// However, it should also be long enough to allow for clock skew across servers.
+    /// </summary>
+    public TimeSpan TemporaryRedirectionUrlValidityDuration
+    {
+        get => _temporaryRedirectionUrlValidityDuration;
+        set
+        {
+            ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(value.TotalMilliseconds, 0);
+            _temporaryRedirectionUrlValidityDuration = value;
+        }
+    }
 }

src/Components/Endpoints/src/PublicAPI.Unshipped.txt

@@ -30,6 +30,8 @@ Microsoft.AspNetCore.Components.Endpoints.RazorComponentsServiceOptions.MaxFormM
 Microsoft.AspNetCore.Components.Endpoints.RazorComponentsServiceOptions.MaxFormMappingRecursionDepth.get -> int
 Microsoft.AspNetCore.Components.Endpoints.RazorComponentsServiceOptions.MaxFormMappingRecursionDepth.set -> void
 Microsoft.AspNetCore.Components.Endpoints.RazorComponentsServiceOptions.RazorComponentsServiceOptions() -> void
+Microsoft.AspNetCore.Components.Endpoints.RazorComponentsServiceOptions.TemporaryRedirectionUrlValidityDuration.get -> System.TimeSpan
+Microsoft.AspNetCore.Components.Endpoints.RazorComponentsServiceOptions.TemporaryRedirectionUrlValidityDuration.set -> void
 Microsoft.AspNetCore.Components.Endpoints.RootComponentMetadata
 Microsoft.AspNetCore.Components.Endpoints.RootComponentMetadata.RootComponentMetadata(System.Type! rootComponentType) -> void
 Microsoft.AspNetCore.Components.Endpoints.RootComponentMetadata.Type.get -> System.Type!

src/Components/Endpoints/test/RazorComponentResultTest.cs

@@ -347,6 +347,7 @@ public async Task StreamingRendering_IsOffByDefault_AndCanBeEnabledForSubtree()
         // Act/Assert: Produce initial output, noting absence of streaming markers at top level
         testContext.TopLevelComponentTask.SetResult();
         await initialOutputTask;
+        await WaitForContentWrittenAsync(testContext.ResponseBody);
         var html = MaskComponentIds(GetStringContent(testContext.ResponseBody));
         Assert.StartsWith("[Top level component: Loaded]", html);
         Assert.Contains("[Within streaming region: <!--bl:X-->Loading...<!--/bl:X-->]", html);