Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues that are variants of those #1182 #1184

Merged
merged 2 commits into from
Jan 18, 2017
Merged

Fix issues that are variants of those #1182 #1184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49746f4
Catch excessive objects in json being parsed, even if following a com…
JonHanna Jan 16, 2017
65ef5a1
Include line info on excessive content for deserialisation.
JonHanna Jan 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions Src/Newtonsoft.Json.Tests/Linq/JArrayTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,31 @@ public void Parse_NoComments()
Assert.AreEqual(3, (int)a[2]);
}

[Test]
public void Parse_ExcessiveContentJustComments()
{
string json = @"[1,2,3]/*comment*/
//Another comment.";

JArray a = JArray.Parse(json);

Assert.AreEqual(3, a.Count);
Assert.AreEqual(1, (int)a[0]);
Assert.AreEqual(2, (int)a[1]);
Assert.AreEqual(3, (int)a[2]);
}

[Test]
public void Parse_ExcessiveContent()
{
string json = @"[1,2,3]/*comment*/
//Another comment.
[]";

ExceptionAssert.Throws<JsonReaderException>(() => JArray.Parse(json),
"Additional text encountered after finished reading JSON content: [. Path '', line 3, position 0.");
}

[Test]
public void Parse_LineInfo()
{
Expand Down
25 changes: 25 additions & 0 deletions Src/Newtonsoft.Json.Tests/Linq/JObjectTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1994,5 +1994,30 @@ public void Parse_NoComments()
Assert.AreEqual(2, (int)o["prop"][1]);
Assert.AreEqual(3, (int)o["prop"][2]);
}

[Test]
public void Parse_ExcessiveContentJustComments()
{
string json = @"{'prop':[1,2,3]}/*comment*/
//Another comment.";

JObject o = JObject.Parse(json);

Assert.AreEqual(3, o["prop"].Count());
Assert.AreEqual(1, (int)o["prop"][0]);
Assert.AreEqual(2, (int)o["prop"][1]);
Assert.AreEqual(3, (int)o["prop"][2]);
}

[Test]
public void Parse_ExcessiveContent()
{
string json = @"{'prop':[1,2,3]}/*comment*/
//Another comment.
[]";

ExceptionAssert.Throws<JsonReaderException>(() => JObject.Parse(json),
"Additional text encountered after finished reading JSON content: [. Path '', line 3, position 0.");
}
}
}
25 changes: 25 additions & 0 deletions Src/Newtonsoft.Json.Tests/Linq/JTokenTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1264,5 +1264,30 @@ public void Parse_NoComments()
Assert.AreEqual(2, (int)o["prop"][1]);
Assert.AreEqual(3, (int)o["prop"][2]);
}

[Test]
public void Parse_ExcessiveContentJustComments()
{
string json = @"{'prop':[1,2,3]}/*comment*/
//Another comment.";

JToken o = JToken.Parse(json);

Assert.AreEqual(3, o["prop"].Count());
Assert.AreEqual(1, (int)o["prop"][0]);
Assert.AreEqual(2, (int)o["prop"][1]);
Assert.AreEqual(3, (int)o["prop"][2]);
}

[Test]
public void Parse_ExcessiveContent()
{
string json = @"{'prop':[1,2,3]}/*comment*/
//Another comment.
{}";

ExceptionAssert.Throws<JsonReaderException>(() => JToken.Parse(json),
"Additional text encountered after finished reading JSON content: {. Path '', line 3, position 0.");
}
}
}
47 changes: 46 additions & 1 deletion Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8253,7 +8253,52 @@ public void AdditionalContentAfterFinish()
reader.Read();

serializer.Deserialize(reader, typeof(MyType));
}, "Additional text found in JSON string after finishing deserializing object.");
}, "Additional text found in JSON string after finishing deserializing object. Path '[1]', line 1, position 5.");
}

[Test]
public void AdditionalContentAfterFinishCheckNotRequested()
{
string json = @"{ ""MyProperty"":{""Key"":""Value""}} A bunch of junk at the end of the json";

JsonSerializer serializer = new JsonSerializer();

var reader = new JsonTextReader(new StringReader(json));

MyType mt = (MyType)serializer.Deserialize(reader, typeof(MyType));
Assert.AreEqual(1, mt.MyProperty.Count);
}

[Test]
public void AdditionalContentAfterCommentsCheckNotRequested()
{
string json = @"{ ""MyProperty"":{""Key"":""Value""}} /*this is a comment */
// this is also a comment
This is just junk, though.";

JsonSerializer serializer = new JsonSerializer();

var reader = new JsonTextReader(new StringReader(json));

MyType mt = (MyType)serializer.Deserialize(reader, typeof(MyType));
Assert.AreEqual(1, mt.MyProperty.Count);
}

[Test]
public void AdditionalContentAfterComments()
{
string json = @"[{ ""MyProperty"":{""Key"":""Value""}} /*this is a comment */
// this is also a comment
,{}";

JsonSerializer serializer = new JsonSerializer();
serializer.CheckAdditionalContent = true;
var reader = new JsonTextReader(new StringReader(json));
reader.Read();
reader.Read();

ExceptionAssert.Throws<JsonSerializationException>(() => serializer.Deserialize(reader, typeof(MyType)),
"Additional text found in JSON string after finishing deserializing object. Path '[1]', line 3, position 2.");
}

[Test]
Expand Down
10 changes: 8 additions & 2 deletions Src/Newtonsoft.Json/JsonConvert.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,9 +962,15 @@ public static void PopulateObject(string value, object target, JsonSerializerSet
{
jsonSerializer.Populate(jsonReader, target);

if (jsonReader.Read() && jsonReader.TokenType != JsonToken.Comment)
if (settings != null && settings.CheckAdditionalContent)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this check because it seems that the current behaviour of throwing when there isn't a setting requesting it is, given that such a setting exists and is available to the user, a bug in itself. Maybe I'm misinterpreting the intent here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that makes sense.

{
throw new JsonSerializationException("Additional text found in JSON string after finishing deserializing object.");
while (jsonReader.Read())
{
if (jsonReader.TokenType != JsonToken.Comment)
{
throw JsonSerializationException.Create(jsonReader, "Additional text found in JSON string after finishing deserializing object.");
}
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Src/Newtonsoft.Json/Linq/JArray.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ internal override JToken CloneToken()
{
JArray a = Load(reader, settings);

if (reader.Read() && reader.TokenType != JsonToken.Comment)
while (reader.Read())
{
throw JsonReaderException.Create(reader, "Additional text found in JSON string after parsing content.");
// Any content encountered here other than a comment will throw in the reader.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As became apparent. There's no way to actually detect reader.TokenType != JsonToken.Comment without catching the exception from the reader itself, and little point since the exception thrown is applicable.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Looks good to me.

}

return a;
Expand Down
4 changes: 2 additions & 2 deletions Src/Newtonsoft.Json/Linq/JObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,9 +426,9 @@ public JToken this[string propertyName]
{
JObject o = Load(reader, settings);

if (reader.Read() && reader.TokenType != JsonToken.Comment)
while (reader.Read())
{
throw JsonReaderException.Create(reader, "Additional text found in JSON string after parsing content.");
// Any content encountered here other than a comment will throw in the reader.
}

return o;
Expand Down
5 changes: 3 additions & 2 deletions Src/Newtonsoft.Json/Linq/JToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2187,11 +2187,12 @@ public static JToken Parse(string json, JsonLoadSettings settings)
{
JToken t = Load(reader, settings);

if (reader.Read() && reader.TokenType != JsonToken.Comment)
while (reader.Read())
{
throw JsonReaderException.Create(reader, "Additional text found in JSON string after parsing content.");
// Any content encountered here other than a comment will throw in the reader.
}


return t;
}
}
Expand Down
2 changes: 1 addition & 1 deletion Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public object Deserialize(JsonReader reader, Type objectType, bool checkAddition
{
if (reader.TokenType != JsonToken.Comment)
{
throw new JsonSerializationException("Additional text found in JSON string after finishing deserializing object.");
throw JsonSerializationException.Create(reader, "Additional text found in JSON string after finishing deserializing object.");
}
}
}
Expand Down