Newtonsoft.Json icon indicating copy to clipboard operation
Newtonsoft.Json copied to clipboard

Empty string is treated as null

Open schani opened this issue 7 years ago • 11 comments

Source/destination types

namespace QuickType
{
    using System;
    using System.Collections.Generic;

    using System.Globalization;
    using Newtonsoft.Json;
    using Newtonsoft.Json.Converters;

    public partial class TopLevel
    {
        [JsonProperty("bar", Required = Required.DisallowNull, NullValueHandling = NullValueHandling.Ignore)]
        public Bar? Bar { get; set; }
    }

    public enum Bar { Foo, Empty };

    public partial class TopLevel
    {
        public static TopLevel FromJson(string json) => JsonConvert.DeserializeObject<TopLevel>(json, QuickType.Converter.Settings);
    }

    static class BarExtensions
    {
        public static Bar? ValueForString(string str)
        {
	    Console.WriteLine("value for string");
            switch (str)
            {
                case "": Console.WriteLine("empty"); return Bar.Empty;
                case "foo": return Bar.Foo;
                default: return null;
            }
        }

        public static Bar ReadJson(JsonReader reader, JsonSerializer serializer)
        {
            var str = serializer.Deserialize<string>(reader);
            var maybeValue = ValueForString(str);
            if (maybeValue.HasValue) return maybeValue.Value;
            throw new Exception("Unknown enum case " + str);
        }

        public static void WriteJson(this Bar value, JsonWriter writer, JsonSerializer serializer)
        {
            switch (value)
            {
                case Bar.Empty: serializer.Serialize(writer, ""); break;
                case Bar.Foo: serializer.Serialize(writer, "foo"); break;
            }
        }
    }

    public static class Serialize
    {
        public static string ToJson(this TopLevel self) => JsonConvert.SerializeObject(self, QuickType.Converter.Settings);
    }

    internal class Converter: JsonConverter
    {
        public override bool CanConvert(Type t) => t == typeof(Bar) || t == typeof(Bar?);

        public override object ReadJson(JsonReader reader, Type t, object existingValue, JsonSerializer serializer)
        {
            if (t == typeof(Bar))
                return BarExtensions.ReadJson(reader, serializer);
            if (t == typeof(Bar?))
            {
                if (reader.TokenType == JsonToken.Null) return null;
                return BarExtensions.ReadJson(reader, serializer);
            }
            throw new Exception("Unknown type");
        }

        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
        {
            var t = value.GetType();
            if (t == typeof(Bar))
            {
                ((Bar)value).WriteJson(writer, serializer);
                return;
            }
            throw new Exception("Unknown type");
        }

        public static readonly JsonSerializerSettings Settings = new JsonSerializerSettings
        {
            MetadataPropertyHandling = MetadataPropertyHandling.Ignore,
            DateParseHandling = DateParseHandling.None,
            Converters = { 
                new Converter()
            }
        };
    }

    class Program
    {
        static void Main(string[] args)
        {
            var path = args[0];
            var json = System.IO.File.ReadAllText(path);
	    var data = TopLevel.FromJson(json);
	    Console.WriteLine("have data");
            var output = data.ToJson();
            Console.WriteLine("{0}", output);
        }
    }
}

Source/destination JSON

{
        "bar": ""
}

Expected behavior

value for string
empty
have data
{"bar":""}

Actual behavior

value for string
empty

Unhandled Exception: Newtonsoft.Json.JsonSerializationException: Required property 'bar' expects a non-null value. Path '', line 3, position 1.
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EndProcessProperty(Object newObject, JsonReader reader, JsonObjectContract contract, Int32 initialDepth, JsonProperty property, PropertyPresence presence, Boolean setDefaultValue)
   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.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[T](String value, JsonSerializerSettings settings)
   at QuickType.TopLevel.FromJson(String json) in /private/tmp/csharp-071760/Program.cs:line 20
   at QuickType.Program.Main(String[] args) in /private/tmp/csharp-071760/Program.cs:line 102

Remarks

It works for {"bar":"foo"}, so the problem seems to be that the string is empty.

schani avatar Apr 25 '18 13:04 schani

Line 12 Required = Required.DisallowNull should be replaced by Required = Required.AllowNull

As the property bar disallow null, converter send an expected JsonSerializationException when bar property is empty

EDIT : as bar is empty and not null in json, the state should not be stopped by DisallowNull.

worming004 avatar May 09 '18 04:05 worming004

@worming004 I'm sorry, I don't understand your comment. What does "the state should not be stopped" mean?

schani avatar May 09 '18 04:05 schani

@schani sorry, It's the morning. I mean I understand that empty is not equal as null and the DisallowNull should not throw an exception as bar's value is empty.

However. In newtonsoft it's in the purpose to set empty string to null by using the method CoerceEmptyStringToNull. JsonSerializerInternalReader line 2513

Also, the rfc do not explain what to do with empty string. I do not know if it's a good or a bad thing that newtonsoft manage empty string or not ..

worming004 avatar May 09 '18 05:05 worming004

@worming004 So we agree this is a bug?

I can see that you might want the coercing behavior in some weird special cases, but at the very least it should be possible to disable it.

schani avatar May 09 '18 06:05 schani

I will say that is an intended behavior as the method name is explicit. I think it's not a bug.

As you say, it could be great to configure Newtonsoft as to not manage empty string as null.

worming004 avatar May 10 '18 18:05 worming004

So it's a bug in the specification, rather than in the implementation.

schani avatar May 10 '18 19:05 schani

I think that the RFC says that an empty string value is a string with no value. So the way JSON.NET handles this case currently seems rather odd.

Note: When we're talking about Enums, we're talking about integers. So the JSON should say: {"bar": 1} where 1 == Bar.Empty. That would make much more sense and it would use less resources.

Anyway, since the user code can make sense of the JSON input and produce a consumable Bar? output I believe that the result of the converter should be used in this case, thus I believe that this problem is valid and should be worked on.

Now I'm not sure what the resolution would be. Maybe an attribute on a field where we can configure how the strings should be treated.It feels kind of strange that in a case of nullable we went with "" == null.

Maybe null wasn't supported with JSON 1? I can see that it was added later on and this CoerceEmptyStringToNull hasn't been changed for a long time.

adamtajti avatar May 11 '18 14:05 adamtajti

Is there any reason this couldn't be settable in the JsonSerializerSettings?

schani avatar May 11 '18 17:05 schani

This is creating a problem for my integration. In the Database we have a field that doesn't support NULL, and I just noticed the end users are adding empty strings there. When I'm transferring this data to another database, using a service that converts this data into JSON, I got failures on the target table because the column doesn't accept NULL.

I was thinking that if I have a JSON with a empty string when I deserialize I should get the same value, an empty string.

This looks like a bug on the implementation to me and not in the specification as mentioned before.

erlis avatar Sep 30 '19 20:09 erlis

I was thinking that if I have a JSON with a empty string when I deserialize I should get the same value, an empty string.

I tend to agree with this. If the property is not found in the JSON, the value should remain null. But if the property is found, the value should be set to the exact value, when able.

JerrodV avatar Mar 26 '20 15:03 JerrodV

Same problem here,is there any fix?

mrjohnr avatar Oct 24 '20 11:10 mrjohnr

Bump. Is there a fix?

OutstandingNemo avatar Dec 06 '22 06:12 OutstandingNemo