Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Route parameter serialization made consistent for custom primitive types #1689

Merged
merged 2 commits into from
Aug 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/Framework/Framework/Routing/DotvvmRoute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using DotVVM.Framework.Configuration;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using DotVVM.Framework.Utils;

namespace DotVVM.Framework.Routing
{
Expand Down Expand Up @@ -127,13 +128,7 @@ protected override string BuildUrlCore(Dictionary<string, object?> values)
var convertedValues =
values.ToDictionary(
v => v.Key,
v => {
var strVal = v.Value is IConvertible convertible ?
convertible.ToString(CultureInfo.InvariantCulture) :
v.Value?.ToString();

return strVal == null ? null : Uri.EscapeDataString(strVal);
},
v => UrlHelper.ParameterToString(v.Value),
StringComparer.OrdinalIgnoreCase
);
try
Expand Down
32 changes: 27 additions & 5 deletions src/Framework/Framework/Routing/UrlHelper.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Reflection;
using System.Text;
Expand Down Expand Up @@ -33,28 +34,28 @@ public static string BuildUrlSuffix(string? urlSuffix, object? query)
case IEnumerable<KeyValuePair<string, object>> keyValueCollection:
foreach (var item in keyValueCollection.Where(i => i.Value != null))
{
AppendQueryParam(ref resultSuffix, item.Key, item.Value.ToString().NotNull());
AppendQueryParam(ref resultSuffix, item.Key, ParameterToString(item.Value));
}
break;
default:
foreach (var prop in query.GetType().GetProperties().Where(p => p.GetValue(query) != null))
{
AppendQueryParam(ref resultSuffix, prop.Name, prop.GetValue(query)!.ToString().NotNull());
AppendQueryParam(ref resultSuffix, prop.Name, ParameterToString(prop.GetValue(query)!));
}
break;
}

return resultSuffix + (!suffixContainsHash ? "" : urlSuffix.Substring(hashIndex));
}

private static string AppendQueryParam(ref string urlSuffix, string name, string value)
private static string AppendQueryParam(ref string urlSuffix, string name, string? value)
{
urlSuffix += (urlSuffix.LastIndexOf('?') < 0 ? "?" : "&");
var hasValue = value.Trim() != string.Empty;
var hasValue = !string.IsNullOrWhiteSpace(value);

return (!hasValue) ?
urlSuffix += Uri.EscapeDataString(name) :
urlSuffix += $"{Uri.EscapeDataString(name)}={Uri.EscapeDataString(value)}";
urlSuffix += $"{Uri.EscapeDataString(name)}={value}";
}

/// <summary>
Expand Down Expand Up @@ -127,5 +128,26 @@ private static bool ContainsOnlyValidUrlChars(string url)
}
return true;
}

public static string? ParameterToString(object? value)
{
if (value is null)
{
return null;
}
else if (ReflectionUtils.TryGetCustomPrimitiveTypeRegistration(value.GetType()) is { } registration)
{
return Uri.EscapeDataString(registration.ToStringMethod(value));
}
else if (value is IConvertible convertible)
{
return Uri.EscapeDataString(convertible.ToString(CultureInfo.InvariantCulture));
}
else
{
var strVal = value.ToString();
return strVal == null ? null : Uri.EscapeDataString(strVal);
}
}
}
}
30 changes: 29 additions & 1 deletion src/Tests/Routing/DotvvmRouteTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using System.Globalization;
using DotVVM.Framework.Tests.Binding;

namespace DotVVM.Framework.Tests.Routing
{
Expand Down Expand Up @@ -458,6 +459,18 @@ public void DotvvmRoute_BuildUrl_UrlEncode()
});
}

[TestMethod]

public void DotvvmRoute_BuildUrl_CustomPrimitiveType()
{
CultureUtils.RunWithCulture("cs-CZ", () => {
var route = new DotvvmRoute("Test/{Id}", null, null, null, configuration);

var result = route.BuildUrl(new { Id = new DecimalNumber(123.4m) }) + UrlHelper.BuildUrlSuffix(null, new { Id = new DecimalNumber(555.5m) });
Assert.AreEqual("~/Test/123%2C4?Id=555%2C5", result);
});
}

[TestMethod]
public void DotvvmRoute_BuildUrl_Invalid_UnclosedParameter()
{
Expand All @@ -469,7 +482,6 @@ public void DotvvmRoute_BuildUrl_Invalid_UnclosedParameter()
});
}


[TestMethod]

public void DotvvmRoute_BuildUrl_Invalid_UnclosedParameterConstraint()
Expand Down Expand Up @@ -669,6 +681,22 @@ public void DotvvmRoute_UrlWithoutTypes()
}
}

record struct DecimalNumber(decimal Value) : IFormattable
{
public static bool TryParse(string value, out decimal result)
{
if (decimal.TryParse(value, out var r))
{
result = r;
return true;
}
result = default;
return false;
}
public override string ToString() => Value.ToString();
public string ToString(string format, IFormatProvider formatProvider) => Value.ToString(null, formatProvider);
}

public class TestPresenterWithoutInterface
{

Expand Down
Loading