ArgumentNullException in Json.NET 6.0.7 when deserializing into NameValueCollection

Question

I've written some custom JsonConverters to deserialize json text into System.Net.Mail.MailMessage objects. Here's the complete code, which can be run in LINQPad. Interestingly, this code runs as expected in Json.NET 4.5.11:

void Main()
{
const string JsonMessage = @"{
  ""From"": {
    ""Address"": ""askywalker@theEmpire.gov"",
    ""DisplayName"": ""Darth Vader""
  },
  ""Sender"": null,
  ""ReplyTo"": null,
  ""ReplyToList"": [],
  ""To"": [
    {
      ""Address"": ""lskywalker@theRebellion.org"",
      ""DisplayName"": ""Luke Skywalker""
    }
  ],
  ""Bcc"": [],
  ""CC"": [
    {
      ""Address"": ""lorgana@alderaan.gov"",
      ""DisplayName"": ""Princess Leia""
    }
  ],
  ""Priority"": 0,
  ""DeliveryNotificationOptions"": 0,
  ""Subject"": ""Family tree"",
  ""SubjectEncoding"": null,
  ""Headers"": [],
  ""HeadersEncoding"": null,
  ""Body"": ""<strong>I am your father!</strong>"",
  ""BodyEncoding"": ""US-ASCII"",
  ""BodyTransferEncoding"": -1,
  ""IsBodyHtml"": true,
  ""Attachments"": [
    {
      ""FileName"": ""skywalker family tree.jpg"",
      ""ContentBase64"": ""AQIDBAU=""
    }
  ],
  ""AlternateViews"": []
}";
    JsonConvert.DeserializeObject<MailMessage>(JsonMessage, 
    new MailAddressReadConverter(), new AttachmentReadConverter(), new EncodingReadConverter()).Dump();

}

    public class MailAddressReadConverter : JsonConverter
    {
        public override bool CanConvert(Type objectType)
        {
            return objectType == typeof(MailAddress);
        }

        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
        {
            var messageJObject = serializer.Deserialize<JObject>(reader);
            if (messageJObject == null)
            {
                return null;
            }

            var address = messageJObject.GetValue("Address", StringComparison.OrdinalIgnoreCase).ToObject<string>();

            JToken displayNameToken;
            string displayName;
            if (messageJObject.TryGetValue("DisplayName", StringComparison.OrdinalIgnoreCase, out displayNameToken)
                && !string.IsNullOrEmpty(displayName = displayNameToken.ToObject<string>()))
            {
                return new MailAddress(address, displayName);
            }

            return new MailAddress(address);
        }

        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
        {
            throw new NotImplementedException();
        }
    }

        public class AttachmentReadConverter : JsonConverter
    {
        public override bool CanConvert(Type objectType)
        {
            return objectType == typeof(Attachment);
        }

        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
        {
            var info = serializer.Deserialize<AttachmentInfo>(reader);

            var attachment = info != null
                ? new Attachment(new MemoryStream(Convert.FromBase64String(info.ContentBase64)), "application/octet-stream")
                {
                    ContentDisposition = { FileName = info.FileName }
                }
                : null;
            return attachment;
        }

        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
        {
            throw new NotImplementedException();
        }

        private class AttachmentInfo
        {
            [JsonProperty(Required = Required.Always)]
            public string FileName { get; set; }

            [JsonProperty(Required = Required.Always)]
            public string ContentBase64 { get; set; }
        }
    }

        public class EncodingReadConverter : JsonConverter
    {
        public override bool CanConvert(Type objectType)
        {
            return typeof(Encoding).IsAssignableFrom(objectType);
        }

        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
        {
            var encodingName = serializer.Deserialize<string>(reader);
            return encodingName.NullSafe(s => Encoding.GetEncoding(s));
        }

        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
        {
            throw new NotImplementedException();
        }
    }

The exception hit is:

System.ArgumentNullException : Value cannot be null.
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at Newtonsoft.Json.Serialization.JsonArrayContract.CreateWrapper(Object list)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateList(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, Object existingValue, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonConverter[] converters)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, JsonConverter[] converters)

Is this a bug in JSON 6? Am I doing something wrong?

EDIT: through further debugging, I've determined that the issue is the Headers property.

Answer 1

The basic problem here is that MailMessage.Headers returns a NameValueCollection, which is sort of like a dictionary, but doesn't implement IDictionary<TKey, TValue> or even the non-generic IDictionary. Instead it implements the non-generic interfaces ICollection and IEnumerable. What these interfaces actually do is to loop through the keys of the collection only, completely ignoring the values.

Thus, if I create a NameValueCollection like so:

    public static NameValueCollection CreateCollection()
    {
        NameValueCollection collection = new NameValueCollection();
        FillCollection(collection);
        return collection;
    }

    private static void FillCollection(NameValueCollection collection)
    {
        collection.Add("Sam", "Dot Net Perls");
        collection.Add("Bill", "Microsoft");
        collection.Add("Bill", "White House");
        collection.Add("Sam", "IBM");
    }

and serialize it with Json.NET 6.0.7, it sees the incoming class is a non-generic collection and serializes it as an array:

        var collection = CreateCollection();
        var json = JsonConvert.SerializeObject(collection);
        Debug.WriteLine(json);

producing:

        ["Sam","Bill"]

As you can see, the values have been stripped.

Then upon deserialization, Json.NET attempts to convert the array of strings back to a NameValueCollection, but has no way to do so. In particular, it tries to construct a temporary list to hold the data being read, but gets confused over the base type of the list, and throws an exception. This is possibly a bug in Json.NET, but even if it didn't throw the exception, data was already lost on storage. This can be reproduced with a simple test class like the following:

public class NameValueCollectionWrapper
{
    public NameValueCollectionWrapper()
    {
        this.Collection = new NameValueCollection();
    }

    public NameValueCollection Collection { get; private set; }
}

So, the question is, do you want to read the headers, or do you want to ignore them? And if you want to read them, in what format will you receive them? If you want to send and receive them successfully, you will need to write a custom JsonConverter. Doing this is a little tricky because NameValueCollection is almost like a Dictionary<string, string []>, but it preserves the order in which keys are added, which Dictionary does not. Ideally, serialization should preserve that order. This can be accomplished by creating and serializing a adapter pattern wrapper IDictionary<string, string []> such as the one from this answer to how to convert NameValueCollection to JSON string?:

public class NameValueCollectionDictionaryAdapter<TNameValueCollection> : IDictionary<string, string[]>
    where TNameValueCollection : NameValueCollection, new()
{
    readonly TNameValueCollection collection;

    public NameValueCollectionDictionaryAdapter() : this(new TNameValueCollection()) { }

    public NameValueCollectionDictionaryAdapter(TNameValueCollection collection)
    {
        this.collection = collection;
    }

    // Method instead of a property to guarantee that nobody tries to serialize it.
    public TNameValueCollection GetCollection() { return collection; }

    #region IDictionary<string,string[]> Members

    public void Add(string key, string[] value)
    {
        if (collection.GetValues(key) != null)
            throw new ArgumentException("Duplicate key " + key);
        if (value == null)
            collection.Add(key, null);
        else
            foreach (var str in value)
                collection.Add(key, str);
    }

    public bool ContainsKey(string key) { return collection.GetValues(key) != null; }

    public ICollection<string> Keys { get { return collection.AllKeys; } }

    public bool Remove(string key)
    {
        bool found = ContainsKey(key);
        if (found)
            collection.Remove(key);
        return found;
    }

    public bool TryGetValue(string key, out string[] value)
    {
        return (value = collection.GetValues(key)) != null;
    }

    public ICollection<string[]> Values
    {
        get
        {
            return new ReadOnlyCollectionAdapter<KeyValuePair<string, string[]>, string[]>(this, p => p.Value);
        }
    }

    public string[] this[string key]
    {
        get
        {
            var value = collection.GetValues(key);
            if (value == null)
                throw new KeyNotFoundException(key);
            return value;
        }
        set
        {
            Remove(key);
            Add(key, value);
        }
    }

    #endregion

    #region ICollection<KeyValuePair<string,string[]>> Members

    public void Add(KeyValuePair<string, string[]> item) { Add(item.Key, item.Value); }

    public void Clear() { collection.Clear(); }

    public bool Contains(KeyValuePair<string, string[]> item)
    {
        string[] value;
        if (!TryGetValue(item.Key, out value))
            return false;
        return EqualityComparer<string[]>.Default.Equals(item.Value, value); // Consistent with Dictionary<TKey, TValue>
    }

    public void CopyTo(KeyValuePair<string, string[]>[] array, int arrayIndex)
    {
        foreach (var item in this)
            array[arrayIndex++] = item;
    }

    public int Count { get { return collection.Count; } }

    public bool IsReadOnly { get { return false; } }

    public bool Remove(KeyValuePair<string, string[]> item)
    {
        if (Contains(item))
            return Remove(item.Key);
        return false;
    }

    #endregion

    #region IEnumerable<KeyValuePair<string,string[]>> Members

    public IEnumerator<KeyValuePair<string, string[]>> GetEnumerator()
    {
        foreach (string key in collection)
            yield return new KeyValuePair<string, string[]>(key, collection.GetValues(key));
    }

    #endregion

    #region IEnumerable Members

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { return GetEnumerator(); }

    #endregion
}

public static class NameValueCollectionExtensions
{
    public static NameValueCollectionDictionaryAdapter<TNameValueCollection> ToDictionaryAdapter<TNameValueCollection>(this TNameValueCollection collection)
        where TNameValueCollection : NameValueCollection, new()
    {
        if (collection == null)
            throw new ArgumentNullException();
        return new NameValueCollectionDictionaryAdapter<TNameValueCollection>(collection);
    }
}

public class ReadOnlyCollectionAdapter<TIn, TOut> : CollectionAdapterBase<TIn, TOut, ICollection<TIn>>
{
    public ReadOnlyCollectionAdapter(ICollection<TIn> collection, Func<TIn, TOut> toOuter)
        : base(() => collection, toOuter)
    {
    }

    public override void Add(TOut item) { throw new NotImplementedException(); }

    public override void Clear() { throw new NotImplementedException(); }

    public override bool IsReadOnly { get { return true; } }

    public override bool Remove(TOut item) { throw new NotImplementedException(); }
}

public abstract class CollectionAdapterBase<TIn, TOut, TCollection> : ICollection<TOut> 
    where TCollection : ICollection<TIn>
{
    readonly Func<TCollection> getCollection;
    readonly Func<TIn, TOut> toOuter;

    public CollectionAdapterBase(Func<TCollection> getCollection, Func<TIn, TOut> toOuter)
    {
        if (getCollection == null || toOuter == null)
            throw new ArgumentNullException();
        this.getCollection = getCollection;
        this.toOuter = toOuter;
    }

    protected TCollection Collection { get { return getCollection(); } }

    protected TOut ToOuter(TIn inner) { return toOuter(inner); }

    #region ICollection<TOut> Members

    public abstract void Add(TOut item);

    public abstract void Clear();

    public virtual bool Contains(TOut item)
    {
        var comparer = EqualityComparer<TOut>.Default;
        foreach (var member in Collection)
            if (comparer.Equals(item, ToOuter(member)))
                return true;
        return false;
    }

    public void CopyTo(TOut[] array, int arrayIndex)
    {
        foreach (var item in this)
            array[arrayIndex++] = item;
    }

    public int Count { get { return Collection.Count; } }

    public abstract bool IsReadOnly { get; }

    public abstract bool Remove(TOut item);

    #endregion

    #region IEnumerable<TOut> Members

    public IEnumerator<TOut> GetEnumerator()
    {
        foreach (var item in Collection)
            yield return ToOuter(item);
    }

    #endregion

    #region IEnumerable Members

    IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); }

    #endregion
}

Next, create the following JsonConverter which both serializes and deserializes a NameValueCollection and skips values in the broken, old format:

public class NameValueJsonConverter<TNameValueCollection> : JsonConverter
    where TNameValueCollection : NameValueCollection, new()
{
    public override bool CanConvert(Type objectType)
    {
        return typeof(TNameValueCollection).IsAssignableFrom(objectType);
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        if (reader.SkipComments().TokenType == JsonToken.Null)
            return null;

        var collection = (TNameValueCollection)existingValue ?? new TNameValueCollection();
        var dictionaryWrapper = collection.ToDictionaryAdapter();

        if (reader.TokenType != JsonToken.StartObject)
        {
            // Old buggy name value collection format in which the values were not written and so cannot be recovered.
            // Skip the token and all its children.
            reader.Skip();
        }
        else
        {
            serializer.Populate(reader, dictionaryWrapper);
        }

        return collection;
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        var collection = (TNameValueCollection)value;
        var dictionaryWrapper = new NameValueCollectionDictionaryAdapter<TNameValueCollection>(collection);
        serializer.Serialize(writer, dictionaryWrapper);
    }
}

public static partial class JsonExtensions
{
    public static JsonReader SkipComments(this JsonReader reader)
    {
        while (reader.TokenType == JsonToken.Comment && reader.Read())
            ;
        return reader;
    }
}

Finally, apply NameValueJsonConverter<NameValueCollection> as you do your other converters. This produces output in Json dictionary style while preserving order, for instance:

{"Sam":["Dot Net Perls","IBM"],"Bill":["Microsoft","White House"]}

I don't have Json.NET 4.x available to test, but I doubt it correctly serialized both the keys and values of a NameValueCollection. You may want to install that version to doublecheck what it did.

Update

Just checked Json.NET 4.5.11. In that version the NameValueCollection property in my NameValueCollectionWrapper test class is serialized as an array of key strings, which is then ignored on deserialization (the collection comes back empty). So it's probably a regression that Json.NET version 6 throws an exception rather than ignoring the property.