From 4184b2406d8c65dda7f7fb4faf365f79a2b45f02 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Tue, 10 Apr 2018 05:07:39 -0700 Subject: [PATCH] Updated to make routing always use UrlEncoder.Default and not depend on DI to get it. [Fixes #513] RedirectToAction with Non-English Characters in Parameters and Authentication Causes Error --- .../RoutingBenchmark.cs | 6 +- .../RoutingServiceCollectionExtensions.cs | 14 +- .../UriBuilderContextPooledObjectPolicy.cs | 15 +- src/Microsoft.AspNetCore.Routing/RouteBase.cs | 3 +- .../Template/TemplateBinder.cs | 11 +- .../Tree/TreeRouteBuilder.cs | 35 ++++- .../RouteTest.cs | 33 ++++ .../Template/TemplateBinderTests.cs | 144 ++++++++---------- .../Tree/TreeRouteBuilderTest.cs | 3 +- .../Tree/TreeRouterTest.cs | 86 ++++++++++- 10 files changed, 232 insertions(+), 118 deletions(-) diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/RoutingBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/RoutingBenchmark.cs index 303fb5e811..d499fd706a 100644 --- a/benchmarks/Microsoft.AspNetCore.Routing.Performance/RoutingBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/RoutingBenchmark.cs @@ -7,15 +7,12 @@ using System.Text.Encodings.Web; using System.Threading.Tasks; using BenchmarkDotNet.Attributes; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Template; using Microsoft.AspNetCore.Routing.Tree; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.Options; -using System.Diagnostics; namespace Microsoft.AspNetCore.Routing.Performance { @@ -33,8 +30,7 @@ namespace Microsoft.AspNetCore.Routing.Performance var treeBuilder = new TreeRouteBuilder( NullLoggerFactory.Instance, - UrlEncoder.Default, - new DefaultObjectPool(new UriBuilderContextPooledObjectPolicy(UrlEncoder.Default)), + new DefaultObjectPool(new UriBuilderContextPooledObjectPolicy()), new DefaultInlineConstraintResolver(new OptionsManager(new OptionsFactory(Enumerable.Empty>(), Enumerable.Empty>())))); treeBuilder.MapInbound(handler, TemplateParser.Parse("api/Widgets"), "default", 0); diff --git a/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs index a0b2b6b891..d6883f6c95 100644 --- a/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs @@ -2,11 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Text.Encodings.Web; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Tree; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.ObjectPool; namespace Microsoft.Extensions.DependencyInjection @@ -29,17 +29,21 @@ namespace Microsoft.Extensions.DependencyInjection } services.TryAddTransient(); - services.TryAddSingleton(UrlEncoder.Default); services.TryAddSingleton>(s => { var provider = s.GetRequiredService(); - var encoder = s.GetRequiredService(); - return provider.Create(new UriBuilderContextPooledObjectPolicy(encoder)); + return provider.Create(new UriBuilderContextPooledObjectPolicy()); }); // The TreeRouteBuilder is a builder for creating routes, it should stay transient because it's // stateful. - services.TryAddTransient(); + services.TryAdd(ServiceDescriptor.Transient(s => + { + var loggerFactory = s.GetRequiredService(); + var objectPool = s.GetRequiredService>(); + var constraintResolver = s.GetRequiredService(); + return new TreeRouteBuilder(loggerFactory, objectPool, constraintResolver); + })); services.TryAddSingleton(typeof(RoutingMarkerService)); diff --git a/src/Microsoft.AspNetCore.Routing/Internal/UriBuilderContextPooledObjectPolicy.cs b/src/Microsoft.AspNetCore.Routing/Internal/UriBuilderContextPooledObjectPolicy.cs index 0db44758d9..953d6a86c4 100644 --- a/src/Microsoft.AspNetCore.Routing/Internal/UriBuilderContextPooledObjectPolicy.cs +++ b/src/Microsoft.AspNetCore.Routing/Internal/UriBuilderContextPooledObjectPolicy.cs @@ -1,7 +1,6 @@ // 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.Text.Encodings.Web; using Microsoft.Extensions.ObjectPool; @@ -9,21 +8,9 @@ namespace Microsoft.AspNetCore.Routing.Internal { public class UriBuilderContextPooledObjectPolicy : IPooledObjectPolicy { - private readonly UrlEncoder _encoder; - - public UriBuilderContextPooledObjectPolicy(UrlEncoder encoder) - { - if (encoder == null) - { - throw new ArgumentNullException(nameof(encoder)); - } - - _encoder = encoder; - } - public UriBuildingContext Create() { - return new UriBuildingContext(_encoder); + return new UriBuildingContext(UrlEncoder.Default); } public bool Return(UriBuildingContext obj) diff --git a/src/Microsoft.AspNetCore.Routing/RouteBase.cs b/src/Microsoft.AspNetCore.Routing/RouteBase.cs index 64a7be7693..6d1eb7bd27 100644 --- a/src/Microsoft.AspNetCore.Routing/RouteBase.cs +++ b/src/Microsoft.AspNetCore.Routing/RouteBase.cs @@ -241,9 +241,8 @@ namespace Microsoft.AspNetCore.Routing { if (_binder == null) { - var urlEncoder = context.RequestServices.GetRequiredService(); var pool = context.RequestServices.GetRequiredService>(); - _binder = new TemplateBinder(urlEncoder, pool, ParsedTemplate, Defaults); + _binder = new TemplateBinder(UrlEncoder.Default, pool, ParsedTemplate, Defaults); } } diff --git a/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs b/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs index 68770fd2bc..802352e935 100644 --- a/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs +++ b/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs @@ -20,6 +20,13 @@ namespace Microsoft.AspNetCore.Routing.Template private readonly RouteValueDictionary _filters; private readonly RouteTemplate _template; + /// + /// Creates a new instance of . + /// + /// The . + /// The . + /// The to bind values to. + /// The default values for . public TemplateBinder( UrlEncoder urlEncoder, ObjectPool pool, @@ -90,8 +97,8 @@ namespace Microsoft.AspNetCore.Routing.Template } } - if (!hasNewParameterValue && - !hasCurrentParameterValue && + if (!hasNewParameterValue && + !hasCurrentParameterValue && _defaults?.ContainsKey(parameter.Name) != true) { // This is an unsatisfied parameter value and there are no defaults. We might still diff --git a/src/Microsoft.AspNetCore.Routing/Tree/TreeRouteBuilder.cs b/src/Microsoft.AspNetCore.Routing/Tree/TreeRouteBuilder.cs index e5f7d80823..a746e7d170 100644 --- a/src/Microsoft.AspNetCore.Routing/Tree/TreeRouteBuilder.cs +++ b/src/Microsoft.AspNetCore.Routing/Tree/TreeRouteBuilder.cs @@ -25,28 +25,49 @@ namespace Microsoft.AspNetCore.Routing.Tree private readonly IInlineConstraintResolver _constraintResolver; /// - /// Initializes a new instance of . + /// + /// This constructor is obsolete and will be removed in a future version. The recommended + /// alternative is the overload that does not take a UrlEncoder. + /// + /// Initializes a new instance of . /// /// The . /// The . /// The . /// The . + [Obsolete("This constructor is obsolete and will be removed in a future version. The recommended " + + "alternative is the overload that does not take a UrlEncoder.")] public TreeRouteBuilder( ILoggerFactory loggerFactory, UrlEncoder urlEncoder, ObjectPool objectPool, IInlineConstraintResolver constraintResolver) + : this(loggerFactory, objectPool, constraintResolver) + { + if (urlEncoder == null) + { + throw new ArgumentNullException(nameof(urlEncoder)); + } + + _urlEncoder = urlEncoder; + } + + /// + /// Initializes a new instance of . + /// + /// The . + /// The . + /// The . + public TreeRouteBuilder( + ILoggerFactory loggerFactory, + ObjectPool objectPool, + IInlineConstraintResolver constraintResolver) { if (loggerFactory == null) { throw new ArgumentNullException(nameof(loggerFactory)); } - if (urlEncoder == null) - { - throw new ArgumentNullException(nameof(urlEncoder)); - } - if (objectPool == null) { throw new ArgumentNullException(nameof(objectPool)); @@ -57,7 +78,7 @@ namespace Microsoft.AspNetCore.Routing.Tree throw new ArgumentNullException(nameof(constraintResolver)); } - _urlEncoder = urlEncoder; + _urlEncoder = UrlEncoder.Default; _objectPool = objectPool; _constraintResolver = constraintResolver; diff --git a/test/Microsoft.AspNetCore.Routing.Tests/RouteTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/RouteTest.cs index 7eee101ebb..e6ced54ff0 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/RouteTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/RouteTest.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; +using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; @@ -14,6 +15,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.Options; +using Microsoft.Extensions.WebEncoders.Testing; using Moq; using Xunit; @@ -647,6 +649,37 @@ namespace Microsoft.AspNetCore.Routing Assert.Empty(pathData.DataTokens); } + [Fact] + public void GetVirtualPath_AlwaysUsesDefaultUrlEncoder() + { + // Arrange + var services = new ServiceCollection(); + services.AddSingleton(NullLoggerFactory.Instance); + services.AddSingleton(); + services.AddRouting(); + // This test encoder should not be used by Routing and should always use the default one. + services.AddSingleton(new UrlTestEncoder()); + var httpContext = new DefaultHttpContext + { + RequestServices = services.BuildServiceProvider(), + }; + + var context = new VirtualPathContext( + httpContext, + values: new RouteValueDictionary(new { name = "name with %special #characters Jörn" }), + ambientValues: new RouteValueDictionary(new { controller = "Home", action = "Index" })); + + var route = CreateRoute("{controller}/{action}"); + + // Act + var pathData = route.GetVirtualPath(context); + + // Assert + Assert.Equal("/Home/Index?name=name%20with%20%25special%20%23characters%20J%C3%B6rn", pathData.VirtualPath); + Assert.Same(route, pathData.Router); + Assert.Empty(pathData.DataTokens); + } + [Fact] public void GetVirtualPath_ForListOfStrings() { diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateBinderTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateBinderTests.cs index ab06c3f2cc..d4dacaaa6b 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateBinderTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateBinderTests.cs @@ -9,7 +9,6 @@ using Microsoft.AspNetCore.Routing.Internal; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.Options; -using Microsoft.Extensions.WebEncoders.Testing; using Xunit; namespace Microsoft.AspNetCore.Routing.Template.Tests @@ -31,7 +30,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests "Test/{val1}/{val2}", new RouteValueDictionary(new {val1 = "", val2 = ""}), new RouteValueDictionary(new {val1 = "a"}), - "/UrlEncode[[Test]]/UrlEncode[[a]]" + "/Test/a" }, { "Test/{val1}/{val2}/{val3}", @@ -43,67 +42,67 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests "Test/{val1}/{val2}", new RouteValueDictionary(new {val1 = "", val2 = ""}), new RouteValueDictionary(new {val1 = "a", val2 = "b"}), - "/UrlEncode[[Test]]/UrlEncode[[a]]/UrlEncode[[b]]" + "/Test/a/b" }, { "Test/{val1}/{val2}/{val3}", new RouteValueDictionary(new {val1 = "", val2 = "", val3 = ""}), new RouteValueDictionary(new {val1 = "a", val2 = "b", val3 = "c"}), - "/UrlEncode[[Test]]/UrlEncode[[a]]/UrlEncode[[b]]/UrlEncode[[c]]" + "/Test/a/b/c" }, { "Test/{val1}/{val2}/{val3}", new RouteValueDictionary(new {val1 = "", val2 = "", val3 = ""}), new RouteValueDictionary(new {val1 = "a", val2 = "b"}), - "/UrlEncode[[Test]]/UrlEncode[[a]]/UrlEncode[[b]]" + "/Test/a/b" }, { "Test/{val1}/{val2}/{val3}", new RouteValueDictionary(new {val1 = "", val2 = "", val3 = ""}), new RouteValueDictionary(new {val1 = "a"}), - "/UrlEncode[[Test]]/UrlEncode[[a]]" + "/Test/a" }, { "Test/{val1}", new RouteValueDictionary(new {val1 = "42", val2 = "", val3 = ""}), new RouteValueDictionary(), - "/UrlEncode[[Test]]" + "/Test" }, { "Test/{val1}/{val2}/{val3}", new RouteValueDictionary(new {val1 = "42", val2 = (string)null, val3 = (string)null}), new RouteValueDictionary(), - "/UrlEncode[[Test]]" + "/Test" }, { "Test/{val1}/{val2}/{val3}/{val4}", new RouteValueDictionary(new {val1 = "21", val2 = "", val3 = "", val4 = ""}), new RouteValueDictionary(new {val1 = "42", val2 = "11", val3 = "", val4 = ""}), - "/UrlEncode[[Test]]/UrlEncode[[42]]/UrlEncode[[11]]" + "/Test/42/11" }, { "Test/{val1}/{val2}/{val3}", new RouteValueDictionary(new {val1 = "21", val2 = "", val3 = ""}), new RouteValueDictionary(new {val1 = "42"}), - "/UrlEncode[[Test]]/UrlEncode[[42]]" + "/Test/42" }, { "Test/{val1}/{val2}/{val3}/{val4}", new RouteValueDictionary(new {val1 = "21", val2 = "", val3 = "", val4 = ""}), new RouteValueDictionary(new {val1 = "42", val2 = "11"}), - "/UrlEncode[[Test]]/UrlEncode[[42]]/UrlEncode[[11]]" + "/Test/42/11" }, { "Test/{val1}/{val2}/{val3}", new RouteValueDictionary(new {val1 = "21", val2 = (string)null, val3 = (string)null}), new RouteValueDictionary(new {val1 = "42"}), - "/UrlEncode[[Test]]/UrlEncode[[42]]" + "/Test/42" }, { "Test/{val1}/{val2}/{val3}/{val4}", new RouteValueDictionary(new {val1 = "21", val2 = (string)null, val3 = (string)null, val4 = (string)null}), new RouteValueDictionary(new {val1 = "42", val2 = "11"}), - "/UrlEncode[[Test]]/UrlEncode[[42]]/UrlEncode[[11]]" + "/Test/42/11" }, }; @@ -116,10 +115,9 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests string expected) { // Arrange - var encoder = new UrlTestEncoder(); var binder = new TemplateBinder( - encoder, - new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy(encoder)), + UrlEncoder.Default, + new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy()), TemplateParser.Parse(template), defaults); @@ -157,7 +155,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "xx", region = "yy" }), - "/UrlEncode[[language]]/UrlEncode[[xx]]UrlEncode[[-]]UrlEncode[[yy]]"); + "/language/xx-yy"); } [Fact] @@ -168,7 +166,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "xx", region = "yy" }), - "/UrlEncode[[language]]/UrlEncode[[xx]]UrlEncode[[-]]UrlEncode[[yy]]UrlEncode[[a]]"); + "/language/xx-yya"); } [Fact] @@ -179,7 +177,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "xx", region = "yy" }), - "/UrlEncode[[language]]/UrlEncode[[a]]UrlEncode[[xx]]UrlEncode[[-]]UrlEncode[[yy]]"); + "/language/axx-yy"); } public static TheoryData OptionalParamValues => @@ -193,66 +191,66 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), new RouteValueDictionary(new {val3 = "someval3"}), new RouteValueDictionary(new {val3 = "someval3"}), - "/UrlEncode[[Test]]/UrlEncode[[someval1]]/UrlEncode[[someval2]]UrlEncode[[.]]UrlEncode[[someval3]]" + "/Test/someval1/someval2.someval3" }, { "Test/{val1}/{val2}.{val3?}", new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), new RouteValueDictionary(new {val3 = "someval3a"}), new RouteValueDictionary(new {val3 = "someval3v"}), - "/UrlEncode[[Test]]/UrlEncode[[someval1]]/UrlEncode[[someval2]]UrlEncode[[.]]UrlEncode[[someval3v]]" + "/Test/someval1/someval2.someval3v" }, { "Test/{val1}/{val2}.{val3?}", new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), new RouteValueDictionary(new {val3 = "someval3a"}), new RouteValueDictionary(), - "/UrlEncode[[Test]]/UrlEncode[[someval1]]/UrlEncode[[someval2]]UrlEncode[[.]]UrlEncode[[someval3a]]" + "/Test/someval1/someval2.someval3a" }, { "Test/{val1}/{val2}.{val3?}", new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), new RouteValueDictionary(), new RouteValueDictionary(new {val3 = "someval3v"}), - "/UrlEncode[[Test]]/UrlEncode[[someval1]]/UrlEncode[[someval2]]UrlEncode[[.]]UrlEncode[[someval3v]]" + "/Test/someval1/someval2.someval3v" }, { "Test/{val1}/{val2}.{val3?}", new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), new RouteValueDictionary(), new RouteValueDictionary(), - "/UrlEncode[[Test]]/UrlEncode[[someval1]]/UrlEncode[[someval2]]" + "/Test/someval1/someval2" }, { "Test/{val1}.{val2}.{val3}.{val4?}", new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), new RouteValueDictionary(), new RouteValueDictionary(new {val4 = "someval4", val3 = "someval3" }), - "/UrlEncode[[Test]]/UrlEncode[[someval1]]UrlEncode[[.]]UrlEncode[[someval2]]UrlEncode[[.]]" - + "UrlEncode[[someval3]]UrlEncode[[.]]UrlEncode[[someval4]]" + "/Test/someval1.someval2." + + "someval3.someval4" }, { "Test/{val1}.{val2}.{val3}.{val4?}", new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), new RouteValueDictionary(), new RouteValueDictionary(new {val3 = "someval3" }), - "/UrlEncode[[Test]]/UrlEncode[[someval1]]UrlEncode[[.]]UrlEncode[[someval2]]UrlEncode[[.]]" - + "UrlEncode[[someval3]]" + "/Test/someval1.someval2." + + "someval3" }, { "Test/.{val2?}", new RouteValueDictionary(new { }), new RouteValueDictionary(), new RouteValueDictionary(new {val2 = "someval2" }), - "/UrlEncode[[Test]]/UrlEncode[[.]]UrlEncode[[someval2]]" + "/Test/.someval2" }, { "Test/{val1}.{val2}", new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), new RouteValueDictionary(), new RouteValueDictionary(new {val3 = "someval3" }), - "/UrlEncode[[Test]]/UrlEncode[[someval1]]UrlEncode[[.]]UrlEncode[[someval2]]?" + - "UrlEncode[[val3]]=UrlEncode[[someval3]]" + "/Test/someval1.someval2?" + + "val3=someval3" }, }; @@ -266,10 +264,9 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests string expected) { // Arrange - var encoder = new UrlTestEncoder(); var binder = new TemplateBinder( - encoder, - new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy(encoder)), + UrlEncoder.Default, + new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy()), TemplateParser.Parse(template), defaults); @@ -307,7 +304,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "xx", region = "yy" }), - "/UrlEncode[[language]]/UrlEncode[[a]]UrlEncode[[xx]]UrlEncode[[-]]UrlEncode[[yy]]UrlEncode[[a]]"); + "/language/axx-yya"); } [Fact] @@ -340,7 +337,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en" }), new RouteValueDictionary(new { lang = "xx" }), - "/UrlEncode[[language]]/UrlEncode[[xx]]"); + "/language/xx"); } [Fact] @@ -351,7 +348,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en" }), new RouteValueDictionary(new { lang = "xx" }), - "/UrlEncode[[language]]/UrlEncode[[xx]]UrlEncode[[-]]"); + "/language/xx-"); } [Fact] @@ -362,7 +359,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en" }), new RouteValueDictionary(new { lang = "xx" }), - "/UrlEncode[[language]]/UrlEncode[[a]]UrlEncode[[xx]]"); + "/language/axx"); } [Fact] @@ -373,7 +370,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en" }), new RouteValueDictionary(new { lang = "xx" }), - "/UrlEncode[[language]]/UrlEncode[[a]]UrlEncode[[xx]]UrlEncode[[a]]"); + "/language/axxa"); } [Fact] @@ -384,7 +381,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { action = "Index", id = (string)null }), new RouteValueDictionary(new { controller = "home", action = "list", id = (string)null }), new RouteValueDictionary(new { controller = "products" }), - "/UrlEncode[[products]]UrlEncode[[.mvc]]"); + "/products.mvc"); } [Fact] @@ -395,7 +392,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { lang = "xx", region = "yy" }), new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "zz" }), - "/UrlEncode[[language]]/UrlEncode[[zz]]UrlEncode[[-]]UrlEncode[[yy]]"); + "/language/zz-yy"); } [Fact] @@ -407,7 +404,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { id = "defaultid" }), new RouteValueDictionary(new { controller = "home", action = "oldaction" }), new RouteValueDictionary(new { action = "newaction" }), - "/UrlEncode[[home]]/UrlEncode[[newaction]]"); + "/home/newaction"); } [Fact] @@ -440,7 +437,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { }), new RouteValueDictionary(new { controller = "home" }), - "/UrlEncode[[foo]]/UrlEncode[[home]]"); + "/foo/home"); } [Fact] @@ -452,7 +449,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { id = (string)null }), new RouteValueDictionary(new { controller = "home", action = "oldaction", id = (string)null }), new RouteValueDictionary(new { action = "newaction" }), - "/UrlEncode[[home]]/UrlEncode[[newaction]]"); + "/home/newaction"); } [Fact] @@ -463,7 +460,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { language = "en", locale = "US" }), new RouteValueDictionary(), new RouteValueDictionary(new { controller = "Orders" }), - "/UrlEncode[[Orders]]/UrlEncode[[en]]UrlEncode[[-]]UrlEncode[[US]]"); + "/Orders/en-US"); } [Fact] @@ -474,7 +471,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { action = "Index", id = "" }), new RouteValueDictionary(new { controller = "Home", action = "Index", id = "" }), new RouteValueDictionary(new { controller = "Home", action = "TestAction", id = "1", format = (string)null }), - "/UrlEncode[[Home]]UrlEncode[[.mvc]]/UrlEncode[[TestAction]]/UrlEncode[[1]]"); + "/Home.mvc/TestAction/1"); } [Fact] @@ -507,7 +504,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new { p2 = "d2", p3 = "d3" }, new { p1 = "v1", }, new { p2 = "", p3 = "" }, - "/UrlEncode[[v1]]"); + "/v1"); } [Fact] @@ -518,7 +515,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new { action = "Index", id = (string)null }, new { controller = "orig", action = "init", id = "123" }, new { action = "new", }, - "/UrlEncode[[orig]]/UrlEncode[[new]]"); + "/orig/new"); } [Fact] @@ -529,8 +526,8 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new { year = 1995, occasion = "Christmas", action = "Play", SafeParam = "SafeParamValue" }, new { controller = "UrlRouting", action = "Play", category = "Photos", year = "2008", occasion = "Easter", SafeParam = "SafeParamValue" }, new { year = (string)null, occasion = "Hola" }, - "/UrlEncode[[UrlGeneration1]]/UrlEncode[[UrlRouting]]UrlEncode[[.mvc]]/UrlEncode[[Play]]/" - + "UrlEncode[[Photos]]/UrlEncode[[1995]]/UrlEncode[[Hola]]"); + "/UrlGeneration1/UrlRouting.mvc/Play/" + + "Photos/1995/Hola"); } [Fact] @@ -553,8 +550,8 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { year = 1995, occasion = "Christmas", action = "Play", SafeParam = "SafeParamValue" }), ambientValues, values, - "/UrlEncode[[UrlGeneration1]]/UrlEncode[[UrlRouting]]UrlEncode[[.mvc]]/" - + "UrlEncode[[Play]]/UrlEncode[[Photos]]/UrlEncode[[1995]]/UrlEncode[[Hola]]"); + "/UrlGeneration1/UrlRouting.mvc/" + + "Play/Photos/1995/Hola"); } [Fact] @@ -576,7 +573,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { action = "Default" }), ambientValues, values, - "/UrlEncode[[subtest]]UrlEncode[[.mvc]]/UrlEncode[[Default]]/UrlEncode[[b]]"); + "/subtest.mvc/Default/b"); } [Fact] @@ -594,8 +591,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { controller = "Home" }), new RouteValueDictionary(new { controller = "home", action = "Index", id = (string)null }), values, - "/%23;%3F%3A@%26%3D%2B$,.mvc/showcategory/123?so%3Frt=de%3Fsc&maxPrice=100", - UrlEncoder.Default); + "/%23;%3F%3A@%26%3D%2B$,.mvc/showcategory/123?so%3Frt=de%3Fsc&maxPrice=100"); } [Fact] @@ -609,8 +605,8 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { controller = "Home" }), new RouteValueDictionary(new { controller = "home", action = "Index", id = (string)null }), values, - "/UrlEncode[[products]]UrlEncode[[.mvc]]/UrlEncode[[showcategory]]/UrlEncode[[123]]" + - "?UrlEncode[[so?rt]]=UrlEncode[[de?sc]]&UrlEncode[[maxPrice]]=UrlEncode[[100]]"); + "/products.mvc/showcategory/123" + + "?so%3Frt=de%3Fsc&maxPrice=100"); } [Fact] @@ -630,8 +626,8 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests maxPrice = 100, custom = "customValue" }), - "/UrlEncode[[products]]UrlEncode[[.mvc]]/UrlEncode[[showcategory]]/UrlEncode[[123]]" + - "?UrlEncode[[sort]]=UrlEncode[[desc]]&UrlEncode[[maxPrice]]=UrlEncode[[100]]"); + "/products.mvc/showcategory/123" + + "?sort=desc&maxPrice=100"); } [Fact] @@ -642,8 +638,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { controller = "ho%me", action = "li st" }), new RouteValueDictionary(), - "/bl%25og/ho%25me/he%20llo/li%20st", - UrlEncoder.Default); + "/bl%25og/ho%25me/he%20llo/li%20st"); } [Fact] @@ -654,8 +649,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, new RouteValueDictionary(new { controller = "/home", action = "/my/index" }), new RouteValueDictionary(), - "/home/%2Fmy%2Findex", - UrlEncoder.Default); + "/home/%2Fmy%2Findex"); } [Fact] @@ -666,7 +660,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { id = "defaultid" }), new RouteValueDictionary(new { p1 = "v1" }), new RouteValueDictionary(new { p2 = "v2a/v2b" }), - "/UrlEncode[[v1]]/UrlEncode[[v2a/v2b]]"); + "/v1/v2a%2Fv2b"); } [Fact] @@ -677,7 +671,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { id = "defaultid" }), new RouteValueDictionary(new { p1 = "v1" }), new RouteValueDictionary(new { p2 = "" }), - "/UrlEncode[[v1]]"); + "/v1"); } [Fact] @@ -688,7 +682,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests new RouteValueDictionary(new { id = "defaultid" }), new RouteValueDictionary(new { p1 = "v1" }), new RouteValueDictionary(new { p2 = (string)null }), - "/UrlEncode[[v1]]"); + "/v1"); } [Fact] @@ -699,7 +693,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, null, new RouteValueDictionary(new { }), - "/UrlEncode[[foo]]"); + "/foo"); } [Fact] @@ -710,7 +704,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests null, null, new RouteValueDictionary(new { }), - "/UrlEncode[[foo]]"); + "/foo"); } [Fact] @@ -718,10 +712,9 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests { // Arrange var template = "{area?}/{controller=Home}/{action=Index}/{id?}"; - var encoder = new UrlTestEncoder(); var binder = new TemplateBinder( - new UrlTestEncoder(), - new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy(encoder)), + UrlEncoder.Default, + new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy()), TemplateParser.Parse(template), defaults: null); var ambientValues = new RouteValueDictionary(); @@ -1145,15 +1138,12 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests RouteValueDictionary defaults, RouteValueDictionary ambientValues, RouteValueDictionary values, - string expected, - UrlEncoder encoder = null) + string expected) { // Arrange - encoder = encoder ?? new UrlTestEncoder(); - var binder = new TemplateBinder( - encoder, - new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy(encoder)), + UrlEncoder.Default, + new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy()), TemplateParser.Parse(template), defaults); diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouteBuilderTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouteBuilderTest.cs index 97e166c55e..2645e78439 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouteBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouteBuilderTest.cs @@ -244,13 +244,12 @@ namespace Microsoft.AspNetCore.Routing.Tree private static TreeRouteBuilder CreateBuilder() { var objectPoolProvider = new DefaultObjectPoolProvider(); - var objectPolicy = new UriBuilderContextPooledObjectPolicy(UrlEncoder.Default); + var objectPolicy = new UriBuilderContextPooledObjectPolicy(); var objectPool = objectPoolProvider.Create(objectPolicy); var constraintResolver = GetInlineConstraintResolver(); var builder = new TreeRouteBuilder( NullLoggerFactory.Instance, - UrlEncoder.Default, objectPool, constraintResolver); return builder; diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs index 61b5cd8607..8538c10e06 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs @@ -13,7 +13,6 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.Options; -using Microsoft.Extensions.WebEncoders.Testing; using Moq; using Xunit; @@ -23,9 +22,72 @@ namespace Microsoft.AspNetCore.Routing.Tree { private static readonly RequestDelegate NullHandler = (c) => Task.FromResult(0); - private static UrlEncoder Encoder = UrlTestEncoder.Default; private static ObjectPool Pool = new DefaultObjectPoolProvider().Create( - new UriBuilderContextPooledObjectPolicy(Encoder)); + new UriBuilderContextPooledObjectPolicy()); + + [Fact] + public async Task TreeRouter_RouteAsync_MatchesCatchAllRoutesWithDefaults_UsingObsoleteConstructo() + { + // Arrange + var routes = new[] { + "{parameter1=1}/{parameter2=2}/{parameter3=3}/{*parameter4=4}", + }; + var url = "/a/b/c"; + var routeValues = new[] { "a", "b", "c", "4" }; + + var expectedRouteGroup = CreateRouteGroup(0, "{parameter1=1}/{parameter2=2}/{parameter3=3}/{*parameter4=4}"); + var routeValueKeys = new[] { "parameter1", "parameter2", "parameter3", "parameter4" }; + var expectedRouteValues = new RouteValueDictionary(); + for (int i = 0; i < routeValueKeys.Length; i++) + { + expectedRouteValues.Add(routeValueKeys[i], routeValues[i]); + } + + var builder = CreateBuilderUsingObsoleteConstructor(); + + // We setup the route entries in reverse order of precedence to ensure that when we + // try to route the request, the route with a higher precedence gets tried first. + foreach (var template in routes.Reverse()) + { + MapInboundEntry(builder, template); + } + + var route = builder.Build(); + + var context = CreateRouteContext(url); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.Equal(expectedRouteGroup, context.RouteData.Values["test_route_group"]); + foreach (var entry in expectedRouteValues) + { + var data = Assert.Single(context.RouteData.Values, v => v.Key == entry.Key); + Assert.Equal(entry.Value, data.Value); + } + } + + [Fact] + public async Task TreeRouter_RouteAsync_DoesNotMatchRoutesWithIntermediateDefaultRouteValues_UsingObsoleteConstructor() + { + // Arrange + var url = "/a/b"; + + var builder = CreateBuilderUsingObsoleteConstructor(); + + MapInboundEntry(builder, "a/b/{parameter3=3}/d"); + + var route = builder.Build(); + + var context = CreateRouteContext(url); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.Null(context.Handler); + } [Theory] [InlineData("template/5", "template/{parameter:int}")] @@ -1990,15 +2052,31 @@ namespace Microsoft.AspNetCore.Routing.Tree private static TreeRouteBuilder CreateBuilder() { var objectPoolProvider = new DefaultObjectPoolProvider(); - var objectPolicy = new UriBuilderContextPooledObjectPolicy(UrlEncoder.Default); + var objectPolicy = new UriBuilderContextPooledObjectPolicy(); var objectPool = objectPoolProvider.Create(objectPolicy); var constraintResolver = CreateConstraintResolver(); + var builder = new TreeRouteBuilder( + NullLoggerFactory.Instance, + objectPool, + constraintResolver); + return builder; + } + + private static TreeRouteBuilder CreateBuilderUsingObsoleteConstructor() + { + var objectPoolProvider = new DefaultObjectPoolProvider(); + var objectPolicy = new UriBuilderContextPooledObjectPolicy(); + var objectPool = objectPoolProvider.Create(objectPolicy); + + var constraintResolver = CreateConstraintResolver(); +#pragma warning disable CS0618 // Type or member is obsolete var builder = new TreeRouteBuilder( NullLoggerFactory.Instance, UrlEncoder.Default, objectPool, constraintResolver); +#pragma warning restore CS0618 // Type or member is obsolete return builder; }