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