New ctor for FormUrlEncodedContent accepting Dictionary

[ManickaP]

Background and Motivation

#38494

Once we annotated FormUrlEncodedContent we have prevented people from using the ctor with Dictionary. Dictionary's TKey cannot be null, colliding with key nullability in IEnumerable<KeyValuePair<string?, string?>>.

Changing the existing API might be problematic since the type have always accepted null keys and replaces them with an empty string:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/FormUrlEncodedContent.cs

Line 37 in 5f96636

builder.Append(Encode(pair.Key));

Proposal is to add a new constructor accepting IDictionary<string, string?> and thus enabling uses like:

var nullableValueDictionary = new Dictionary<string, string?>
{
    ["foo"] = "bar"
};

_ = new FormUrlEncodedContent(nullableValueDictionary);

Proposed API

namespace  System.Net.Http
{
    public partial class FormUrlEncodedContent : ByteArrayContent
    {
+       public FormUrlEncodedContent(IDictionary<string, string?> dictionary);

        // Existing ctor
        public FormUrlEncodedContent(IEnumerable<KeyValuePair<string?, string?>> nameValueCollection);
        ...
    }
}   

Alternative Designs

Making the value non-null as well will ease the usage even more. Enabling usage with Dictionary<string, string>:

var dictionary = new Dictionary<string, string>
{
    ["foo"] = "bar"
};
_ = new FormUrlEncodedContent(dictionary);

Api proposal:

namespace  System.Net.Http
{
    public partial class FormUrlEncodedContent : ByteArrayContent
    {
        // non-nullable value as well, we cannot overload on nullability so we cannot add this later, we must chose one
+       public FormUrlEncodedContent(IDictionary<string, string> dictionary);

        // Existing ctor
        public FormUrlEncodedContent(IEnumerable<KeyValuePair<string?, string?>> nameValueCollection)
        ...
    }
}   

Usage Examples

var nullableValueDictionary = new Dictionary<string, string?>
{
    ["foo"] = "bar"
};
_ = new FormUrlEncodedContent(nullableValueDictionary);

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

---

Issue meta data

Issue content:

## Background and Motivation https://github.com//issues/38494 Once we annotated FormUrlEncodedContent we have prevented people from using the ctor with Dictionary. Dictionary's TKey cannot be null, colliding with key nullability in IEnumerable<KeyValuePair<string?, string?>>. Changing the existing API might be problematic since the type have always accepted null keys and replaces them with an empty string: runtime/src/libraries/System.Net.Http/src/System/Net/Http/FormUrlEncodedContent.cs Line 37 in 5f96636 builder.Append(Encode(pair.Key)); Proposal is to add a new constructor accepting IDictionary<string, string?> and thus enabling uses like: var nullableValueDictionary = new Dictionary<string, string?> { ["foo"] = "bar" }; _ = new FormUrlEncodedContent(nullableValueDictionary); Proposed API namespace System.Net.Http { public partial class FormUrlEncodedContent : ByteArrayContent { + public FormUrlEncodedContent(IDictionary<string, string?> dictionary); // Existing ctor public FormUrlEncodedContent(IEnumerable<KeyValuePair<string?, string?>> nameValueCollection); ... } } Alternative Designs Making the value non-null as well will ease the usage even more. Enabling usage with Dictionary<string, string>: var dictionary = new Dictionary<string, string> { ["foo"] = "bar" }; _ = new FormUrlEncodedContent(dictionary); Api proposal: namespace System.Net.Http { public partial class FormUrlEncodedContent : ByteArrayContent { // non-nullable value as well, we cannot overload on nullability so we cannot add this later, we must chose one + public FormUrlEncodedContent(IDictionary<string, string> dictionary); // Existing ctor public FormUrlEncodedContent(IEnumerable<KeyValuePair<string?, string?>> nameValueCollection) ... } } Usage Examples var nullableValueDictionary = new Dictionary<string, string?> { ["foo"] = "bar" }; _ = new FormUrlEncodedContent(nullableValueDictionary); ```</td> </tr> <tr> <td>Issue author:</td> <td>ManickaP</td> </tr> <tr> <td>Assignees:</td> <td>-</td> </tr> <tr> <td>Milestone:</td> <td>-</td> </tr> </table> </details>

[karelz]

Triage: Next step -- research if FromUrlEncodedContent can meaningfully use empty string (aka null passed in). If yes, we are ok with this API. Otherwise, we should just make the technical breaking change.

[karelz]

@ManickaP can you please look at it when you get a chance, so that we can make decision? Thanks!

[LeaFrock]

IMO, I prefer public FormUrlEncodedContent(IDictionary<string, string?> dictionary). It makes no breaking changes, and callers can write codes like below,

var nullableValueDictionary = new Dictionary<string, string?>
{
    ["foo"] = default //use easier syntax sugar instead of string.Empty(or "" which is possible to make a typo like " ") and don't worry about exceptions
};
_ = new FormUrlEncodedContent(nullableValueDictionary);

@karelz @ManickaP I can make a PR to finish it if you have more imporant things to deal with.

BTW, I think Encode here can be optimized.

  // Escape spaces as '+'.
  return Uri.EscapeDataString(data).Replace("%20", "+");

Since this method will be used every time on the keys and values, may String.Replace be removed if we have another method for escaping data string? It's just due to the difference of treating WHITESPACE by W3C and by RFC 2396. Need I to create a stand-alone issue about it?

[karelz]

Triage: Some more tests are needed to determine if we need new API - @ManickaP is on point.