Merge pull request #1502 from riganti/fix/query-parameters-null-values

Fixed building query parameters in URL suffixes
riganti · Nov 2, 2022 · aba45be · aba45be
2 parents 2cc6b81 + 54b4aeb
commit aba45be
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 4 deletions.
diff --git a/src/Framework/Framework/Routing/UrlHelper.cs b/src/Framework/Framework/Routing/UrlHelper.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Reflection;
 using System.Text;
 using DotVVM.Framework.Utils;
@@ -24,19 +25,19 @@ public static string BuildUrlSuffix(string? urlSuffix, object? query)
                 case null:
                     break;
                 case IEnumerable<KeyValuePair<string, string>> keyValueCollection:
-                    foreach (var item in keyValueCollection)
+                    foreach (var item in keyValueCollection.Where(i => i.Value != null))
                     {
                         AppendQueryParam(ref resultSuffix, item.Key, item.Value);
                     }
                     break;
                 case IEnumerable<KeyValuePair<string, object>> keyValueCollection:
-                    foreach (var item in keyValueCollection)
+                    foreach (var item in keyValueCollection.Where(i => i.Value != null))
                     {
                         AppendQueryParam(ref resultSuffix, item.Key, item.Value.ToString().NotNull());
                     }
                     break;
                 default:
-                    foreach (var prop in query.GetType().GetProperties())
+                    foreach (var prop in query.GetType().GetProperties().Where(p => p.GetValue(query) != null))
                     {
                         AppendQueryParam(ref resultSuffix, prop.Name, prop.GetValue(query)!.ToString().NotNull());
                     }
@@ -47,7 +48,14 @@ public static string BuildUrlSuffix(string? urlSuffix, object? query)
         }
 
         private static string AppendQueryParam(ref string urlSuffix, string name, string value)
-            => urlSuffix += (urlSuffix.LastIndexOf('?') < 0 ? "?" : "&") + $"{Uri.EscapeDataString(name)}={Uri.EscapeDataString(value)}";
+        {
+            urlSuffix += (urlSuffix.LastIndexOf('?') < 0 ? "?" : "&");
+            var hasValue = value.Trim() != string.Empty;
+
+            return (!hasValue) ?
+                urlSuffix += Uri.EscapeDataString(name) :
+                urlSuffix += $"{Uri.EscapeDataString(name)}={Uri.EscapeDataString(value)}";
+        }
 
         /// <summary>
         /// Checks whether the URL is local.

diff --git a/src/Tests/Routing/UrlHelperTests.cs b/src/Tests/Routing/UrlHelperTests.cs
@@ -31,5 +31,51 @@ public void UrlHelper_IsLocalUrl(string url, bool expectedResult)
             Assert.AreEqual(expectedResult, result);
         }
 
+        [TestMethod]
+        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>("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);
+        }
+
+        [TestMethod]
+        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>("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);
+        }
+
+        [TestMethod]
+        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);
+        }
+
+        private class TestUrlSuffixDescriptor
+        {
+            public string key1 { get; set; } = "value1";
+            public object key2 { get; set; } = null;
+            public object key3 { get; set; } = string.Empty;
+            public string key4 { get; set; } = "value4";
+        }
     }
 }