Skip to content

Commit

Permalink
Fix JTokenWriter when writing comment to an object (#2493)
Browse files Browse the repository at this point in the history
  • Loading branch information
JamesNK authored Mar 11, 2021
1 parent 583eb12 commit 1745d7c
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 21 deletions.
77 changes: 77 additions & 0 deletions Src/Newtonsoft.Json.Tests/Issues/Issue2492.cs
Original file line number Diff line number Diff line change
@@ -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
25 changes: 17 additions & 8 deletions Src/Newtonsoft.Json/Linq/JContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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>
Expand All @@ -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))
{
Expand All @@ -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);
}
}

Expand Down
6 changes: 3 additions & 3 deletions Src/Newtonsoft.Json/Linq/JObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions Src/Newtonsoft.Json/Linq/JProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions Src/Newtonsoft.Json/Linq/JToken.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
15 changes: 10 additions & 5 deletions Src/Newtonsoft.Json/Linq/JTokenWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1745d7c

Please sign in to comment.