From 1745d7c14ec7e4244a5ca1c7ddf5d955cf7d1f43 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 11 Mar 2021 21:53:58 +1300 Subject: [PATCH] Fix JTokenWriter when writing comment to an object (#2493) --- Src/Newtonsoft.Json.Tests/Issues/Issue2492.cs | 77 +++++++++++++++++++ Src/Newtonsoft.Json/Linq/JContainer.cs | 25 ++++-- Src/Newtonsoft.Json/Linq/JObject.cs | 6 +- Src/Newtonsoft.Json/Linq/JProperty.cs | 6 +- Src/Newtonsoft.Json/Linq/JToken.cs | 4 +- Src/Newtonsoft.Json/Linq/JTokenWriter.cs | 15 ++-- 6 files changed, 112 insertions(+), 21 deletions(-) create mode 100644 Src/Newtonsoft.Json.Tests/Issues/Issue2492.cs diff --git a/Src/Newtonsoft.Json.Tests/Issues/Issue2492.cs b/Src/Newtonsoft.Json.Tests/Issues/Issue2492.cs new file mode 100644 index 000000000..70268abfc --- /dev/null +++ b/Src/Newtonsoft.Json.Tests/Issues/Issue2492.cs @@ -0,0 +1,77 @@ +#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 + +#if (NET45 || NET50) +#if DNXCORE50 +using Xunit; +using Test = Xunit.FactAttribute; +using Assert = Newtonsoft.Json.Tests.XUnitAssert; +#else +using NUnit.Framework; +#endif +using System.Collections.Generic; +using Newtonsoft.Json.Serialization; +using Newtonsoft.Json.Converters; +using System.Collections; +using System; +using System.IO; +using Newtonsoft.Json.Linq; + +namespace Newtonsoft.Json.Tests.Issues +{ + [TestFixture] + public class Issue2492 + { + [Test] + public void Test_Object() + { + string jsontext = @"{ ""ABC"": //DEF +{}}"; + + using var stringReader = new StringReader(jsontext); + using var jsonReader = new JsonTextReader(stringReader); + + JsonSerializer serializer = JsonSerializer.Create(); + var x = serializer.Deserialize(jsonReader); + + Assert.AreEqual(JTokenType.Object, x["ABC"].Type); + } + + [Test] + public void Test_Integer() + { + string jsontext = "{ \"ABC\": /*DEF*/ 1}"; + + using var stringReader = new StringReader(jsontext); + using var jsonReader = new JsonTextReader(stringReader); + + JsonSerializer serializer = JsonSerializer.Create(); + var x = serializer.Deserialize(jsonReader); + + Assert.AreEqual(JTokenType.Integer, x["ABC"].Type); + } + } +} +#endif \ No newline at end of file diff --git a/Src/Newtonsoft.Json/Linq/JContainer.cs b/Src/Newtonsoft.Json/Linq/JContainer.cs index ac3a18a0c..945de5c7b 100644 --- a/Src/Newtonsoft.Json/Linq/JContainer.cs +++ b/Src/Newtonsoft.Json/Linq/JContainer.cs @@ -114,7 +114,7 @@ internal JContainer(JContainer other) int i = 0; foreach (JToken child in other) { - AddInternal(i, child, false); + TryAddInternal(i, child, false); i++; } @@ -349,7 +349,7 @@ internal JToken EnsureParentToken(JToken? item, bool skipParentCheck) internal abstract int IndexOfItem(JToken? item); - internal virtual void InsertItem(int index, JToken? item, bool skipParentCheck) + internal virtual bool InsertItem(int index, JToken? item, bool skipParentCheck) { IList children = ChildrenTokens; @@ -396,6 +396,8 @@ internal virtual void InsertItem(int index, JToken? item, bool skipParentCheck) OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)); } #endif + + return true; } internal virtual void RemoveItemAt(int index) @@ -633,12 +635,17 @@ internal virtual void ValidateToken(JToken o, JToken? existing) /// The content to be added. public virtual void Add(object? content) { - AddInternal(ChildrenTokens.Count, content, false); + TryAddInternal(ChildrenTokens.Count, content, false); + } + + internal bool TryAdd(object? content) + { + return TryAddInternal(ChildrenTokens.Count, content, false); } internal void AddAndSkipParentCheck(JToken token) { - AddInternal(ChildrenTokens.Count, token, true); + TryAddInternal(ChildrenTokens.Count, token, true); } /// @@ -647,10 +654,10 @@ internal void AddAndSkipParentCheck(JToken token) /// The content to be added. public void AddFirst(object? content) { - AddInternal(0, content, false); + TryAddInternal(0, content, false); } - internal void AddInternal(int index, object? content, bool skipParentCheck) + internal bool TryAddInternal(int index, object? content, bool skipParentCheck) { if (IsMultiContent(content)) { @@ -659,15 +666,17 @@ internal void AddInternal(int index, object? content, bool skipParentCheck) int multiIndex = index; foreach (object c in enumerable) { - AddInternal(multiIndex, c, skipParentCheck); + TryAddInternal(multiIndex, c, skipParentCheck); multiIndex++; } + + return true; } else { JToken item = CreateFromContent(content); - InsertItem(index, item, skipParentCheck); + return InsertItem(index, item, skipParentCheck); } } diff --git a/Src/Newtonsoft.Json/Linq/JObject.cs b/Src/Newtonsoft.Json/Linq/JObject.cs index 8684b9f15..371a951ff 100644 --- a/Src/Newtonsoft.Json/Linq/JObject.cs +++ b/Src/Newtonsoft.Json/Linq/JObject.cs @@ -135,15 +135,15 @@ internal override int IndexOfItem(JToken? item) return _properties.IndexOfReference(item); } - internal override void InsertItem(int index, JToken? item, bool skipParentCheck) + internal override bool InsertItem(int index, JToken? item, bool skipParentCheck) { // don't add comments to JObject, no name to reference comment by if (item != null && item.Type == JTokenType.Comment) { - return; + return false; } - base.InsertItem(index, item, skipParentCheck); + return base.InsertItem(index, item, skipParentCheck); } internal override void ValidateToken(JToken o, JToken? existing) diff --git a/Src/Newtonsoft.Json/Linq/JProperty.cs b/Src/Newtonsoft.Json/Linq/JProperty.cs index 3805bd606..b59918d89 100644 --- a/Src/Newtonsoft.Json/Linq/JProperty.cs +++ b/Src/Newtonsoft.Json/Linq/JProperty.cs @@ -241,12 +241,12 @@ internal override int IndexOfItem(JToken? item) return _content.IndexOf(item); } - internal override void InsertItem(int index, JToken? item, bool skipParentCheck) + internal override bool InsertItem(int index, JToken? item, bool skipParentCheck) { // don't add comments to JProperty if (item != null && item.Type == JTokenType.Comment) { - return; + return false; } if (Value != null) @@ -254,7 +254,7 @@ internal override void InsertItem(int index, JToken? item, bool skipParentCheck) throw new JsonException("{0} cannot have multiple values.".FormatWith(CultureInfo.InvariantCulture, typeof(JProperty))); } - base.InsertItem(0, item, false); + return base.InsertItem(0, item, false); } internal override bool ContainsItem(JToken? item) diff --git a/Src/Newtonsoft.Json/Linq/JToken.cs b/Src/Newtonsoft.Json/Linq/JToken.cs index 31b7fb36c..d1f6c451f 100644 --- a/Src/Newtonsoft.Json/Linq/JToken.cs +++ b/Src/Newtonsoft.Json/Linq/JToken.cs @@ -240,7 +240,7 @@ public void AddAfterSelf(object? content) } int index = _parent.IndexOfItem(this); - _parent.AddInternal(index + 1, content, false); + _parent.TryAddInternal(index + 1, content, false); } /// @@ -255,7 +255,7 @@ public void AddBeforeSelf(object? content) } int index = _parent.IndexOfItem(this); - _parent.AddInternal(index, content, false); + _parent.TryAddInternal(index, content, false); } /// diff --git a/Src/Newtonsoft.Json/Linq/JTokenWriter.cs b/Src/Newtonsoft.Json/Linq/JTokenWriter.cs index f49169e9e..9e281c608 100644 --- a/Src/Newtonsoft.Json/Linq/JTokenWriter.cs +++ b/Src/Newtonsoft.Json/Linq/JTokenWriter.cs @@ -196,12 +196,17 @@ internal void AddValue(JValue? value, JsonToken token) { if (_parent != null) { - _parent.Add(value); - _current = _parent.Last; - - if (_parent.Type == JTokenType.Property) + // TryAdd will return false if an invalid JToken type is added. + // For example, a JComment can't be added to a JObject. + // If there is an invalid JToken type then skip it. + if (_parent.TryAdd(value)) { - _parent = _parent.Parent; + _current = _parent.Last; + + if (_parent.Type == JTokenType.Property) + { + _parent = _parent.Parent; + } } } else