[Blazor] Fixed encoded values on Blazor Router (#53470) (#53538)

As part of the routing unification process we switched the way we were
decoding the URL prior to feeding it to routing and that introduced a
small regression in interactive routing compared to .NET 7.0.

This commit fixes that regression by using the same logic for decoding
the URL in the client that is used on the server.

In addition to that, the Blazor router now post processes the URL to
replace instances of `%2F` with `/` when providing values to maintain
the behavior in 7.0 where it used UnescapeDataString.

This also makes the routing on the server and on the client consistent
in their handling of encoded `/` characters.

Author: javiercn

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

@@ -19,10 +19,11 @@
     <Compile Include="$(ComponentsSharedSourceRoot)src\HotReloadManager.cs" LinkBase="HotReload" />
     <Compile Include="$(SharedSourceRoot)LinkerFlags.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)QueryStringEnumerable.cs" LinkBase="Shared" />
+    <Compile Include="$(SharedSourceRoot)UrlDecoder\UrlDecoder.cs" LinkBase="Shared" />
   </ItemGroup>
 
   <Import Project="Microsoft.AspNetCore.Components.Routing.targets" />
-  
+
   <ItemGroup>
     <Reference Include="Microsoft.Extensions.Logging.Abstractions" />
     <Reference Include="Microsoft.Extensions.DependencyInjection.Abstractions" />

src/Components/Components/src/Routing/RouteContext.cs

@@ -1,7 +1,11 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Buffers;
 using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;
+using System.Text;
+using Microsoft.AspNetCore.Internal;
 using Microsoft.AspNetCore.Routing.Tree;
 using static Microsoft.AspNetCore.Internal.LinkerFlags;
 
@@ -11,7 +15,28 @@ internal sealed class RouteContext
 {
     public RouteContext(string path)
     {
-        Path = Uri.UnescapeDataString(path);
+        Path = path.Contains('%') ? GetDecodedPath(path) : path;
+
+        [SkipLocalsInit]
+        static string GetDecodedPath(string path)
+        {
+            using var uriBuffer = path.Length < 128 ?
+                new UriBuffer(stackalloc byte[path.Length]) :
+                new UriBuffer(path.Length);
+
+            var utf8Span = uriBuffer.Buffer;
+
+            if (Encoding.UTF8.TryGetBytes(path.AsSpan(), utf8Span, out var written))
+            {
+                utf8Span = utf8Span[..written];
+                var decodedLength = UrlDecoder.DecodeInPlace(utf8Span, isFormEncoding: false);
+                utf8Span = utf8Span[..decodedLength];
+                path = Encoding.UTF8.GetString(utf8Span);
+                return path;
+            }
+
+            return path;
+        }
     }
 
     public string Path { get; set; }
@@ -24,4 +49,27 @@ public RouteContext(string path)
     public Type? Handler => Entry?.Handler;
 
     public IReadOnlyDictionary<string, object?>? Parameters => RouteValues;
+
+    private readonly ref struct UriBuffer
+    {
+        private readonly byte[]? _pooled;
+
+        public Span<byte> Buffer { get; }
+
+        public UriBuffer(int length)
+        {
+            _pooled = ArrayPool<byte>.Shared.Rent(length);
+            Buffer = _pooled.AsSpan(0, length);
+        }
+
+        public UriBuffer(Span<byte> buffer) => Buffer = buffer;
+
+        public void Dispose()
+        {
+            if (_pooled != null)
+            {
+                ArrayPool<byte>.Shared.Return(_pooled);
+            }
+        }
+    }
 }

src/Components/Components/src/Routing/RouteTable.cs

@@ -28,6 +28,18 @@ internal static RouteData ProcessParameters(RouteData endpointRouteData)
                 ((Type page, string template) key) => RouteTableFactory.CreateEntry(key.page, key.template));
 
             var routeValueDictionary = new RouteValueDictionary(endpointRouteData.RouteValues);
+            foreach (var kvp in endpointRouteData.RouteValues)
+            {
+                if (kvp.Value is string value)
+                {
+                    // At this point the values have already been URL decoded, but we might not have decoded '/' characters.
+                    // as that can cause issues when routing the request (You wouldn't be able to accept parameters that contained '/').
+                    // To be consistent with existing Blazor quirks that used Uri.UnescapeDataString, we'll replace %2F with /.
+                    // We don't want to call Uri.UnescapeDataString here as that would decode other characters that we don't want to decode,
+                    // for example, any value that was "double" encoded (for whatever reason) within the original URL.
+                    routeValueDictionary[kvp.Key] = value.Replace("%2F", "/", StringComparison.OrdinalIgnoreCase);
+                }
+            }
             ProcessParameters(entry, routeValueDictionary);
             return new RouteData(endpointRouteData.PageType, routeValueDictionary)
             {
@@ -66,6 +78,19 @@ private static void ProcessParameters(InboundRouteEntry entry, RouteValueDiction
             }
         }
 
+        foreach (var kvp in routeValues)
+        {
+            if (kvp.Value is string value)
+            {
+                // At this point the values have already been URL decoded, but we might not have decoded '/' characters.
+                // as that can cause issues when routing the request (You wouldn't be able to accept parameters that contained '/').
+                // To be consistent with existing Blazor quirks that used Uri.UnescapeDataString, we'll replace %2F with /.
+                // We don't want to call Uri.UnescapeDataString here as that would decode other characters that we don't want to decode,
+                // for example, any value that was "double" encoded (for whatever reason) within the original URL.
+                routeValues[kvp.Key] = value.Replace("%2F", "/", StringComparison.OrdinalIgnoreCase);
+            }
+        }
+
         foreach (var parameter in entry.RoutePattern.Parameters)
         {
             // Add null values for optional route parameters that weren't provided.

src/Components/test/E2ETest/ServerRenderingTests/UnifiedRoutingTests.cs

@@ -24,10 +24,19 @@ public UnifiedRoutingTests(
     public override Task InitializeAsync()
         => InitializeAsync(BrowserFixture.StreamingContext);
 
-    [Fact]
-    public void Routing_CanRenderPagesWithParameters_And_TransitionToInteractive()
+    [Theory]
+    [InlineData("routing/parameters/value", "value")]
+    // Issue 53138
+    [InlineData("%F0%9F%99%82/routing/parameters/http%3A%2F%2Fwww.example.com%2Flogin%2Fcallback", "http://www.example.com/login/callback")]
+    // Note this double encodes the final 2 slashes
+    [InlineData("%F0%9F%99%82/routing/parameters/http%3A%2F%2Fwww.example.com%2520login%2520callback", "http://www.example.com%20login%20callback")]
+    // Issue 53262
+    [InlineData("routing/parameters/%21%40%23%24%25%5E%26%2A%28%29_%2B-%3D%5B%5D%7B%7D%5C%5C%7C%3B%27%3A%5C%22%3E%3F.%2F", """!@#$%^&*()_+-=[]{}\\|;':\">?./""")]
+    // Issue 52808
+    [InlineData("routing/parameters/parts%20w%2F%20issue", "parts w/ issue")]
+    public void Routing_CanRenderPagesWithParameters_And_TransitionToInteractive(string url, string expectedValue)
     {
-        ExecuteRoutingTestCore("routing/parameters/value", "value");
+        ExecuteRoutingTestCore(url, expectedValue);
     }
 
     [Fact]
@@ -36,10 +45,12 @@ public void Routing_CanRenderPagesWithConstrainedParameters_And_TransitionToInte
         ExecuteRoutingTestCore("routing/constraints/5", "5");
     }
 
-    [Fact]
-    public void Routing_CanRenderPagesWithComplexSegments_And_TransitionToInteractive()
+    [Theory]
+    [InlineData("routing/complex-segment(value)", "value")]
+    [InlineData("%F0%9F%99%82/routing/%F0%9F%99%82complex-segment(http%3A%2F%2Fwww.example.com%2Flogin%2Fcallback)", "http://www.example.com/login/callback")]
+    public void Routing_CanRenderPagesWithComplexSegments_And_TransitionToInteractive(string url, string expectedValue)
     {
-        ExecuteRoutingTestCore("routing/complex-segment(value)", "value");
+        ExecuteRoutingTestCore(url, expectedValue);
     }
 
     [Fact]
@@ -54,10 +65,12 @@ public void Routing_CanRenderPagesWithOptionalParameters_And_TransitionToInterac
         ExecuteRoutingTestCore("routing/optional", "null");
     }
 
-    [Fact]
-    public void Routing_CanRenderPagesWithCatchAllParameters_And_TransitionToInteractive()
+    [Theory]
+    [InlineData("routing/catch-all/rest/of/the/path", "rest/of/the/path")]
+    [InlineData("%F0%9F%99%82/routing/catch-all/http%3A%2F%2Fwww.example.com%2Flogin%2Fcallback/another", "http://www.example.com/login/callback/another")]
+    public void Routing_CanRenderPagesWithCatchAllParameters_And_TransitionToInteractive(string url, string expectedValue)
     {
-        ExecuteRoutingTestCore("routing/catch-all/rest/of/the/path", "rest/of/the/path");
+        ExecuteRoutingTestCore(url, expectedValue);
     }
 
     [Fact]

src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/Routing/Encoded/EncodedCatchAll.razor

@@ -0,0 +1,25 @@
+@page "/🙂/routing/catch-all/{*parameter}"
+
+<h3>Catch all</h3>
+
+<p id="parameter-value">@Parameter</p>
+
+@if (_interactive)
+{
+    <p id="interactive">Rendered interactive.</p>
+}
+
+@code {
+    private bool _interactive;
+
+    [Parameter] public string Parameter { get; set; }
+
+    protected override void OnAfterRender(bool firstRender)
+    {
+        if (firstRender)
+        {
+            _interactive = true;
+            StateHasChanged();
+        }
+    }
+}

src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/Routing/Encoded/EncodedComplexSegments.razor

@@ -0,0 +1,24 @@
+@page "/🙂/routing/🙂complex-segment({parameter})"
+<h3>Complex segment parameters</h3>
+
+<p id="parameter-value">@Parameter</p>
+
+@if(_interactive)
+{
+    <p id="interactive">Rendered interactive.</p>
+}
+
+@code {
+    private bool _interactive;
+
+    [Parameter] public string Parameter { get; set; }
+
+    protected override void OnAfterRender(bool firstRender)
+    {
+        if (firstRender)
+        {
+            _interactive = true;
+            StateHasChanged();
+        }
+    }
+}

src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/Routing/Encoded/EncodedParameters.razor

@@ -0,0 +1,24 @@
+@page "/🙂/routing/parameters/{parameter}"
+<h3>Parameters</h3>
+
+<p id="parameter-value">@Parameter</p>
+
+@if (_interactive)
+{
+    <p id="interactive">Rendered interactive.</p>
+}
+
+@code {
+    private bool _interactive;
+
+    [Parameter] public string Parameter { get; set; }
+
+    protected override void OnAfterRender(bool firstRender)
+    {
+        if (firstRender)
+        {
+            _interactive = true;
+            StateHasChanged();
+        }
+    }
+}

src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/Routing/RoutingTestCasesWithEncoding.razor

@@ -0,0 +1,15 @@
+@page "/🙂/routing"
+@inject NavigationManager NavigationManager
+<h3>Routing test cases with encoded urls</h3>
+
+<ul>
+    <li>
+        <a href="%F0%9F%99%82/routing/catch-all/http%3A%2F%2Fwww.example.com%2Flogin%2Fcallback/another">Catch all</a>
+    </li>
+    <li>
+        <a href="%F0%9F%99%82/routing/parameters/http%3A%2F%2Fwww.example.com%2Flogin%2Fcallback">Parameters</a>
+    </li>
+    <li>
+        <a href="%F0%9F%99%82/routing/%F0%9F%99%82complex-segment(http%3A%2F%2Fwww.example.com%2Flogin%2Fcallback)">Complex segments</a>
+    </li>
+</ul>