From 0bfb665cc2d951f4674d7b9baca00dcc588556aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Standa=20Luke=C5=A1?= <exyi@outlook.cz>
Date: Fri, 15 Dec 2023 14:14:12 +0100
Subject: [PATCH] Fix building of query string parameters in UrlHelper

---
 src/Framework/Framework/Routing/DotvvmRoute.cs |  2 +-
 src/Framework/Framework/Routing/UrlHelper.cs   | 12 +++++-------
 src/Tests/Routing/UrlHelperTests.cs            | 12 ++++++------
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/src/Framework/Framework/Routing/DotvvmRoute.cs b/src/Framework/Framework/Routing/DotvvmRoute.cs
index 488e725c77..41225a55d8 100644
--- a/src/Framework/Framework/Routing/DotvvmRoute.cs
+++ b/src/Framework/Framework/Routing/DotvvmRoute.cs
@@ -128,7 +128,7 @@ protected override string BuildUrlCore(Dictionary<string, object?> values)
             var convertedValues =
                 values.ToDictionary(
                     v => v.Key,
-                    v => UrlHelper.ParameterToString(v.Value),
+                    v => UrlHelper.ParameterToString(v.Value) is string x ? Uri.EscapeDataString(x) : null,
                     StringComparer.OrdinalIgnoreCase
                 );
             try
diff --git a/src/Framework/Framework/Routing/UrlHelper.cs b/src/Framework/Framework/Routing/UrlHelper.cs
index 1ec51999e0..953ae4c19a 100644
--- a/src/Framework/Framework/Routing/UrlHelper.cs
+++ b/src/Framework/Framework/Routing/UrlHelper.cs
@@ -51,11 +51,10 @@ public static string BuildUrlSuffix(string? urlSuffix, object? query)
         private static string AppendQueryParam(ref string urlSuffix, string name, string? value)
         {
             urlSuffix += (urlSuffix.LastIndexOf('?') < 0 ? "?" : "&");
-            var hasValue = !string.IsNullOrWhiteSpace(value);
 
-            return (!hasValue) ?
+            return string.IsNullOrWhiteSpace(value) ?
                 urlSuffix += Uri.EscapeDataString(name) :
-                urlSuffix += $"{Uri.EscapeDataString(name)}={value}";
+                urlSuffix += $"{Uri.EscapeDataString(name)}={Uri.EscapeDataString(value)}";
         }
 
         /// <summary>
@@ -137,16 +136,15 @@ private static bool ContainsOnlyValidUrlChars(string url)
             }
             else if (ReflectionUtils.TryGetCustomPrimitiveTypeRegistration(value.GetType()) is { } registration)
             {
-                return Uri.EscapeDataString(registration.ToStringMethod(value));
+                return registration.ToStringMethod(value);
             }
             else if (value is IConvertible convertible)
             {
-                return Uri.EscapeDataString(convertible.ToString(CultureInfo.InvariantCulture));
+                return convertible.ToString(CultureInfo.InvariantCulture);
             }
             else
             {
-                var strVal = value.ToString();
-                return strVal == null ? null : Uri.EscapeDataString(strVal);
+                return value.ToString();
             }
         }
     }
diff --git a/src/Tests/Routing/UrlHelperTests.cs b/src/Tests/Routing/UrlHelperTests.cs
index 6d1384244d..9495743924 100644
--- a/src/Tests/Routing/UrlHelperTests.cs
+++ b/src/Tests/Routing/UrlHelperTests.cs
@@ -37,13 +37,13 @@ public void UrlHelper_BuildUrlSuffix_EnumerableStringString()
             var suffix = "suffix";
             var query = new List<KeyValuePair<string, string>>()
             {
-                new KeyValuePair<string, string>("key1", "value1"),
+                new KeyValuePair<string, string>("key1", "value1+x"),
                 new KeyValuePair<string, string>("key2", null!),
                 new KeyValuePair<string, string>("key3", string.Empty),
                 new KeyValuePair<string, string>("key4", "value4")
             };
             var result = UrlHelper.BuildUrlSuffix(suffix, query);
-            Assert.AreEqual("suffix?key1=value1&key3&key4=value4", result);
+            Assert.AreEqual("suffix?key1=value1%2Bx&key3&key4=value4", result);
         }
 
         [TestMethod]
@@ -52,13 +52,13 @@ public void UrlHelper_BuildUrlSuffix_EnumerableStringObject()
             var suffix = "suffix";
             var query = new List<KeyValuePair<string, object>>()
             {
-                new KeyValuePair<string, object>("key1", "value1"),
+                new KeyValuePair<string, object>("key1", "value1+x"),
                 new KeyValuePair<string, object>("key2", null!),
                 new KeyValuePair<string, object>("key3", string.Empty),
                 new KeyValuePair<string, object>("key4", "value4")
             };
             var result = UrlHelper.BuildUrlSuffix(suffix, query);
-            Assert.AreEqual("suffix?key1=value1&key3&key4=value4", result);
+            Assert.AreEqual("suffix?key1=value1%2Bx&key3&key4=value4", result);
         }
 
         [TestMethod]
@@ -67,12 +67,12 @@ public void UrlHelper_BuildUrlSuffix_Object()
             var suffix = "suffix";
             var query = new TestUrlSuffixDescriptor();
             var result = UrlHelper.BuildUrlSuffix(suffix, query);
-            Assert.AreEqual("suffix?key1=value1&key3&key4=value4", result);
+            Assert.AreEqual("suffix?key1=value1%2Bx&key3&key4=value4", result);
         }
 
         private class TestUrlSuffixDescriptor
         {
-            public string key1 { get; set; } = "value1";
+            public string key1 { get; set; } = "value1+x";
             public object key2 { get; set; } = null;
             public object key3 { get; set; } = string.Empty;
             public string key4 { get; set; } = "value4";