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 committed Oct 31, 2018
1 parent 734b919 commit c74a945
Show file tree
Hide file tree
Showing 29 changed files with 550 additions and 72 deletions.
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
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
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 @@ protected virtual IReadOnlyList<ActionDescriptor> SelectBestActions(IReadOnlyLis
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 @@ protected virtual IReadOnlyList<ActionDescriptor> SelectBestActions(IReadOnlyLis
// 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
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
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
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 KnownRouteValueConstraint(IActionDescriptorCollectionProvider actionDescr
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
@@ -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 class PageLinkGeneratorExtensions
}

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
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
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
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;
}
}
}
}
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
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
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,
}
}
}
}
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

0 comments on commit c74a945

Please sign in to comment.