From e079301905988f1d78702cbd8e7d90a39aaf7c26 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 22 Sep 2018 13:34:13 -0700 Subject: [PATCH] Fix ignored values being set in extension data (#1851) --- Src/Newtonsoft.Json.Tests/Issues/Issue1719.cs | 119 ++++++++++++++++++ Src/Newtonsoft.Json.Tests/Issues/Issue1834.cs | 4 +- .../JsonSerializerInternalReader.cs | 58 +++++++-- 3 files changed, 166 insertions(+), 15 deletions(-) create mode 100644 Src/Newtonsoft.Json.Tests/Issues/Issue1719.cs diff --git a/Src/Newtonsoft.Json.Tests/Issues/Issue1719.cs b/Src/Newtonsoft.Json.Tests/Issues/Issue1719.cs new file mode 100644 index 00000000..c8396482 --- /dev/null +++ b/Src/Newtonsoft.Json.Tests/Issues/Issue1719.cs @@ -0,0 +1,119 @@ +#region License +// Copyright (c) 2007 James Newton-King +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +#endregion + +using System; +using System.Collections; +using System.Collections.Generic; +using System.ComponentModel; +using System.IO; +using System.Reflection; +using System.Reflection.Emit; +using System.Runtime.Serialization; +#if !(NET20 || NET35 || NET40 || PORTABLE40) +using System.Threading.Tasks; +#endif +using Newtonsoft.Json.Converters; +using Newtonsoft.Json.Linq; +using Newtonsoft.Json.Serialization; +using Newtonsoft.Json.Utilities; +#if DNXCORE50 +using Xunit; +using Test = Xunit.FactAttribute; +using Assert = Newtonsoft.Json.Tests.XUnitAssert; +#else +using NUnit.Framework; +#endif + +namespace Newtonsoft.Json.Tests.Issues +{ + [TestFixture] + public class Issue1719 : TestFixtureBase + { + [Test] + public void Test() + { + ExtensionDataTestClass a = JsonConvert.DeserializeObject("{\"E\":null}", new JsonSerializerSettings + { + NullValueHandling = NullValueHandling.Ignore, + }); + + Assert.IsNull(a.PropertyBag); + } + + [Test] + public void Test_PreviousWorkaround() + { + ExtensionDataTestClassWorkaround a = JsonConvert.DeserializeObject("{\"E\":null}", new JsonSerializerSettings + { + NullValueHandling = NullValueHandling.Ignore, + }); + + Assert.IsNull(a.PropertyBag); + } + + [Test] + public void Test_DefaultValue() + { + ExtensionDataWithDefaultValueTestClass a = JsonConvert.DeserializeObject("{\"E\":2}", new JsonSerializerSettings + { + DefaultValueHandling = DefaultValueHandling.Ignore, + }); + + Assert.IsNull(a.PropertyBag); + } + + class ExtensionDataTestClass + { + public B? E { get; set; } + + [JsonExtensionData] + public IDictionary PropertyBag { get; set; } + } + + class ExtensionDataWithDefaultValueTestClass + { + [DefaultValue(2)] + public int? E { get; set; } + + [JsonExtensionData] + public IDictionary PropertyBag { get; set; } + } + + enum B + { + One, + Two + } + + class ExtensionDataTestClassWorkaround + { + [JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate, NullValueHandling = NullValueHandling.Include)] + public B? E { get; set; } + + [JsonExtensionData] + public IDictionary PropertyBag { get; set; } + } + } +} \ No newline at end of file diff --git a/Src/Newtonsoft.Json.Tests/Issues/Issue1834.cs b/Src/Newtonsoft.Json.Tests/Issues/Issue1834.cs index 46ccea4a..1886ef9d 100644 --- a/Src/Newtonsoft.Json.Tests/Issues/Issue1834.cs +++ b/Src/Newtonsoft.Json.Tests/Issues/Issue1834.cs @@ -23,7 +23,6 @@ // OTHER DEALINGS IN THE SOFTWARE. #endregion -#if !NET20 using System; using System.Collections; using System.Collections.Generic; @@ -103,5 +102,4 @@ namespace Newtonsoft.Json.Tests.Issues public string Bar { get; set; } } } -} -#endif \ No newline at end of file +} \ No newline at end of file diff --git a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs b/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs index 0de92a37..23923262 100644 --- a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs +++ b/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs @@ -995,8 +995,28 @@ namespace Newtonsoft.Json.Serialization private bool SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, object target) { - if (CalculatePropertyDetails(property, ref propertyConverter, containerContract, containerProperty, reader, target, out bool useExistingValue, out object currentValue, out JsonContract propertyContract, out bool gottenCurrentValue)) + bool skipSettingProperty = CalculatePropertyDetails( + property, + ref propertyConverter, + containerContract, + containerProperty, + reader, + target, + out bool useExistingValue, + out object currentValue, + out JsonContract propertyContract, + out bool gottenCurrentValue, + out bool ignoredValue); + + if (skipSettingProperty) { + // Don't set extension data if the value was ignored + // e.g. a null with NullValueHandling should not go in ExtensionData + if (ignoredValue) + { + return true; + } + return false; } @@ -1041,12 +1061,24 @@ namespace Newtonsoft.Json.Serialization return useExistingValue; } - private bool CalculatePropertyDetails(JsonProperty property, ref JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, object target, out bool useExistingValue, out object currentValue, out JsonContract propertyContract, out bool gottenCurrentValue) + private bool CalculatePropertyDetails( + JsonProperty property, + ref JsonConverter propertyConverter, + JsonContainerContract containerContract, + JsonProperty containerProperty, + JsonReader reader, + object target, + out bool useExistingValue, + out object currentValue, + out JsonContract propertyContract, + out bool gottenCurrentValue, + out bool ignoredValue) { currentValue = null; useExistingValue = false; propertyContract = null; gottenCurrentValue = false; + ignoredValue = false; if (property.Ignored) { @@ -1086,6 +1118,7 @@ namespace Newtonsoft.Json.Serialization // test tokenType here because null might not be convertible to some types, e.g. ignoring null when applied to DateTime if (tokenType == JsonToken.Null && ResolvedNullValueHandling(containerContract as JsonObjectContract, property) == NullValueHandling.Ignore) { + ignoredValue = true; return true; } @@ -1095,6 +1128,7 @@ namespace Newtonsoft.Json.Serialization && JsonTokenUtils.IsPrimitiveToken(tokenType) && MiscellaneousUtils.ValueEquals(reader.Value, property.GetResolvedDefaultValue())) { + ignoredValue = true; return true; } @@ -2283,9 +2317,9 @@ namespace Newtonsoft.Json.Serialization { case JsonToken.PropertyName: { - string memberName = reader.Value.ToString(); + string propertyName = reader.Value.ToString(); - if (CheckPropertyName(reader, memberName)) + if (CheckPropertyName(reader, propertyName)) { continue; } @@ -2294,18 +2328,18 @@ namespace Newtonsoft.Json.Serialization { // attempt exact case match first // then try match ignoring case - JsonProperty property = contract.Properties.GetClosestMatchProperty(memberName); + JsonProperty property = contract.Properties.GetClosestMatchProperty(propertyName); if (property == null) { if (TraceWriter != null && TraceWriter.LevelFilter >= TraceLevel.Verbose) { - TraceWriter.Trace(TraceLevel.Verbose, JsonPosition.FormatMessage(reader as IJsonLineInfo, reader.Path, "Could not find member '{0}' on {1}".FormatWith(CultureInfo.InvariantCulture, memberName, contract.UnderlyingType)), null); + TraceWriter.Trace(TraceLevel.Verbose, JsonPosition.FormatMessage(reader as IJsonLineInfo, reader.Path, "Could not find member '{0}' on {1}".FormatWith(CultureInfo.InvariantCulture, propertyName, contract.UnderlyingType)), null); } if (Serializer._missingMemberHandling == MissingMemberHandling.Error) { - throw JsonSerializationException.Create(reader, "Could not find member '{0}' on object of type '{1}'".FormatWith(CultureInfo.InvariantCulture, memberName, contract.UnderlyingType.Name)); + throw JsonSerializationException.Create(reader, "Could not find member '{0}' on object of type '{1}'".FormatWith(CultureInfo.InvariantCulture, propertyName, contract.UnderlyingType.Name)); } if (!reader.Read()) @@ -2313,7 +2347,7 @@ namespace Newtonsoft.Json.Serialization break; } - SetExtensionData(contract, member, reader, memberName, newObject); + SetExtensionData(contract, member, reader, propertyName, newObject); continue; } @@ -2325,7 +2359,7 @@ namespace Newtonsoft.Json.Serialization } SetPropertyPresence(reader, property, propertiesPresence); - SetExtensionData(contract, member, reader, memberName, newObject); + SetExtensionData(contract, member, reader, propertyName, newObject); } else { @@ -2338,7 +2372,7 @@ namespace Newtonsoft.Json.Serialization if (!reader.ReadForType(property.PropertyContract, propertyConverter != null)) { - throw JsonSerializationException.Create(reader, "Unexpected end when setting {0}'s value.".FormatWith(CultureInfo.InvariantCulture, memberName)); + throw JsonSerializationException.Create(reader, "Unexpected end when setting {0}'s value.".FormatWith(CultureInfo.InvariantCulture, propertyName)); } SetPropertyPresence(reader, property, propertiesPresence); @@ -2346,13 +2380,13 @@ namespace Newtonsoft.Json.Serialization // set extension data if property is ignored or readonly if (!SetPropertyValue(property, propertyConverter, contract, member, reader, newObject)) { - SetExtensionData(contract, member, reader, memberName, newObject); + SetExtensionData(contract, member, reader, propertyName, newObject); } } } catch (Exception ex) { - if (IsErrorHandled(newObject, contract, memberName, reader as IJsonLineInfo, reader.Path, ex)) + if (IsErrorHandled(newObject, contract, propertyName, reader as IJsonLineInfo, reader.Path, ex)) { HandleError(reader, true, initialDepth); }