From 2ab24aa0f4339d4ff98ec2b6db5187c564cb8ed6 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 15 Dec 2015 10:28:45 -0800 Subject: [PATCH] Remove custom url encoding Fixes #214 --- src/Microsoft.AspNet.Routing/RouteBase.cs | 8 +- .../Template/TemplateBinder.cs | 108 +++-- .../RouteCollectionTest.cs | 38 +- .../RouteTest.cs | 27 +- .../Template/TemplateBinderTests.cs | 440 +++++++++--------- .../Tree/TreeRouterTest.cs | 4 +- .../project.json | 1 + 7 files changed, 343 insertions(+), 283 deletions(-) diff --git a/src/Microsoft.AspNet.Routing/RouteBase.cs b/src/Microsoft.AspNet.Routing/RouteBase.cs index ddc53d30d3..0167483b63 100644 --- a/src/Microsoft.AspNet.Routing/RouteBase.cs +++ b/src/Microsoft.AspNet.Routing/RouteBase.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Routing.Internal; @@ -115,7 +116,7 @@ namespace Microsoft.AspNet.Routing /// public virtual VirtualPathData GetVirtualPath(VirtualPathContext context) { - EnsureBinder(); + EnsureBinder(context.HttpContext); EnsureLoggers(context.HttpContext); var values = _binder.GetValues(context.AmbientValues, context.Values); @@ -237,11 +238,12 @@ namespace Microsoft.AspNet.Routing } } - private void EnsureBinder() + private void EnsureBinder(HttpContext context) { if (_binder == null) { - _binder = new TemplateBinder(ParsedTemplate, Defaults); + var urlEncoder = context.RequestServices.GetRequiredService(); + _binder = new TemplateBinder(ParsedTemplate, urlEncoder, Defaults); } } diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs b/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs index 183effd021..58bdde63fe 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs @@ -5,8 +5,9 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; +using System.IO; using System.Text; -using System.Text.RegularExpressions; +using System.Text.Encodings.Web; using Microsoft.AspNet.Http.Extensions; namespace Microsoft.AspNet.Routing.Template @@ -15,15 +16,25 @@ namespace Microsoft.AspNet.Routing.Template { private readonly IReadOnlyDictionary _defaults; private readonly RouteTemplate _template; + private readonly UrlEncoder _urlEncoder; - public TemplateBinder(RouteTemplate template, IReadOnlyDictionary defaults) + public TemplateBinder( + RouteTemplate template, + UrlEncoder urlEncoder, + IReadOnlyDictionary defaults) { if (template == null) { throw new ArgumentNullException(nameof(template)); } + if (urlEncoder == null) + { + throw new ArgumentNullException(nameof(urlEncoder)); + } + _template = template; + _urlEncoder = urlEncoder; _defaults = defaults; } @@ -172,7 +183,7 @@ namespace Microsoft.AspNet.Routing.Template // Step 2: If the route is a match generate the appropriate URI public string BindValues(RouteValueDictionary acceptedValues) { - var context = new UriBuildingContext(); + var context = new UriBuildingContext(_urlEncoder); for (var i = 0; i < _template.Segments.Count; i++) { @@ -250,10 +261,6 @@ namespace Microsoft.AspNet.Routing.Template context.EndSegment(); } - // Encode the URI before we append the query string, otherwise we would double encode the query string - var encoded = new StringBuilder(); - encoded.Append(UriEncode(context.Build())); - // Generate the query string from the remaining values var queryBuilder = new QueryBuilder(); foreach (var kvp in acceptedValues) @@ -265,7 +272,7 @@ namespace Microsoft.AspNet.Routing.Template } var converted = Convert.ToString(kvp.Value, CultureInfo.InvariantCulture); - if (String.IsNullOrEmpty(converted)) + if (string.IsNullOrEmpty(converted)) { continue; } @@ -273,19 +280,9 @@ namespace Microsoft.AspNet.Routing.Template queryBuilder.Add(kvp.Key, converted); } - encoded.Append(queryBuilder.ToString()); - return encoded.ToString(); - } - - private static string UriEncode(string str) - { - var escape = Uri.EscapeUriString(str); - return Regex.Replace(escape, "([#;?:@&=+$,])", EscapeReservedCharacters); - } - - private static string EscapeReservedCharacters(Match m) - { - return "%" + Convert.ToUInt16(m.Value[0]).ToString("x2", CultureInfo.InvariantCulture); + var uri = context.GetUri(); + uri.Append(queryBuilder); + return uri.ToString(); } private TemplatePart GetParameter(string name) @@ -417,16 +414,22 @@ namespace Microsoft.AspNet.Routing.Template private readonly StringBuilder _uri; // Holds the 'optional' parts of the uri. We need a secondary buffer to handle cases where an optional - // segment is in the middle of the uri. We don't know whether or not we need to write it out - if it's + // segment is in the middle of the uri. We don't know if we need to write it out - if it's // followed by other optional segments than we will just throw it away. - private readonly StringBuilder _buffer; + private readonly List _buffer; + private readonly UrlEncoder _urlEncoder; + private readonly StringWriter _uriWriter; private bool _hasEmptySegment; + private int _lastValueOffset; - public UriBuildingContext() + public UriBuildingContext(UrlEncoder urlEncoder) { + _urlEncoder = urlEncoder; _uri = new StringBuilder(); - _buffer = new StringBuilder(); + _buffer = new List(); + _uriWriter = new StringWriter(_uri); + _lastValueOffset = -1; BufferState = SegmentState.Beginning; UriState = SegmentState.Beginning; @@ -457,7 +460,17 @@ namespace Microsoft.AspNet.Routing.Template return false; } - _uri.Append(_buffer); + for (var i = 0; i < _buffer.Count; i++) + { + if (_buffer[i].RequiresEncoding) + { + _urlEncoder.Encode(_uriWriter, _buffer[i].Value); + } + else + { + _uri.Append(_buffer[i].Value); + } + } _buffer.Clear(); if (UriState == SegmentState.Beginning && BufferState == SegmentState.Beginning) @@ -471,13 +484,27 @@ namespace Microsoft.AspNet.Routing.Template BufferState = SegmentState.Inside; UriState = SegmentState.Inside; - _uri.Append(value); + _lastValueOffset = _uri.Length; + // Allow the first segment to have a leading slash. + // This prevents the leading slash from PathString segments from being encoded. + if (_uri.Length == 0 && value.Length > 0 && value[0] == '/') + { + _uri.Append("/"); + _urlEncoder.Encode(_uriWriter, value, 1, value.Length - 1); + } + else + { + _urlEncoder.Encode(_uriWriter, value); + } + return true; } public void Remove(string literal) { - _uri.Length -= literal.Length; + Debug.Assert(_lastValueOffset != -1, "Cannot invoke Remove more than once."); + _uri.Length = _lastValueOffset; + _lastValueOffset = -1; } public bool Buffer(string value) @@ -516,15 +543,15 @@ namespace Microsoft.AspNet.Routing.Template if (UriState == SegmentState.Beginning && BufferState == SegmentState.Beginning) { - if (_uri.Length != 0 || _buffer.Length != 0) + if (_uri.Length != 0 || _buffer.Count != 0) { - _buffer.Append("/"); + _buffer.Add(new BufferValue("/", requiresEncoding: false)); } BufferState = SegmentState.Inside; } - _buffer.Append(value); + _buffer.Add(new BufferValue(value, requiresEncoding: true)); return true; } @@ -534,15 +561,15 @@ namespace Microsoft.AspNet.Routing.Template UriState = SegmentState.Beginning; } - internal string Build() + internal StringBuilder GetUri() { // We can ignore any currently buffered segments - they are are guaranteed to be 'defaults'. - return _uri.ToString(); + return _uri; } private string DebuggerToString() { - return string.Format("{{Accepted: '{0}' Buffered: '{1}'}}", _uri.ToString(), _buffer.ToString()); + return string.Format("{{Accepted: '{0}' Buffered: '{1}'}}", _uri, string.Join("", _buffer)); } } @@ -557,5 +584,18 @@ namespace Microsoft.AspNet.Routing.Template Beginning, Inside, } + + private struct BufferValue + { + public BufferValue(string value, bool requiresEncoding) + { + Value = value; + RequiresEncoding = requiresEncoding; + } + + public bool RequiresEncoding { get; } + + public string Value { get; } + } } } diff --git a/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs b/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs index 5ce6300173..a906462f34 100644 --- a/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs @@ -4,11 +4,13 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNet.Http; -using Microsoft.AspNet.Routing.Template; -using Microsoft.Extensions.Logging.Testing; +using Microsoft.AspNet.Http.Internal; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; using Microsoft.Extensions.OptionsModel; using Moq; using Xunit; @@ -345,8 +347,7 @@ namespace Microsoft.AspNet.Routing string template, RouteValueDictionary values, string expectedUrl, - bool lowercaseUrls - ) + bool lowercaseUrls) { // Arrange var routeCollection = new RouteCollection(); @@ -530,17 +531,19 @@ namespace Microsoft.AspNet.Routing options = new RouteOptions(); } - var request = new Mock(MockBehavior.Strict); var optionsAccessor = new Mock>(MockBehavior.Strict); optionsAccessor.SetupGet(o => o.Value).Returns(options); + var serviceProvider = new ServiceCollection() + .AddSingleton(loggerFactory) + .AddSingleton(UrlEncoder.Default) + .AddSingleton(optionsAccessor.Object) + .BuildServiceProvider(); + var context = new Mock(MockBehavior.Strict); - context.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) - .Returns(loggerFactory); - context.Setup(m => m.RequestServices.GetService(typeof(IOptions))) - .Returns(optionsAccessor.Object); + context.SetupGet(m => m.RequestServices).Returns(serviceProvider); context.SetupGet(c => c.Request).Returns(request.Object); return new VirtualPathContext(context.Object, null, null, routeName); @@ -554,14 +557,19 @@ namespace Microsoft.AspNet.Routing var optionsAccessor = new Mock>(MockBehavior.Strict); optionsAccessor.SetupGet(o => o.Value).Returns(options); - var context = new Mock(MockBehavior.Strict); - context.Setup(m => m.RequestServices.GetService(typeof(IOptions))) - .Returns(optionsAccessor.Object); - context.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) - .Returns(NullLoggerFactory.Instance); + var serviceProvider = new ServiceCollection() + .AddSingleton(NullLoggerFactory.Instance) + .AddSingleton(UrlEncoder.Default) + .AddSingleton(optionsAccessor.Object) + .BuildServiceProvider(); + + var context = new DefaultHttpContext + { + RequestServices = serviceProvider + }; return new VirtualPathContext( - context.Object, + context, ambientValues: null, values: values, routeName: routeName); diff --git a/test/Microsoft.AspNet.Routing.Tests/RouteTest.cs b/test/Microsoft.AspNet.Routing.Tests/RouteTest.cs index 13b21d78c8..19892783cc 100644 --- a/test/Microsoft.AspNet.Routing.Tests/RouteTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/RouteTest.cs @@ -4,9 +4,11 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Routing.Constraints; using Microsoft.AspNet.Testing; using Microsoft.Extensions.DependencyInjection; @@ -373,7 +375,7 @@ namespace Microsoft.AspNet.Routing Assert.Null(context.Handler); } -#region Route Matching + #region Route Matching // PathString in HttpAbstractions guarantees a leading slash - so no value in testing other cases. [Fact] @@ -595,9 +597,9 @@ namespace Microsoft.AspNet.Routing return new RouteContext(context.Object); } -#endregion + #endregion -#region Route Binding + #region Route Binding [Fact] public void GetVirtualPath_Success() @@ -1296,11 +1298,16 @@ namespace Microsoft.AspNet.Routing RouteValueDictionary values, RouteValueDictionary ambientValues) { - var context = new Mock(MockBehavior.Strict); - context.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) - .Returns(NullLoggerFactory.Instance); + var serviceProvider = new ServiceCollection() + .AddSingleton(NullLoggerFactory.Instance) + .AddSingleton(UrlEncoder.Default) + .BuildServiceProvider(); + var context = new DefaultHttpContext + { + RequestServices = serviceProvider + }; - return new VirtualPathContext(context.Object, ambientValues, values); + return new VirtualPathContext(context, ambientValues, values); } private static VirtualPathContext CreateVirtualPathContext(string routeName) @@ -1308,9 +1315,9 @@ namespace Microsoft.AspNet.Routing return new VirtualPathContext(null, null, null, routeName); } -#endregion + #endregion -#region Route Registration + #region Route Registration public static IEnumerable DataTokens { @@ -1476,7 +1483,7 @@ namespace Microsoft.AspNet.Routing Assert.Equal("RouteName", name); } -#endregion + #endregion // DataTokens test data for TemplateRoute.GetVirtualPath public static IEnumerable DataTokensTestData diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs index b97b307800..b3556825d5 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateBinderTests.cs @@ -4,8 +4,10 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.Encodings.Web; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.OptionsModel; +using Microsoft.Extensions.WebEncoders.Testing; using Xunit; namespace Microsoft.AspNet.Routing.Template.Tests @@ -14,116 +16,97 @@ namespace Microsoft.AspNet.Routing.Template.Tests { private readonly IInlineConstraintResolver _inlineConstraintResolver = GetInlineConstraintResolver(); - public static IEnumerable EmptyAndNullDefaultValues - { - get + public static TheoryData EmptyAndNullDefaultValues => + new TheoryData { - return new object[][] { - new object[] - { - "Test/{val1}/{val2}", - new RouteValueDictionary(new {val1 = "", val2 = ""}), - new RouteValueDictionary(new {val2 = "SomeVal2"}), - null, - }, - new object[] - { - "Test/{val1}/{val2}", - new RouteValueDictionary(new {val1 = "", val2 = ""}), - new RouteValueDictionary(new {val1 = "a"}), - "Test/a" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}", - new RouteValueDictionary(new {val1 = "", val3 = ""}), - new RouteValueDictionary(new {val2 = "a"}), - null - }, - new object[] - { - "Test/{val1}/{val2}", - new RouteValueDictionary(new {val1 = "", val2 = ""}), - new RouteValueDictionary(new {val1 = "a", val2 = "b"}), - "Test/a/b" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}", - new RouteValueDictionary(new {val1 = "", val2 = "", val3 = ""}), - new RouteValueDictionary(new {val1 = "a", val2 = "b", val3 = "c"}), - "Test/a/b/c" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}", - new RouteValueDictionary(new {val1 = "", val2 = "", val3 = ""}), - new RouteValueDictionary(new {val1 = "a", val2 = "b"}), - "Test/a/b" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}", - new RouteValueDictionary(new {val1 = "", val2 = "", val3 = ""}), - new RouteValueDictionary(new {val1 = "a"}), - "Test/a" - }, - new object[] - { - "Test/{val1}", - new RouteValueDictionary(new {val1 = "42", val2 = "", val3 = ""}), - new RouteValueDictionary(), - "Test" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}", - new RouteValueDictionary(new {val1 = "42", val2 = (string)null, val3 = (string)null}), - new RouteValueDictionary(), - "Test" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}/{val4}", - new RouteValueDictionary(new {val1 = "21", val2 = "", val3 = "", val4 = ""}), - new RouteValueDictionary(new {val1 = "42", val2 = "11", val3 = "", val4 = ""}), - "Test/42/11" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}", - new RouteValueDictionary(new {val1 = "21", val2 = "", val3 = ""}), - new RouteValueDictionary(new {val1 = "42"}), - "Test/42" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}/{val4}", - new RouteValueDictionary(new {val1 = "21", val2 = "", val3 = "", val4 = ""}), - new RouteValueDictionary(new {val1 = "42", val2 = "11"}), - "Test/42/11" - }, - new object[] - { - "Test/{val1}/{val2}/{val3}", - new RouteValueDictionary(new {val1 = "21", val2 = (string)null, val3 = (string)null}), - new RouteValueDictionary(new {val1 = "42"}), - "Test/42" - }, - new object[] - { - "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"}), - "Test/42/11" - }, - }; - } - } + "Test/{val1}/{val2}", + new RouteValueDictionary(new {val1 = "", val2 = ""}), + new RouteValueDictionary(new {val2 = "SomeVal2"}), + null + }, + { + "Test/{val1}/{val2}", + new RouteValueDictionary(new {val1 = "", val2 = ""}), + new RouteValueDictionary(new {val1 = "a"}), + "UrlEncode[[Test]]/UrlEncode[[a]]" + }, + { + "Test/{val1}/{val2}/{val3}", + new RouteValueDictionary(new {val1 = "", val3 = ""}), + new RouteValueDictionary(new {val2 = "a"}), + null + }, + { + "Test/{val1}/{val2}", + new RouteValueDictionary(new {val1 = "", val2 = ""}), + new RouteValueDictionary(new {val1 = "a", val2 = "b"}), + "UrlEncode[[Test]]/UrlEncode[[a]]/UrlEncode[[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/{val1}/{val2}/{val3}", + new RouteValueDictionary(new {val1 = "", val2 = "", val3 = ""}), + new RouteValueDictionary(new {val1 = "a", val2 = "b"}), + "UrlEncode[[Test]]/UrlEncode[[a]]/UrlEncode[[b]]" + }, + { + "Test/{val1}/{val2}/{val3}", + new RouteValueDictionary(new {val1 = "", val2 = "", val3 = ""}), + new RouteValueDictionary(new {val1 = "a"}), + "UrlEncode[[Test]]/UrlEncode[[a]]" + }, + { + "Test/{val1}", + new RouteValueDictionary(new {val1 = "42", val2 = "", val3 = ""}), + new RouteValueDictionary(), + "UrlEncode[[Test]]" + }, + { + "Test/{val1}/{val2}/{val3}", + new RouteValueDictionary(new {val1 = "42", val2 = (string)null, val3 = (string)null}), + new RouteValueDictionary(), + "UrlEncode[[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/{val1}/{val2}/{val3}", + new RouteValueDictionary(new {val1 = "21", val2 = "", val3 = ""}), + new RouteValueDictionary(new {val1 = "42"}), + "UrlEncode[[Test]]/UrlEncode[[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/{val1}/{val2}/{val3}", + new RouteValueDictionary(new {val1 = "21", val2 = (string)null, val3 = (string)null}), + new RouteValueDictionary(new {val1 = "42"}), + "UrlEncode[[Test]]/UrlEncode[[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]]" + }, + }; [Theory] - [MemberData("EmptyAndNullDefaultValues")] + [MemberData(nameof(EmptyAndNullDefaultValues))] public void Binding_WithEmptyAndNull_DefaultValues( string template, IReadOnlyDictionary defaults, @@ -133,6 +116,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests // Arrange var binder = new TemplateBinder( TemplateParser.Parse(template), + new UrlTestEncoder(), defaults); // Act & Assert @@ -169,7 +153,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "xx", region = "yy" }), - "language/xx-yy"); + "UrlEncode[[language]]/UrlEncode[[xx]]UrlEncode[[-]]UrlEncode[[yy]]"); } [Fact] @@ -180,7 +164,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "xx", region = "yy" }), - "language/xx-yya"); + "UrlEncode[[language]]/UrlEncode[[xx]]UrlEncode[[-]]UrlEncode[[yy]]UrlEncode[[a]]"); } [Fact] @@ -191,96 +175,84 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "xx", region = "yy" }), - "language/axx-yy"); + "UrlEncode[[language]]/UrlEncode[[a]]UrlEncode[[xx]]UrlEncode[[-]]UrlEncode[[yy]]"); } - public static IEnumerable OptionalParamValues - { - get + public static TheoryData OptionalParamValues => + new TheoryData { - return new object[][] + // defaults + // ambient values + // values { - // defaults - // ambient values - // values - new object[] - { - "Test/{val1}/{val2}.{val3?}", - new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), - new RouteValueDictionary(new {val3 = "someval3"}), - new RouteValueDictionary(new {val3 = "someval3"}), - "Test/someval1/someval2.someval3", - }, - new object[] - { - "Test/{val1}/{val2}.{val3?}", - new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), - new RouteValueDictionary(new {val3 = "someval3a"}), - new RouteValueDictionary(new {val3 = "someval3v"}), - "Test/someval1/someval2.someval3v", - }, - new object[] - { - "Test/{val1}/{val2}.{val3?}", - new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), - new RouteValueDictionary(new {val3 = "someval3a"}), - new RouteValueDictionary(), - "Test/someval1/someval2.someval3a", - }, - new object[] - { - "Test/{val1}/{val2}.{val3?}", - new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), - new RouteValueDictionary(), - new RouteValueDictionary(new {val3 = "someval3v"}), - "Test/someval1/someval2.someval3v", - }, - new object[] - { - "Test/{val1}/{val2}.{val3?}", - new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), - new RouteValueDictionary(), - new RouteValueDictionary(), - "Test/someval1/someval2", - }, - new object[] - { - "Test/{val1}.{val2}.{val3}.{val4?}", - new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), - new RouteValueDictionary(), - new RouteValueDictionary(new {val4 = "someval4", val3 = "someval3" }), - "Test/someval1.someval2.someval3.someval4", - }, - new object[] - { - "Test/{val1}.{val2}.{val3}.{val4?}", - new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), - new RouteValueDictionary(), - new RouteValueDictionary(new {val3 = "someval3" }), - "Test/someval1.someval2.someval3", - }, - new object[] - { - "Test/.{val2?}", - new RouteValueDictionary(new { }), - new RouteValueDictionary(), - new RouteValueDictionary(new {val2 = "someval2" }), - "Test/.someval2", - }, - new object[] - { - "Test/{val1}.{val2}", - new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2" }), - new RouteValueDictionary(), - new RouteValueDictionary(new {val3 = "someval3" }), - "Test/someval1.someval2?val3=someval3", - }, - }; - } - } + "Test/{val1}/{val2}.{val3?}", + 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/{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/{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/{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/{val1}/{val2}.{val3?}", + new RouteValueDictionary(new {val1 = "someval1", val2 = "someval2"}), + new RouteValueDictionary(), + new RouteValueDictionary(), + "UrlEncode[[Test]]/UrlEncode[[someval1]]/UrlEncode[[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/{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/.{val2?}", + new RouteValueDictionary(new { }), + new RouteValueDictionary(), + new RouteValueDictionary(new {val2 = "someval2" }), + "UrlEncode[[Test]]/UrlEncode[[.]]UrlEncode[[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]]?val3=someval3" + }, + }; [Theory] - [MemberData("OptionalParamValues")] + [MemberData(nameof(OptionalParamValues))] public void GetVirtualPathWithMultiSegmentWithOptionalParam( string template, IReadOnlyDictionary defaults, @@ -291,6 +263,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests // Arrange var binder = new TemplateBinder( TemplateParser.Parse(template), + new UrlTestEncoder(), defaults); // Act & Assert @@ -318,7 +291,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests Assert.Equal(expected, boundTemplate); } } - + [Fact] public void GetVirtualPathWithMultiSegmentParamsOnNeitherEndMatches() { @@ -327,7 +300,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "xx", region = "yy" }), - "language/axx-yya"); + "UrlEncode[[language]]/UrlEncode[[a]]UrlEncode[[xx]]UrlEncode[[-]]UrlEncode[[yy]]UrlEncode[[a]]"); } [Fact] @@ -360,7 +333,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en" }), new RouteValueDictionary(new { lang = "xx" }), - "language/xx"); + "UrlEncode[[language]]/UrlEncode[[xx]]"); } [Fact] @@ -371,7 +344,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en" }), new RouteValueDictionary(new { lang = "xx" }), - "language/xx-"); + "UrlEncode[[language]]/UrlEncode[[xx]]UrlEncode[[-]]"); } [Fact] @@ -382,7 +355,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en" }), new RouteValueDictionary(new { lang = "xx" }), - "language/axx"); + "UrlEncode[[language]]/UrlEncode[[a]]UrlEncode[[xx]]"); } [Fact] @@ -393,7 +366,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { lang = "en" }), new RouteValueDictionary(new { lang = "xx" }), - "language/axxa"); + "UrlEncode[[language]]/UrlEncode[[a]]UrlEncode[[xx]]UrlEncode[[a]]"); } [Fact] @@ -404,7 +377,7 @@ namespace Microsoft.AspNet.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" }), - "products.mvc"); + "UrlEncode[[products]]UrlEncode[[.mvc]]"); } [Fact] @@ -415,7 +388,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { lang = "xx", region = "yy" }), new RouteValueDictionary(new { lang = "en", region = "US" }), new RouteValueDictionary(new { lang = "zz" }), - "language/zz-yy"); + "UrlEncode[[language]]/UrlEncode[[zz]]UrlEncode[[-]]UrlEncode[[yy]]"); } [Fact] @@ -427,7 +400,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { id = "defaultid" }), new RouteValueDictionary(new { controller = "home", action = "oldaction" }), new RouteValueDictionary(new { action = "newaction" }), - "home/newaction"); + "UrlEncode[[home]]/UrlEncode[[newaction]]"); } [Fact] @@ -460,7 +433,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { }), new RouteValueDictionary(new { controller = "home" }), - "foo/home"); + "UrlEncode[[foo]]/UrlEncode[[home]]"); } [Fact] @@ -472,7 +445,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { id = (string)null }), new RouteValueDictionary(new { controller = "home", action = "oldaction", id = (string)null }), new RouteValueDictionary(new { action = "newaction" }), - "home/newaction"); + "UrlEncode[[home]]/UrlEncode[[newaction]]"); } [Fact] @@ -483,7 +456,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { language = "en", locale = "US" }), new RouteValueDictionary(), new RouteValueDictionary(new { controller = "Orders" }), - "Orders/en-US"); + "UrlEncode[[Orders]]/UrlEncode[[en]]UrlEncode[[-]]UrlEncode[[US]]"); } [Fact] @@ -494,7 +467,7 @@ namespace Microsoft.AspNet.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 }), - "Home.mvc/TestAction/1"); + "UrlEncode[[Home]]UrlEncode[[.mvc]]/UrlEncode[[TestAction]]/UrlEncode[[1]]"); } [Fact] @@ -527,7 +500,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new { p2 = "d2", p3 = "d3" }, new { p1 = "v1", }, new { p2 = "", p3 = "" }, - "v1"); + "UrlEncode[[v1]]"); } [Fact] @@ -538,7 +511,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new { action = "Index", id = (string)null }, new { controller = "orig", action = "init", id = "123" }, new { action = "new", }, - "orig/new"); + "UrlEncode[[orig]]/UrlEncode[[new]]"); } [Fact] @@ -549,7 +522,8 @@ namespace Microsoft.AspNet.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" }, - "UrlGeneration1/UrlRouting.mvc/Play/Photos/1995/Hola"); + "UrlEncode[[UrlGeneration1]]/UrlEncode[[UrlRouting]]UrlEncode[[.mvc]]/UrlEncode[[Play]]/" + + "UrlEncode[[Photos]]/UrlEncode[[1995]]/UrlEncode[[Hola]]"); } [Fact] @@ -572,7 +546,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { year = 1995, occasion = "Christmas", action = "Play", SafeParam = "SafeParamValue" }), ambientValues, values, - "UrlGeneration1/UrlRouting.mvc/Play/Photos/1995/Hola"); + "UrlEncode[[UrlGeneration1]]/UrlEncode[[UrlRouting]]UrlEncode[[.mvc]]/" + + "UrlEncode[[Play]]/UrlEncode[[Photos]]/UrlEncode[[1995]]/UrlEncode[[Hola]]"); } [Fact] @@ -594,7 +569,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { action = "Default" }), ambientValues, values, - "subtest.mvc/Default/b"); + "UrlEncode[[subtest]]UrlEncode[[.mvc]]/UrlEncode[[Default]]/UrlEncode[[b]]"); } [Fact] @@ -612,7 +587,8 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { controller = "Home" }), new RouteValueDictionary(new { controller = "home", action = "Index", id = (string)null }), values, - "%23%3b%3f%3a%40%26%3d%2b%24%2c.mvc/showcategory/123?so%3Frt=de%3Fsc&maxPrice=100"); + "%23;%3F%3A@%26%3D%2B$,.mvc/showcategory/123?so%3Frt=de%3Fsc&maxPrice=100", + UrlEncoder.Default); } [Fact] @@ -626,7 +602,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { controller = "Home" }), new RouteValueDictionary(new { controller = "home", action = "Index", id = (string)null }), values, - "products.mvc/showcategory/123?so%3Frt=de%3Fsc&maxPrice=100"); + "UrlEncode[[products]]UrlEncode[[.mvc]]/UrlEncode[[showcategory]]/UrlEncode[[123]]?so%3Frt=de%3Fsc&maxPrice=100"); } [Fact] @@ -636,8 +612,17 @@ namespace Microsoft.AspNet.Routing.Template.Tests "{controller}.mvc/{action}/{id}", new RouteValueDictionary(new { controller = "Home", Custom = "customValue" }), new RouteValueDictionary(new { controller = "Home", action = "Index", id = (string)null }), - new RouteValueDictionary(new { controller = "products", action = "showcategory", id = 123, sort = "desc", maxPrice = 100, custom = "customValue" }), - "products.mvc/showcategory/123?sort=desc&maxPrice=100"); + new RouteValueDictionary( + new + { + controller = "products", + action = "showcategory", + id = 123, + sort = "desc", + maxPrice = 100, + custom = "customValue" + }), + "UrlEncode[[products]]UrlEncode[[.mvc]]/UrlEncode[[showcategory]]/UrlEncode[[123]]?sort=desc&maxPrice=100"); } [Fact] @@ -648,7 +633,20 @@ namespace Microsoft.AspNet.Routing.Template.Tests null, new RouteValueDictionary(new { controller = "ho%me", action = "li st" }), new RouteValueDictionary(), - "bl%25og/ho%25me/he%20llo/li%20st"); + "bl%25og/ho%25me/he%20llo/li%20st", + UrlEncoder.Default); + } + + [Fact] + public void GetVirtualDoesNotEncodeLeadingSlashes() + { + RunTest( + "{controller}/{action}", + null, + new RouteValueDictionary(new { controller = "/home", action = "/my/index" }), + new RouteValueDictionary(), + "/home/%2Fmy%2Findex", + UrlEncoder.Default); } [Fact] @@ -659,7 +657,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { id = "defaultid" }), new RouteValueDictionary(new { p1 = "v1" }), new RouteValueDictionary(new { p2 = "v2a/v2b" }), - "v1/v2a/v2b"); + "UrlEncode[[v1]]/UrlEncode[[v2a/v2b]]"); } [Fact] @@ -670,7 +668,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { id = "defaultid" }), new RouteValueDictionary(new { p1 = "v1" }), new RouteValueDictionary(new { p2 = "" }), - "v1"); + "UrlEncode[[v1]]"); } [Fact] @@ -681,7 +679,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests new RouteValueDictionary(new { id = "defaultid" }), new RouteValueDictionary(new { p1 = "v1" }), new RouteValueDictionary(new { p2 = (string)null }), - "v1"); + "UrlEncode[[v1]]"); } #if ROUTE_COLLECTION @@ -1088,10 +1086,12 @@ namespace Microsoft.AspNet.Routing.Template.Tests IReadOnlyDictionary defaults, RouteValueDictionary ambientValues, RouteValueDictionary values, - string expected) + string expected, + UrlEncoder encoder = null) { // Arrange - var binder = new TemplateBinder(TemplateParser.Parse(template), defaults); + encoder = encoder ?? new UrlTestEncoder(); + var binder = new TemplateBinder(TemplateParser.Parse(template), encoder, defaults); // Act & Assert var result = binder.GetValues(ambientValues, values); diff --git a/test/Microsoft.AspNet.Routing.Tests/Tree/TreeRouterTest.cs b/test/Microsoft.AspNet.Routing.Tests/Tree/TreeRouterTest.cs index cdef62983e..ec17c5e133 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Tree/TreeRouterTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Tree/TreeRouterTest.cs @@ -4,12 +4,14 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Routing.Template; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; using Microsoft.Extensions.OptionsModel; +using Microsoft.Extensions.WebEncoders.Testing; using Moq; using Xunit; @@ -1594,7 +1596,7 @@ namespace Microsoft.AspNet.Routing.Tree entry.Constraints = constraints; entry.Defaults = defaults; - entry.Binder = new TemplateBinder(entry.Template, defaults); + entry.Binder = new TemplateBinder(entry.Template, UrlEncoder.Default, defaults); entry.Order = order; entry.GenerationPrecedence = RoutePrecedence.ComputeGenerated(entry.Template); entry.RequiredLinkValues = new RouteValueDictionary(requiredValues); diff --git a/test/Microsoft.AspNet.Routing.Tests/project.json b/test/Microsoft.AspNet.Routing.Tests/project.json index f5c02dce72..e57fde09ec 100644 --- a/test/Microsoft.AspNet.Routing.Tests/project.json +++ b/test/Microsoft.AspNet.Routing.Tests/project.json @@ -8,6 +8,7 @@ "Microsoft.AspNet.Testing": "1.0.0-*", "Microsoft.Extensions.DependencyInjection": "1.0.0-*", "Microsoft.Extensions.Logging.Testing": "1.0.0-*", + "Microsoft.Extensions.WebEncoders": "1.0.0-*", "xunit.runner.aspnet": "2.0.0-aspnet-*" }, "frameworks": {