Fix JTokenWriter when writing comment to an object (#2493)

Author: JamesNK

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<JToken>(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<JToken>(jsonReader);
+
+            Assert.AreEqual(JTokenType.Integer, x["ABC"].Type);
+        }
+    }
+}
+#endif

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<JToken> 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)
         /// <param name="content">The content to be added.</param>
         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);
         }
 
         /// <summary>
@@ -647,10 +654,10 @@ internal void AddAndSkipParentCheck(JToken token)
         /// <param name="content">The content to be added.</param>
         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);
             }
         }
 

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)

Src/Newtonsoft.Json/Linq/JProperty.cs

@@ -241,20 +241,20 @@ 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)
             {
                 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)

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);
         }
 
         /// <summary>
@@ -255,7 +255,7 @@ public void AddBeforeSelf(object? content)
             }
 
             int index = _parent.IndexOfItem(this);
-            _parent.AddInternal(index, content, false);
+            _parent.TryAddInternal(index, content, false);
         }
 
         /// <summary>

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