From bdc73900b5bc0ceef9e47fd33a11d47e74b12a73 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 17 May 2012 15:27:26 +1200 Subject: [PATCH] -Added CheckAdditionalContent to JsonSerializer/JsonSerializerSettings -Fixed bad JSON in an array with error handling creating an infinite loop -JsonTextReader no longer returns no content after consecutive underlying content read failures --- .../JsonTextReaderTest.cs | 3 +- .../Properties/AssemblyInfo.cs | 2 +- .../Serialization/JsonSerializerTest.cs | 22 ++++++ .../SerializationErrorHandlingTests.cs | 79 +++++++++++++++---- Src/Newtonsoft.Json/JsonConvert.cs | 14 +--- Src/Newtonsoft.Json/JsonReader.cs | 13 ++- Src/Newtonsoft.Json/JsonSerializer.cs | 16 +++- Src/Newtonsoft.Json/JsonSerializerSettings.cs | 8 ++ Src/Newtonsoft.Json/JsonTextReader.cs | 25 +----- .../Properties/AssemblyInfo.cs | 2 +- .../JsonSerializerInternalReader.cs | 69 +++++++++++----- .../Serialization/JsonSerializerProxy.cs | 8 +- 12 files changed, 182 insertions(+), 79 deletions(-) diff --git a/Src/Newtonsoft.Json.Tests/JsonTextReaderTest.cs b/Src/Newtonsoft.Json.Tests/JsonTextReaderTest.cs index 008c827e..405b706a 100644 --- a/Src/Newtonsoft.Json.Tests/JsonTextReaderTest.cs +++ b/Src/Newtonsoft.Json.Tests/JsonTextReaderTest.cs @@ -2529,7 +2529,8 @@ bye", reader.Value); toggleReaderError.Error = false; - Assert.IsFalse(jsonTextReader.Read()); + //a reader use to skip to the end after 3 errors in a row + //Assert.IsFalse(jsonTextReader.Read()); } public class ToggleReaderError : TextReader diff --git a/Src/Newtonsoft.Json.Tests/Properties/AssemblyInfo.cs b/Src/Newtonsoft.Json.Tests/Properties/AssemblyInfo.cs index 9fd3c176..85599bcd 100644 --- a/Src/Newtonsoft.Json.Tests/Properties/AssemblyInfo.cs +++ b/Src/Newtonsoft.Json.Tests/Properties/AssemblyInfo.cs @@ -72,5 +72,5 @@ using System.Security; // by using the '*' as shown below: [assembly: AssemblyVersion("4.5.0.0")] #if !PocketPC -[assembly: AssemblyFileVersion("4.5.5.14913")] +[assembly: AssemblyFileVersion("4.5.5.14917")] #endif diff --git a/Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs b/Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs index c650990f..81315563 100644 --- a/Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs +++ b/Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs @@ -6256,6 +6256,28 @@ Parameter name: value", }", json); } #endif + + [Test] + public void CheckAdditionalContent() + { + string json = "{one:1}{}"; + + JsonSerializerSettings settings = new JsonSerializerSettings(); + JsonSerializer s = JsonSerializer.Create(settings); + IDictionary o = s.Deserialize>(new JsonTextReader(new StringReader(json))); + + Assert.IsNotNull(o); + Assert.AreEqual(1, o["one"]); + + settings.CheckAdditionalContent = true; + s = JsonSerializer.Create(settings); + ExceptionAssert.Throws( + "Additional text encountered after finished reading JSON content: {. Path '', line 1, position 7.", + () => + { + s.Deserialize>(new JsonTextReader(new StringReader(json))); + }); + } } #if !(SILVERLIGHT || NETFX_CORE || PORTABLE) diff --git a/Src/Newtonsoft.Json.Tests/Serialization/SerializationErrorHandlingTests.cs b/Src/Newtonsoft.Json.Tests/Serialization/SerializationErrorHandlingTests.cs index 9100e33e..e559f04f 100644 --- a/Src/Newtonsoft.Json.Tests/Serialization/SerializationErrorHandlingTests.cs +++ b/Src/Newtonsoft.Json.Tests/Serialization/SerializationErrorHandlingTests.cs @@ -165,8 +165,16 @@ namespace Newtonsoft.Json.Tests.Serialization { List errors = new List(); - List c = JsonConvert.DeserializeObject>( - @"[ + JsonSerializer serializer = JsonSerializer.Create(new JsonSerializerSettings + { + Error = delegate(object sender, ErrorEventArgs args) + { + errors.Add(args.ErrorContext.Path + " - " + args.ErrorContext.Member + " - " + args.ErrorContext.Error.Message); + args.ErrorContext.Handled = true; + }, + Converters = { new IsoDateTimeConverter() } + }); + var c = serializer.Deserialize>(new JsonTextReader(new StringReader(@"[ ""2009-09-09T00:00:00Z"", ""I am not a date and will error!"", [ @@ -175,16 +183,8 @@ namespace Newtonsoft.Json.Tests.Serialization ""1977-02-20T00:00:00Z"", null, ""2000-12-01T00:00:00Z"" - ]", - new JsonSerializerSettings - { - Error = delegate(object sender, ErrorEventArgs args) - { - errors.Add(args.ErrorContext.Path + " - " + args.ErrorContext.Member + " - " + args.ErrorContext.Error.Message); - args.ErrorContext.Handled = true; - }, - Converters = {new IsoDateTimeConverter()} - }); + ]"))); + // 2009-09-09T00:00:00Z // 1977-02-20T00:00:00Z @@ -377,11 +377,60 @@ namespace Newtonsoft.Json.Tests.Serialization ErrorPerson[] result = serializer.Deserialize(new JsonTextReader(new ThrowingReader())); Assert.IsNull(result); - Assert.AreEqual(4, errors.Count); + Assert.AreEqual(3, errors.Count); Assert.AreEqual("too far", errors[0]); Assert.AreEqual("too far", errors[1]); - Assert.AreEqual("too far", errors[2]); - Assert.AreEqual("Unexpected end when deserializing array. Path '[1023]', line 1, position 65536.", errors[3]); + Assert.AreEqual("Infinite loop detected from error handling. Path '[1023]', line 1, position 65536.", errors[2]); + } + + [Test] + public void InfiniteLoopArrayHandling() + { + IList errors = new List(); + + object o = JsonConvert.DeserializeObject( + "[0,x]", + typeof (int[]), + new JsonSerializerSettings + { + Error = (sender, arg) => + { + errors.Add(arg.ErrorContext.Error.Message); + arg.ErrorContext.Handled = true; + } + }); + + Assert.IsNull(o); + + Assert.AreEqual(3, errors.Count); + Assert.AreEqual("Unexpected character encountered while parsing value: x. Path '[0]', line 1, position 3.", errors[0]); + Assert.AreEqual("Unexpected character encountered while parsing value: x. Path '[0]', line 1, position 3.", errors[1]); + Assert.AreEqual("Infinite loop detected from error handling. Path '[0]', line 1, position 3.", errors[2]); + } + + [Test] + public void InfiniteLoopArrayHandlingInObject() + { + IList errors = new List(); + + Dictionary o = JsonConvert.DeserializeObject>( + "{'badarray':[0,x,2],'goodarray':[0,1,2]}", + new JsonSerializerSettings + { + Error = (sender, arg) => + { + errors.Add(arg.ErrorContext.Error.Message); + arg.ErrorContext.Handled = true; + } + }); + + Assert.IsNull(o); + + Assert.AreEqual(4, errors.Count); + Assert.AreEqual("Unexpected character encountered while parsing value: x. Path 'badarray[0]', line 1, position 15.", errors[0]); + Assert.AreEqual("Unexpected character encountered while parsing value: x. Path 'badarray[0]', line 1, position 15.", errors[1]); + Assert.AreEqual("Infinite loop detected from error handling. Path 'badarray[0]', line 1, position 15.", errors[2]); + Assert.AreEqual("Unexpected character encountered while parsing value: x. Path 'badarray[0]', line 1, position 15.", errors[3]); } public class ThrowingReader : TextReader diff --git a/Src/Newtonsoft.Json/JsonConvert.cs b/Src/Newtonsoft.Json/JsonConvert.cs index 3b5edbd1..c50e9ef6 100644 --- a/Src/Newtonsoft.Json/JsonConvert.cs +++ b/Src/Newtonsoft.Json/JsonConvert.cs @@ -918,17 +918,11 @@ namespace Newtonsoft.Json StringReader sr = new StringReader(value); JsonSerializer jsonSerializer = JsonSerializer.Create(settings); - object deserializedValue; + // by default DeserializeObject should check for additional content + if (!jsonSerializer.IsCheckAdditionalContentSet()) + jsonSerializer.CheckAdditionalContent = true; - using (JsonReader jsonReader = new JsonTextReader(sr)) - { - deserializedValue = jsonSerializer.Deserialize(jsonReader, type); - - if (jsonReader.Read() && jsonReader.TokenType != JsonToken.Comment) - throw new JsonSerializationException("Additional text found in JSON string after finishing deserializing object."); - } - - return deserializedValue; + return jsonSerializer.Deserialize(new JsonTextReader(sr), type); } #if !(NET20 || NET35 || SILVERLIGHT || PORTABLE) diff --git a/Src/Newtonsoft.Json/JsonReader.cs b/Src/Newtonsoft.Json/JsonReader.cs index 211658a6..1e1fae44 100644 --- a/Src/Newtonsoft.Json/JsonReader.cs +++ b/Src/Newtonsoft.Json/JsonReader.cs @@ -112,6 +112,7 @@ namespace Newtonsoft.Json private int? _maxDepth; private bool _hasExceededMaxDepth; internal DateParseHandling _dateParseHandling; + private readonly List _stack; /// /// Gets the current reader state. @@ -122,8 +123,6 @@ namespace Newtonsoft.Json get { return _currentState; } } - private readonly List _stack; - /// /// Gets or sets a value indicating whether the underlying stream or /// should be closed when the reader is closed. @@ -239,6 +238,14 @@ namespace Newtonsoft.Json set { _culture = value; } } + internal JsonPosition GetPosition(int depth) + { + if (depth < _stack.Count) + return _stack[depth]; + + return _currentPosition; + } + /// /// Initializes a new instance of the class with the specified . /// @@ -705,8 +712,6 @@ namespace Newtonsoft.Json /// The value. protected void SetToken(JsonToken newToken, object value) { - JsonToken previousToken = _tokenType; - _tokenType = newToken; _value = value; diff --git a/Src/Newtonsoft.Json/JsonSerializer.cs b/Src/Newtonsoft.Json/JsonSerializer.cs index cb8ba89c..fe7d4ff5 100644 --- a/Src/Newtonsoft.Json/JsonSerializer.cs +++ b/Src/Newtonsoft.Json/JsonSerializer.cs @@ -64,6 +64,7 @@ namespace Newtonsoft.Json private CultureInfo _culture; private int? _maxDepth; private bool _maxDepthSet; + private bool? _checkAdditionalContent; /// /// Occurs when the errors during serialization and deserialization. @@ -348,6 +349,18 @@ namespace Newtonsoft.Json _maxDepthSet = true; } } + + public virtual bool CheckAdditionalContent + { + get { return _checkAdditionalContent ?? JsonSerializerSettings.DefaultCheckAdditionalContent; } + set { _checkAdditionalContent = value; } + } + + internal bool IsCheckAdditionalContentSet() + { + return (_checkAdditionalContent != null); + } + #endregion /// @@ -392,6 +405,7 @@ namespace Newtonsoft.Json jsonSerializer.DefaultValueHandling = settings.DefaultValueHandling; jsonSerializer.ConstructorHandling = settings.ConstructorHandling; jsonSerializer.Context = settings.Context; + jsonSerializer._checkAdditionalContent = settings._checkAdditionalContent; // reader/writer specific // unset values won't override reader/writer set values @@ -523,7 +537,7 @@ namespace Newtonsoft.Json } JsonSerializerInternalReader serializerReader = new JsonSerializerInternalReader(this); - object value = serializerReader.Deserialize(reader, objectType); + object value = serializerReader.Deserialize(reader, objectType, CheckAdditionalContent); // reset reader back to previous options if (previousCulture != null) diff --git a/Src/Newtonsoft.Json/JsonSerializerSettings.cs b/Src/Newtonsoft.Json/JsonSerializerSettings.cs index d993cac4..4f4f78c3 100644 --- a/Src/Newtonsoft.Json/JsonSerializerSettings.cs +++ b/Src/Newtonsoft.Json/JsonSerializerSettings.cs @@ -53,12 +53,14 @@ namespace Newtonsoft.Json internal const DateTimeZoneHandling DefaultDateTimeZoneHandling = DateTimeZoneHandling.RoundtripKind; internal const DateParseHandling DefaultDateParseHandling = DateParseHandling.DateTime; internal static readonly CultureInfo DefaultCulture; + internal const bool DefaultCheckAdditionalContent = false; internal Formatting? _formatting; internal DateFormatHandling? _dateFormatHandling; internal DateTimeZoneHandling? _dateTimeZoneHandling; internal DateParseHandling? _dateParseHandling; internal CultureInfo _culture; + internal bool? _checkAdditionalContent; internal int? _maxDepth; internal bool _maxDepthSet; @@ -214,6 +216,12 @@ namespace Newtonsoft.Json set { _culture = value; } } + public bool CheckAdditionalContent + { + get { return _checkAdditionalContent ?? DefaultCheckAdditionalContent; } + set { _checkAdditionalContent = value; } + } + static JsonSerializerSettings() { DefaultContext = new StreamingContext(); diff --git a/Src/Newtonsoft.Json/JsonTextReader.cs b/Src/Newtonsoft.Json/JsonTextReader.cs index 7c1a8df0..ef8ffbc6 100644 --- a/Src/Newtonsoft.Json/JsonTextReader.cs +++ b/Src/Newtonsoft.Json/JsonTextReader.cs @@ -55,8 +55,6 @@ namespace Newtonsoft.Json private const char UnicodeReplacementChar = '\uFFFD'; private readonly TextReader _reader; - private int _readerFailureCount; - private char[] _chars; private int _charsUsed; private int _charPos; @@ -76,10 +74,7 @@ namespace Newtonsoft.Json throw new ArgumentNullException("reader"); _reader = reader; - _readerFailureCount = 0; - _lineNumber = 1; - _chars = new char[4097]; } @@ -326,25 +321,7 @@ namespace Newtonsoft.Json int attemptCharReadCount = _chars.Length - _charsUsed - 1; - int charsRead; - - try - { - charsRead = _reader.Read(_chars, _charsUsed, attemptCharReadCount); - _readerFailureCount = 0; - } - catch - { - // to prevent infinate loop when error handling is enabled - if (!_isEndOfFile) - { - _readerFailureCount++; - if (_readerFailureCount >= 3) - _isEndOfFile = true; - } - - throw; - } + int charsRead = _reader.Read(_chars, _charsUsed, attemptCharReadCount); _charsUsed += charsRead; diff --git a/Src/Newtonsoft.Json/Properties/AssemblyInfo.cs b/Src/Newtonsoft.Json/Properties/AssemblyInfo.cs index 2f760e90..a914e2aa 100644 --- a/Src/Newtonsoft.Json/Properties/AssemblyInfo.cs +++ b/Src/Newtonsoft.Json/Properties/AssemblyInfo.cs @@ -90,7 +90,7 @@ using System.Security; // by using the '*' as shown below: [assembly: AssemblyVersion("4.5.0.0")] #if !PocketPC -[assembly: AssemblyFileVersion("4.5.5.14913")] +[assembly: AssemblyFileVersion("4.5.5.14917")] #endif [assembly: CLSCompliant(true)] diff --git a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs b/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs index d4bf1d9d..a83b52cb 100644 --- a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs +++ b/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs @@ -106,7 +106,7 @@ namespace Newtonsoft.Json.Serialization return Serializer.ContractResolver.ResolveContract(type); } - public object Deserialize(JsonReader reader, Type objectType) + public object Deserialize(JsonReader reader, Type objectType, bool checkAdditionalContent) { if (reader == null) throw new ArgumentNullException("reader"); @@ -125,16 +125,26 @@ namespace Newtonsoft.Json.Serialization return null; } - if (converter != null && converter.CanRead) - return converter.ReadJson(reader, objectType, null, GetInternalSerializer()); + object deserializedValue; - return CreateValueInternal(reader, objectType, contract, null, null, null, null); + if (converter != null && converter.CanRead) + deserializedValue = converter.ReadJson(reader, objectType, null, GetInternalSerializer()); + else + deserializedValue = CreateValueInternal(reader, objectType, contract, null, null, null, null); + + if (checkAdditionalContent) + { + if (reader.Read() && reader.TokenType != JsonToken.Comment) + throw new JsonSerializationException("Additional text found in JSON string after finishing deserializing object."); + } + + return deserializedValue; } catch (Exception ex) { if (IsErrorHandled(null, contract, null, reader.Path, ex)) { - HandleError(reader, 0); + HandleError(reader, false, 0); return null; } else @@ -793,7 +803,7 @@ To force JSON arrays to deserialize add the JsonArrayAttribute to the type.".For catch (Exception ex) { if (IsErrorHandled(dictionary, contract, keyValue, reader.Path, ex)) - HandleError(reader, initialDepth); + HandleError(reader, true, initialDepth); else throw; } @@ -829,11 +839,12 @@ To force JSON arrays to deserialize add the JsonArrayAttribute to the type.".For contract.InvokeOnDeserializing(list, Serializer.Context); int initialDepth = reader.Depth; - int index = 0; JsonContract collectionItemContract = GetContractSafe(contract.CollectionItemType); JsonConverter collectionItemConverter = GetConverter(collectionItemContract, null, contract, containerProperty); + int? previousErrorIndex = null; + while (true) { try @@ -867,14 +878,27 @@ To force JSON arrays to deserialize add the JsonArrayAttribute to the type.".For } catch (Exception ex) { - if (IsErrorHandled(list, contract, index, reader.Path, ex)) - HandleError(reader, initialDepth); + JsonPosition errorPosition = reader.GetPosition(initialDepth); + + if (IsErrorHandled(list, contract, errorPosition.Position, reader.Path, ex)) + { + HandleError(reader, true, initialDepth); + + if (previousErrorIndex != null && previousErrorIndex == errorPosition.Position) + { + // reader index has not moved since previous error handling + // break out of reading array to prevent infinite loop + throw JsonSerializationException.Create(reader, "Infinite loop detected from error handling.", ex); + } + else + { + previousErrorIndex = errorPosition.Position; + } + } else + { throw; - } - finally - { - index++; + } } } @@ -997,7 +1021,7 @@ To fix this error either change the environment to be fully trusted, change the catch (Exception ex) { if (IsErrorHandled(newObject, contract, memberName, reader.Path, ex)) - HandleError(reader, initialDepth); + HandleError(reader, true, initialDepth); else throw; } @@ -1297,7 +1321,7 @@ To fix this error either change the environment to be fully trusted, change the catch (Exception ex) { if (IsErrorHandled(newObject, contract, memberName, reader.Path, ex)) - HandleError(reader, initialDepth); + HandleError(reader, true, initialDepth); else throw; } @@ -1357,7 +1381,7 @@ To fix this error either change the environment to be fully trusted, change the catch (Exception ex) { if (IsErrorHandled(newObject, contract, property.PropertyName, reader.Path, ex)) - HandleError(reader, initialDepth); + HandleError(reader, true, initialDepth); else throw; } @@ -1376,15 +1400,18 @@ To fix this error either change the environment to be fully trusted, change the } } - private void HandleError(JsonReader reader, int initialDepth) + private void HandleError(JsonReader reader, bool readPasteError, int initialDepth) { ClearErrorContext(); - reader.Skip(); - - while (reader.Depth > (initialDepth + 1)) + if (readPasteError) { - reader.Read(); + reader.Skip(); + + while (reader.Depth > (initialDepth + 1)) + { + reader.Read(); + } } } diff --git a/Src/Newtonsoft.Json/Serialization/JsonSerializerProxy.cs b/Src/Newtonsoft.Json/Serialization/JsonSerializerProxy.cs index 96a99542..779f80a7 100644 --- a/Src/Newtonsoft.Json/Serialization/JsonSerializerProxy.cs +++ b/Src/Newtonsoft.Json/Serialization/JsonSerializerProxy.cs @@ -162,6 +162,12 @@ namespace Newtonsoft.Json.Serialization set { _serializer.MaxDepth = value; } } + public override bool CheckAdditionalContent + { + get { return _serializer.CheckAdditionalContent; } + set { _serializer.CheckAdditionalContent = value; } + } + internal JsonSerializerInternalBase GetInternalSerializer() { if (_serializerReader != null) @@ -189,7 +195,7 @@ namespace Newtonsoft.Json.Serialization internal override object DeserializeInternal(JsonReader reader, Type objectType) { if (_serializerReader != null) - return _serializerReader.Deserialize(reader, objectType); + return _serializerReader.Deserialize(reader, objectType, false); else return _serializer.Deserialize(reader, objectType); }