From 672a5f3c76156a3da171fbec437cfc88c2a28423 Mon Sep 17 00:00:00 2001 From: Robert Miles Date: Mon, 5 Feb 2018 23:25:53 -0500 Subject: [PATCH] Allow null value in query string KVP per RFC 3986 (#994) * Allow null value in query string KVP per RFC 3986 * Tweaks per PR suggestions * Tweaks per PR suggestions, round 2 * Tweaks per PR suggestions, round 3 --- .../QueryString.cs | 49 +++++++++++-------- .../QueryStringTests.cs | 28 ++++++++++- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.AspNetCore.Http.Abstractions/QueryString.cs b/src/Microsoft.AspNetCore.Http.Abstractions/QueryString.cs index cc779ab1b9..772df8dfd9 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/QueryString.cs +++ b/src/Microsoft.AspNetCore.Http.Abstractions/QueryString.cs @@ -121,12 +121,12 @@ namespace Microsoft.AspNetCore.Http { throw new ArgumentNullException(nameof(name)); } - if (value == null) - { - throw new ArgumentNullException(nameof(value)); - } - return new QueryString($"?{UrlEncoder.Default.Encode(name)}={UrlEncoder.Default.Encode(value)}"); + if (!string.IsNullOrEmpty(value)) + { + value = UrlEncoder.Default.Encode(value); + } + return new QueryString($"?{UrlEncoder.Default.Encode(name)}={value}"); } /// @@ -140,11 +140,8 @@ namespace Microsoft.AspNetCore.Http bool first = true; foreach (var pair in parameters) { - builder.Append(first ? "?" : "&"); + AppendKeyValuePair(builder, pair.Key, pair.Value, first); first = false; - builder.Append(UrlEncoder.Default.Encode(pair.Key)); - builder.Append("="); - builder.Append(UrlEncoder.Default.Encode(pair.Value)); } return new QueryString(builder.ToString()); @@ -159,15 +156,21 @@ namespace Microsoft.AspNetCore.Http { var builder = new StringBuilder(); bool first = true; + foreach (var pair in parameters) { + // If nothing in this pair.Values, append null value and continue + if (StringValues.IsNullOrEmpty(pair.Value)) + { + AppendKeyValuePair(builder, pair.Key, null, first); + first = false; + continue; + } + // Otherwise, loop through values in pair.Value foreach (var value in pair.Value) { - builder.Append(first ? "?" : "&"); + AppendKeyValuePair(builder, pair.Key, value, first); first = false; - builder.Append(UrlEncoder.Default.Encode(pair.Key)); - builder.Append("="); - builder.Append(UrlEncoder.Default.Encode(value)); } } @@ -195,10 +198,6 @@ namespace Microsoft.AspNetCore.Http { throw new ArgumentNullException(nameof(name)); } - if (value == null) - { - throw new ArgumentNullException(nameof(value)); - } if (!HasValue || Value.Equals("?", StringComparison.Ordinal)) { @@ -206,10 +205,7 @@ namespace Microsoft.AspNetCore.Http } var builder = new StringBuilder(Value); - builder.Append("&"); - builder.Append(UrlEncoder.Default.Encode(name)); - builder.Append("="); - builder.Append(UrlEncoder.Default.Encode(value)); + AppendKeyValuePair(builder, name, value, first: false); return new QueryString(builder.ToString()); } @@ -250,5 +246,16 @@ namespace Microsoft.AspNetCore.Http { return left.Add(right); } + + private static void AppendKeyValuePair(StringBuilder builder, string key, string value, bool first) + { + builder.Append(first ? "?" : "&"); + builder.Append(UrlEncoder.Default.Encode(key)); + builder.Append("="); + if (!string.IsNullOrEmpty(value)) + { + builder.Append(UrlEncoder.Default.Encode(value)); + } + } } } diff --git a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/QueryStringTests.cs b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/QueryStringTests.cs index 01532d45ab..8327f12509 100644 --- a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/QueryStringTests.cs +++ b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/QueryStringTests.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Primitives; using Xunit; namespace Microsoft.AspNetCore.Http.Abstractions @@ -54,8 +55,10 @@ namespace Microsoft.AspNetCore.Http.Abstractions [InlineData("name", "value", "?name=value")] [InlineData("na me", "val ue", "?na%20me=val%20ue")] [InlineData("name", "", "?name=")] + [InlineData("name", null, "?name=")] [InlineData("", "value", "?=value")] [InlineData("", "", "?=")] + [InlineData("", null, "?=")] public void CreateNameValue_Success(string name, string value, string exepcted) { var query = QueryString.Create(name, value); @@ -70,8 +73,24 @@ namespace Microsoft.AspNetCore.Http.Abstractions new KeyValuePair("key1", "value1"), new KeyValuePair("key2", "value2"), new KeyValuePair("key3", "value3"), + new KeyValuePair("key4", null), + new KeyValuePair("key5", "") }); - Assert.Equal("?key1=value1&key2=value2&key3=value3", query.Value); + Assert.Equal("?key1=value1&key2=value2&key3=value3&key4=&key5=", query.Value); + } + + [Fact] + public void CreateFromListStringValues_Success() + { + var query = QueryString.Create(new[] + { + new KeyValuePair("key1", new StringValues("value1")), + new KeyValuePair("key2", new StringValues("value2")), + new KeyValuePair("key3", new StringValues("value3")), + new KeyValuePair("key4", new StringValues()), + new KeyValuePair("key5", new StringValues("")), + }); + Assert.Equal("?key1=value1&key2=value2&key3=value3&key4=&key5=", query.Value); } [Theory] @@ -94,11 +113,18 @@ namespace Microsoft.AspNetCore.Http.Abstractions [Theory] [InlineData("", "", "", "?=")] + [InlineData("", "", null, "?=")] [InlineData("?", "", "", "?=")] + [InlineData("?", "", null, "?=")] [InlineData("?", "name2", "value2", "?name2=value2")] + [InlineData("?", "name2", "", "?name2=")] + [InlineData("?", "name2", null, "?name2=")] [InlineData("?name1=value1", "name2", "value2", "?name1=value1&name2=value2")] [InlineData("?name1=value1", "na me2", "val ue2", "?name1=value1&na%20me2=val%20ue2")] [InlineData("?name1=value1", "", "", "?name1=value1&=")] + [InlineData("?name1=value1", "", null, "?name1=value1&=")] + [InlineData("?name1=value1", "name2", "", "?name1=value1&name2=")] + [InlineData("?name1=value1", "name2", null, "?name1=value1&name2=")] public void AddNameValue_Success(string query1, string name2, string value2, string expected) { var q1 = new QueryString(query1);