Make enhanced nav much more conservative, and better handle redirections

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

@@ -0,0 +1,94 @@
+// 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 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,
+    // rather than a true 301/302/etc. But we don't want to disclose the redirection target URL to JS because that
+    // info would not normally be available, e.g., when using 'fetch'. So we data-protect the URL and round trip
+    // through a special endpoint that can issue a true redirection.
+    //
+    // The same is used during enhanced navigation if it happens to go to a Blazor endpoint that calls
+    // NavigationManager.NavigateTo, for the same reasons.
+    //
+    // However, if enhanced navigation goes to a non-Blazor endpoint, the server won't do anything special and just
+    // returns a regular 301/302/etc. To handle this,
+    //
+    //  - If it's redirected to an internal URL, the browser will just follow the redirection automatically
+    //    and client-side code will then:
+    //    - Check if it went to a Blazor endpoint, and if so, simply update the client-side URL to match
+    //    - Or if it's a non-Blazor endpoint, behaves like "external URL" below
+    //  - If it's to an external URL:
+    //    - If it's a GET request, the client-side code will retry as a non-enhanced request
+    //    - For other request types, we have to let it fail as it would be unsafe to retry
+
+    private const string RedirectionDataProtectionProviderPurpose = "Microsoft.AspNetCore.Components.Endpoints.OpaqueRedirection,V1";
+    private const string RedirectionEndpointBaseRelativeUrl = "_framework/opaque-redirect";
+
+    public static string CreateProtectedRedirectionUrl(HttpContext httpContext, string destinationUrl)
+    {
+        var protector = CreateProtector(httpContext);
+        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)}";
+    }
+
+    public static void AddBlazorOpaqueRedirectionEndpoint(IEndpointRouteBuilder endpoints)
+    {
+        endpoints.MapGet($"/{RedirectionEndpointBaseRelativeUrl}", httpContext =>
+        {
+            if (!httpContext.Request.Query.TryGetValue("url", out var protectedUrl))
+            {
+                httpContext.Response.StatusCode = 400;
+                return Task.CompletedTask;
+            }
+
+            var protector = CreateProtector(httpContext);
+            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;
+        });
+    }
+
+    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/Builder/RazorComponentsEndpointRouteBuilderExtensions.cs

@@ -31,6 +31,7 @@ public static class RazorComponentsEndpointRouteBuilderExtensions
 
         EnsureRazorComponentServices(endpoints);
         AddBlazorWebJsEndpoint(endpoints);
+        OpaqueRedirection.AddBlazorOpaqueRedirectionEndpoint(endpoints);
 
         return GetOrCreateDataSource<TRootComponent>(endpoints).DefaultBuilder;
     }

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

@@ -29,6 +29,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/src/RazorComponentEndpointInvoker.cs

@@ -35,6 +35,7 @@ private async Task RenderComponentCore(HttpContext context)
     {
         context.Response.ContentType = RazorComponentResultExecutor.DefaultContentType;
         _renderer.InitializeStreamingRenderingFraming(context);
+        EndpointHtmlRenderer.MarkAsAllowingEnhancedNavigation(context);
 
         var endpoint = context.GetEndpoint() ?? throw new InvalidOperationException($"An endpoint must be set on the '{nameof(HttpContext)}'.");
 

src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.Prerendering.cs

@@ -50,6 +50,11 @@ protected override IComponent ResolveComponentForRenderMode([DynamicallyAccessed
         return null;
     }
 
+    public static void MarkAsAllowingEnhancedNavigation(HttpContext context)
+    {
+        context.Response.Headers.Add("blazor-enhanced-nav", "allow");
+    }
+
     public ValueTask<IHtmlAsyncContent> PrerenderComponentAsync(
         HttpContext httpContext,
         [DynamicallyAccessedMembers(Component)] Type componentType,
@@ -149,13 +154,15 @@ public static ValueTask<PrerenderedComponentHtmlContent> HandleNavigationExcepti
                 "Navigation commands can not be issued during server-side prerendering after the response from the server has started. Applications must buffer the" +
                 "response and avoid using features like FlushAsync() before all components on the page have been rendered to prevent failed navigation commands.");
         }
-        else if (IsPossibleExternalDestination(httpContext.Request, navigationException.Location) && httpContext.Request.Headers.ContainsKey("blazor-enhanced-nav"))
+        else if (IsPossibleExternalDestination(httpContext.Request, navigationException.Location)
+            && IsProgressivelyEnhancedNavigation(httpContext.Request))
         {
-            // It's unsafe to do a 301/302/etc to an external destination when this was requested via fetch, because
-            // assuming it doesn't expose CORS headers, we won't be allowed to follow the redirection nor will
-            // we even find out what the destination URL would have been. But since it's our own JS code making this
-            // fetch request, we can have a custom protocol for describing the URL we wanted to redirect to.
-            httpContext.Response.Headers.Add("blazor-enhanced-nav-redirect-location", navigationException.Location);
+            // For progressively-enhanced nav, we prefer to use opaque redirections for external URLs rather than
+            // forcing the request to be retried, since that allows post-redirect-get to work, plus avoids a
+            // duplicated request. The client can't rely on receiving this header, though, since non-Blazor endpoints
+            // wouldn't return it.
+            httpContext.Response.Headers.Add("blazor-enhanced-nav-redirect-location",
+                OpaqueRedirection.CreateProtectedRedirectionUrl(httpContext, navigationException.Location));
             return new ValueTask<PrerenderedComponentHtmlContent>(PrerenderedComponentHtmlContent.Empty);
         }
         else

src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.Streaming.cs

@@ -15,15 +15,14 @@ namespace Microsoft.AspNetCore.Components.Endpoints;
 
 internal partial class EndpointHtmlRenderer
 {
-    private const string _progressivelyEnhancedNavRequestHeaderName = "blazor-enhanced-nav";
     private const string _streamingRenderingFramingHeaderName = "ssr-framing";
     private TextWriter? _streamingUpdatesWriter;
     private HashSet<int>? _visitedComponentIdsInCurrentStreamingBatch;
     private string? _ssrFramingCommentMarkup;
 
     public void InitializeStreamingRenderingFraming(HttpContext httpContext)
     {
-        if (httpContext.Request.Headers.ContainsKey(_progressivelyEnhancedNavRequestHeaderName))
+        if (IsProgressivelyEnhancedNavigation(httpContext.Request))
         {
             var id = Guid.NewGuid().ToString();
             httpContext.Response.Headers.Add(_streamingRenderingFramingHeaderName, id);
@@ -60,7 +59,7 @@ public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilT
         }
         catch (NavigationException navigationException)
         {
-            HandleNavigationAfterResponseStarted(writer, navigationException.Location);
+            HandleNavigationAfterResponseStarted(writer, httpContext, navigationException.Location);
         }
         catch (Exception ex)
         {
@@ -176,10 +175,22 @@ private static void HandleExceptionAfterResponseStarted(HttpContext httpContext,
         writer.Write("</template><blazor-ssr-end></blazor-ssr-end></blazor-ssr>");
     }
 
-    private static void HandleNavigationAfterResponseStarted(TextWriter writer, string destinationUrl)
+    private static void HandleNavigationAfterResponseStarted(TextWriter writer, HttpContext httpContext, string destinationUrl)
     {
-        writer.Write("<blazor-ssr><template type=\"redirection\">");
-        writer.Write(HtmlEncoder.Default.Encode(destinationUrl));
+        writer.Write("<blazor-ssr><template type=\"redirection\"");
+
+        if (string.Equals(httpContext.Request.Method, "POST", StringComparison.OrdinalIgnoreCase))
+        {
+            writer.Write(" from=\"form-post\"");
+        }
+
+        if (IsProgressivelyEnhancedNavigation(httpContext.Request))
+        {
+            writer.Write(" enhanced=\"true\"");
+        }
+
+        writer.Write(">");
+        writer.Write(HtmlEncoder.Default.Encode(OpaqueRedirection.CreateProtectedRedirectionUrl(httpContext, destinationUrl)));
         writer.Write("</template><blazor-ssr-end></blazor-ssr-end></blazor-ssr>");
     }
 
@@ -243,6 +254,13 @@ private void WriteComponentHtml(int componentId, TextWriter output, bool allowBo
         }
     }
 
+    private static bool IsProgressivelyEnhancedNavigation(HttpRequest request)
+    {
+        // For enhanced nav, the Blazor JS code controls the "accept" header precisely, so we can be very specific about the format
+        var accept = request.Headers.Accept;
+        return accept.Count == 1 && string.Equals(accept[0]!, "text/html;blazor-enhanced-nav=on", StringComparison.Ordinal);
+    }
+
     private readonly struct ComponentIdAndDepth
     {
         public int ComponentId { get; }

src/Components/Endpoints/src/Results/RazorComponentResultExecutor.cs

@@ -10,6 +10,7 @@
 using Microsoft.Extensions.DependencyInjection;
 using static Microsoft.AspNetCore.Internal.LinkerFlags;
 using Microsoft.AspNetCore.Http.HttpResults;
+using Microsoft.AspNetCore.Components.Endpoints.Rendering;
 
 namespace Microsoft.AspNetCore.Components.Endpoints;
 
@@ -46,6 +47,7 @@ private static Task RenderComponentToResponse(
         return endpointHtmlRenderer.Dispatcher.InvokeAsync(async () =>
         {
             endpointHtmlRenderer.InitializeStreamingRenderingFraming(httpContext);
+            EndpointHtmlRenderer.MarkAsAllowingEnhancedNavigation(httpContext);
 
             // We could pool these dictionary instances if we wanted, and possibly even the ParameterView
             // backing buffers could come from a pool like they do during rendering.
@@ -55,7 +57,10 @@ private static Task RenderComponentToResponse(
                 { nameof(RazorComponentEndpointHost.ComponentParameters), componentParameters },
             });
 
-            await using var writer = CreateResponseWriter(httpContext.Response.Body);
+            // Matches MVC's MemoryPoolHttpResponseStreamWriterFactory.DefaultBufferSize
+            var defaultBufferSize = 16 * 1024;
+            await using var writer = new HttpResponseStreamWriter(httpContext.Response.Body, Encoding.UTF8, defaultBufferSize, ArrayPool<byte>.Shared, ArrayPool<char>.Shared);
+            using var bufferWriter = new BufferedTextWriter(writer);
 
             // Note that we don't set any interactive rendering mode for the top-level output from a RazorComponentResult,
             // because you never want to serialize the invocation of RazorComponentResultHost. Instead, that host
@@ -71,24 +76,17 @@ private static Task RenderComponentToResponse(
             // in between the first call to htmlContent.WriteTo and the point where we start listening for subsequent
             // streaming SSR batches (inside SendStreamingUpdatesAsync). Otherwise some other code might dispatch to the
             // renderer sync context and cause a batch that would get missed.
-            htmlContent.WriteTo(writer, HtmlEncoder.Default); // Don't use WriteToAsync, as per the comment above
+            htmlContent.WriteTo(bufferWriter, HtmlEncoder.Default); // Don't use WriteToAsync, as per the comment above
 
-            if (!htmlContent.QuiescenceTask.IsCompleted)
+            if (!htmlContent.QuiescenceTask.IsCompletedSuccessfully)
             {
-                await endpointHtmlRenderer.SendStreamingUpdatesAsync(httpContext, htmlContent.QuiescenceTask, writer);
+                await endpointHtmlRenderer.SendStreamingUpdatesAsync(httpContext, htmlContent.QuiescenceTask, bufferWriter);
             }
 
             // Invoke FlushAsync to ensure any buffered content is asynchronously written to the underlying
             // response asynchronously. In the absence of this line, the buffer gets synchronously written to the
             // response as part of the Dispose which has a perf impact.
-            await writer.FlushAsync();
+            await bufferWriter.FlushAsync();
         });
     }
-
-    private static TextWriter CreateResponseWriter(Stream bodyStream)
-    {
-        // Matches MVC's MemoryPoolHttpResponseStreamWriterFactory.DefaultBufferSize
-        const int DefaultBufferSize = 16 * 1024;
-        return new HttpResponseStreamWriter(bodyStream, Encoding.UTF8, DefaultBufferSize, ArrayPool<byte>.Shared, ArrayPool<char>.Shared);
-    }
 }