Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Convert RouteValueDictionary values to string using `CultureInfo.…
Browse files Browse the repository at this point in the history
…InvariantCulture` (#8674)

* Convert `RouteValueDictionary` values to `string` using `CultureInfo.InvariantCulture`
- #8578
- user may override this choice in one case:
  - register a custom `IValueProviderFactory` to pass another `CultureInfo` into the `RouteValueProvider`
- values are used as programmatic tokens outside `RouteValueProvider`

nits:
- take VS suggestions in changed classes
- take VS suggestions in files I had open :)
  • Loading branch information
dougbu authored Oct 31, 2018
1 parent 734b919 commit c74a945
Show file tree
Hide file tree
Showing 29 changed files with 550 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Threading.Tasks;
using BenchmarkDotNet.Attributes;
Expand Down Expand Up @@ -123,8 +124,8 @@ private static IReadOnlyList<ActionDescriptor> NaiveSelectCandidates(ActionDescr
var isMatch = true;
foreach (var kvp in action.RouteValues)
{
var routeValue = Convert.ToString(routeValues[kvp.Key]) ?? string.Empty;

var routeValue = Convert.ToString(routeValues[kvp.Key], CultureInfo.InvariantCulture) ??
string.Empty;
if (string.IsNullOrEmpty(kvp.Value) && string.IsNullOrEmpty(routeValue))
{
// Match
Expand Down Expand Up @@ -156,7 +157,7 @@ private static ActionDescriptor CreateActionDescriptor(object obj)
var routeValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
foreach (var kvp in new RouteValueDictionary(obj))
{
routeValues.Add(kvp.Key, Convert.ToString(kvp.Value) ?? string.Empty);
routeValues.Add(kvp.Key, Convert.ToString(kvp.Value, CultureInfo.InvariantCulture) ?? string.Empty);
}

return new ActionDescriptor()
Expand Down
6 changes: 3 additions & 3 deletions src/Microsoft.AspNetCore.Mvc.Core/Formatters/FormatFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Globalization;
using System.Linq;
using Microsoft.AspNetCore.Mvc.ApiExplorer;
using Microsoft.AspNetCore.Mvc.Filters;
Expand Down Expand Up @@ -59,7 +60,7 @@ public virtual string GetFormat(ActionContext context)
if (context.RouteData.Values.TryGetValue("format", out var obj))
{
// null and string.Empty are equivalent for route values.
var routeValue = obj?.ToString();
var routeValue = Convert.ToString(obj, CultureInfo.InvariantCulture);
return string.IsNullOrEmpty(routeValue) ? null : routeValue;
}

Expand Down Expand Up @@ -166,8 +167,7 @@ public void OnResultExecuting(ResultExecutingContext context)
return;
}

var objectResult = context.Result as ObjectResult;
if (objectResult == null)
if (!(context.Result is ObjectResult objectResult))
{
return;
}
Expand Down
18 changes: 10 additions & 8 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Threading;
using Microsoft.AspNetCore.Mvc.Abstractions;
Expand Down Expand Up @@ -81,11 +82,10 @@ public IReadOnlyList<ActionDescriptor> SelectCandidates(RouteContext context)
var values = new string[keys.Length];
for (var i = 0; i < keys.Length; i++)
{
context.RouteData.Values.TryGetValue(keys[i], out object value);

context.RouteData.Values.TryGetValue(keys[i], out var value);
if (value != null)
{
values[i] = value as string ?? Convert.ToString(value);
values[i] = value as string ?? Convert.ToString(value, CultureInfo.InvariantCulture);
}
}

Expand Down Expand Up @@ -220,9 +220,11 @@ private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraintsCore(
var actionsWithConstraint = new List<ActionSelectorCandidate>();
var actionsWithoutConstraint = new List<ActionSelectorCandidate>();

var constraintContext = new ActionConstraintContext();
constraintContext.Candidates = candidates;
constraintContext.RouteContext = context;
var constraintContext = new ActionConstraintContext
{
Candidates = candidates,
RouteContext = context
};

// Perf: Avoid allocations
for (var i = 0; i < candidates.Count; i++)
Expand Down Expand Up @@ -294,7 +296,7 @@ private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraintsCore(
// canonical entries. When you don't hit a case-sensitive match it will try the case-insensitive dictionary
// so you still get correct behaviors.
//
// The difference here is because while MVC is case-insensitive, doing a case-sensitive comparison is much
// The difference here is because while MVC is case-insensitive, doing a case-sensitive comparison is much
// faster. We also expect that most of the URLs we process are canonically-cased because they were generated
// by Url.Action or another routing api.
//
Expand All @@ -316,7 +318,7 @@ public Cache(ActionDescriptorCollection actions)
OrdinalEntries = new Dictionary<string[], List<ActionDescriptor>>(StringArrayComparer.Ordinal);
OrdinalIgnoreCaseEntries = new Dictionary<string[], List<ActionDescriptor>>(StringArrayComparer.OrdinalIgnoreCase);

// We need to first identify of the keys that action selection will look at (in route data).
// We need to first identify of the keys that action selection will look at (in route data).
// We want to only consider conventionally routed actions here.
var routeKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
for (var i = 0; i < actions.Items.Count; i++)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Globalization;

namespace Microsoft.AspNetCore.Mvc.Internal
{
Expand Down Expand Up @@ -45,7 +46,7 @@ public static string GetNormalizedRouteValue(ActionContext context, string key)
normalizedValue = value;
}

var stringRouteValue = routeValue?.ToString();
var stringRouteValue = Convert.ToString(routeValue, CultureInfo.InvariantCulture);
if (string.Equals(normalizedValue, stringRouteValue, StringComparison.OrdinalIgnoreCase))
{
return normalizedValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,9 @@ public override ValueProviderResult GetValue(string key)
throw new ArgumentNullException(nameof(key));
}

object value;
if (_values.TryGetValue(key, out value))
if (_values.TryGetValue(key, out var value))
{
var stringValue = value as string ?? value?.ToString() ?? string.Empty;
var stringValue = value as string ?? Convert.ToString(value, Culture) ?? string.Empty;
return new ValueProviderResult(stringValue, Culture);
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Core;
Expand Down Expand Up @@ -51,10 +52,9 @@ public bool Match(
throw new ArgumentNullException(nameof(values));
}

object obj;
if (values.TryGetValue(routeKey, out obj))
if (values.TryGetValue(routeKey, out var obj))
{
var value = obj as string;
var value = Convert.ToString(obj, CultureInfo.InvariantCulture);
if (value != null)
{
var actionDescriptors = GetAndValidateActionDescriptors(httpContext);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// 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 Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Mvc.Routing;
using System;

namespace Microsoft.AspNetCore.Routing
{
Expand Down Expand Up @@ -104,7 +104,7 @@ public static string GetPathByPage(
}

var address = CreateAddress(httpContext: null, page, handler, values);
return generator.GetPathByAddress<RouteValuesAddress>(address, address.ExplicitValues, pathBase, fragment, options);
return generator.GetPathByAddress(address, address.ExplicitValues, pathBase, fragment, options);
}

/// <summary>
Expand Down Expand Up @@ -230,4 +230,4 @@ private static RouteValueDictionary GetAmbientValues(HttpContext httpContext)
return httpContext?.Features.Get<IRouteValuesFeature>()?.RouteValues;
}
}
}
}
10 changes: 4 additions & 6 deletions src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Text;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Core;
using Microsoft.AspNetCore.Mvc.Internal;
using Microsoft.AspNetCore.Routing;
Expand Down Expand Up @@ -163,8 +163,7 @@ protected string GenerateUrl(string protocol, string host, string virtualPath, s
// Perf: In most of the common cases, GenerateUrl is called with a null protocol, host and fragment.
// In such cases, we might not need to build any URL as the url generated is mostly same as the virtual path available in pathData.
// For such common cases, this FastGenerateUrl method saves a string allocation per GenerateUrl call.
string url;
if (TryFastGenerateUrl(protocol, host, virtualPath, fragment, out url))
if (TryFastGenerateUrl(protocol, host, virtualPath, fragment, out var url))
{
return url;
}
Expand Down Expand Up @@ -227,8 +226,7 @@ protected string GenerateUrl(string protocol, string host, string path)
// Perf: In most of the common cases, GenerateUrl is called with a null protocol, host and fragment.
// In such cases, we might not need to build any URL as the url generated is mostly same as the virtual path available in pathData.
// For such common cases, this FastGenerateUrl method saves a string allocation per GenerateUrl call.
string url;
if (TryFastGenerateUrl(protocol, host, path, fragment: null, out url))
if (TryFastGenerateUrl(protocol, host, path, fragment: null, out var url))
{
return url;
}
Expand Down Expand Up @@ -351,7 +349,7 @@ private static object CalculatePageName(ActionContext context, RouteValueDiction
}
else if (ambientValues != null)
{
currentPagePath = ambientValues["page"]?.ToString();
currentPagePath = Convert.ToString(ambientValues["page"], CultureInfo.InvariantCulture);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.RazorPages;
using Microsoft.AspNetCore.Routing;

namespace Microsoft.AspNetCore.Mvc.ApplicationModels
Expand Down Expand Up @@ -96,7 +95,7 @@ public PageRouteModel(PageRouteModel other)
public IList<SelectorModel> Selectors { get; }

/// <summary>
/// Gets a collection of route values that must be present in the <see cref="RouteData.Values"/>
/// Gets a collection of route values that must be present in the <see cref="RouteData.Values"/>
/// for the corresponding page to be selected.
/// </summary>
/// <remarks>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;

namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
{
Expand Down Expand Up @@ -57,8 +57,10 @@ public HandlerMethodDescriptor Select(PageContext context)

if (ambiguousMatches == null)
{
ambiguousMatches = new List<HandlerMethodDescriptor>();
ambiguousMatches.Add(bestMatch);
ambiguousMatches = new List<HandlerMethodDescriptor>
{
bestMatch
};
}

ambiguousMatches.Add(handler);
Expand Down Expand Up @@ -165,13 +167,13 @@ private static int GetScore(HandlerMethodDescriptor descriptor)

private static string GetHandlerName(PageContext context)
{
var handlerName = Convert.ToString(context.RouteData.Values[Handler]);
var handlerName = Convert.ToString(context.RouteData.Values[Handler], CultureInfo.InvariantCulture);
if (!string.IsNullOrEmpty(handlerName))
{
return handlerName;
}

if (context.HttpContext.Request.Query.TryGetValue(Handler, out StringValues queryValues))
if (context.HttpContext.Request.Query.TryGetValue(Handler, out var queryValues))
{
return queryValues[0];
}
Expand All @@ -192,4 +194,4 @@ private static string GetFuzzyMatchHttpMethod(PageContext context)
return null;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private static void PopulateRouteModel(PageRouteModel model, string pageRoute, s
if (!AttributeRouteModel.IsOverridePattern(routeTemplate) &&
string.Equals(IndexFileName, fileName, StringComparison.OrdinalIgnoreCase))
{
// For pages without an override route, and ending in /Index.cshtml, we want to allow
// For pages without an override route, and ending in /Index.cshtml, we want to allow
// incoming routing, but force outgoing routes to match to the path sans /Index.
selectorModel.AttributeRouteModel.SuppressLinkGeneration = true;

Expand Down
9 changes: 6 additions & 3 deletions src/Microsoft.AspNetCore.Mvc.TagHelpers/Cache/CacheTagKey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Text;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Razor.Infrastructure;
using Microsoft.AspNetCore.Mvc.TagHelpers.Internal;
using Microsoft.AspNetCore.Razor.TagHelpers;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Internal;
Expand All @@ -25,7 +24,8 @@ public class CacheTagKey : IEquatable<CacheTagKey>
private static readonly Func<IRequestCookieCollection, string, string> CookieAccessor = (c, key) => c[key];
private static readonly Func<IHeaderDictionary, string, string> HeaderAccessor = (c, key) => c[key];
private static readonly Func<IQueryCollection, string, string> QueryAccessor = (c, key) => c[key];
private static readonly Func<RouteValueDictionary, string, string> RouteValueAccessor = (c, key) => c[key]?.ToString();
private static readonly Func<RouteValueDictionary, string, string> RouteValueAccessor = (c, key) =>
Convert.ToString(c[key], CultureInfo.InvariantCulture);

private const string CacheKeyTokenSeparator = "||";
private const string VaryByName = "VaryBy";
Expand Down Expand Up @@ -91,7 +91,10 @@ private CacheTagKey(CacheTagHelperBase tagHelper)
_cookies = ExtractCollection(tagHelper.VaryByCookie, request.Cookies, CookieAccessor);
_headers = ExtractCollection(tagHelper.VaryByHeader, request.Headers, HeaderAccessor);
_queries = ExtractCollection(tagHelper.VaryByQuery, request.Query, QueryAccessor);
_routeValues = ExtractCollection(tagHelper.VaryByRoute, tagHelper.ViewContext.RouteData.Values, RouteValueAccessor);
_routeValues = ExtractCollection(
tagHelper.VaryByRoute,
tagHelper.ViewContext.RouteData.Values,
RouteValueAccessor);
_varyByUser = tagHelper.VaryByUser;
_varyByCulture = tagHelper.VaryByCulture;

Expand Down
5 changes: 3 additions & 2 deletions src/Microsoft.AspNetCore.Mvc.TagHelpers/LinkTagHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Text.Encodings.Web;
using Microsoft.AspNetCore.Hosting;
Expand Down Expand Up @@ -441,7 +442,7 @@ private bool HasStyleSheetLinkType(TagHelperAttributeList attributes)
}
else if (stringValue == null)
{
stringValue = attributeValue.ToString();
stringValue = Convert.ToString(attributeValue, CultureInfo.InvariantCulture);
}

var hasRelStylesheet = string.Equals("stylesheet", stringValue, StringComparison.Ordinal);
Expand Down Expand Up @@ -570,4 +571,4 @@ private enum Mode
Fallback = 2,
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc.Infrastructure;
Expand Down Expand Up @@ -199,22 +200,20 @@ private static string GetActionName(ActionContext context)
throw new ArgumentNullException(nameof(context));
}

object routeValue;
if (!context.RouteData.Values.TryGetValue(ActionNameKey, out routeValue))
if (!context.RouteData.Values.TryGetValue(ActionNameKey, out var routeValue))
{
return null;
}

var actionDescriptor = context.ActionDescriptor;
string normalizedValue = null;
string value;
if (actionDescriptor.RouteValues.TryGetValue(ActionNameKey, out value) &&
if (actionDescriptor.RouteValues.TryGetValue(ActionNameKey, out var value) &&
!string.IsNullOrEmpty(value))
{
normalizedValue = value;
}

var stringRouteValue = routeValue?.ToString();
var stringRouteValue = Convert.ToString(routeValue, CultureInfo.InvariantCulture);
if (string.Equals(normalizedValue, stringRouteValue, StringComparison.OrdinalIgnoreCase))
{
return normalizedValue;
Expand Down
Loading

0 comments on commit c74a945

Please sign in to comment.