From e668b6ed1b9d485a748b104b1ded9f98b36212c3 Mon Sep 17 00:00:00 2001 From: Brian Ritchie Date: Sun, 19 May 2013 14:34:47 -0400 Subject: [PATCH 1/2] Track visited objects during serialization to prevent stack overflows by getting stuck in endless loops --- .../Common/SerializationVisitorTracker.cs | 35 ++++++++++ src/ServiceStack.Text/Common/WriteType.cs | 10 ++- .../ServiceStack.Text.csproj | 1 + src/ServiceStack.Text/TypeSerializer.cs | 66 ++++++++++++++++--- 4 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 src/ServiceStack.Text/Common/SerializationVisitorTracker.cs diff --git a/src/ServiceStack.Text/Common/SerializationVisitorTracker.cs b/src/ServiceStack.Text/Common/SerializationVisitorTracker.cs new file mode 100644 index 000000000..91a474ea9 --- /dev/null +++ b/src/ServiceStack.Text/Common/SerializationVisitorTracker.cs @@ -0,0 +1,35 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; + +namespace ServiceStack.Text.Common +{ + internal static class SerializationVisitorTracker + { + static Dictionary> VisitedCache = new Dictionary>(); + + internal static void TrackSerialization(TextWriter writer) + { + VisitedCache.Add(writer, new List()); + } + + internal static void UnTrackSerialization(TextWriter writer) + { + VisitedCache.Remove(writer); + } + + internal static bool HasVisited(TextWriter writer, object o) + { + return VisitedCache[writer].Exists(x => Object.ReferenceEquals(x, o)); + } + + internal static void Track(TextWriter writer, object o) + { + VisitedCache[writer].Add(o); + } + } + + +} diff --git a/src/ServiceStack.Text/Common/WriteType.cs b/src/ServiceStack.Text/Common/WriteType.cs index d1e1bf7c0..ffe6b9c88 100644 --- a/src/ServiceStack.Text/Common/WriteType.cs +++ b/src/ServiceStack.Text/Common/WriteType.cs @@ -16,6 +16,7 @@ using System.Reflection; using ServiceStack.Text.Json; using ServiceStack.Text.Reflection; +using System.Collections.Generic; namespace ServiceStack.Text.Common { @@ -281,9 +282,11 @@ public static void WriteProperties(TextWriter writer, object value) ? propertyWriter.GetterFn((T)value) : null; - if ((propertyValue == null - || (propertyWriter.DefaultValue != null && propertyWriter.DefaultValue.Equals(propertyValue))) - && !Serializer.IncludeNullValues) continue; + if ( ((propertyValue == null + || (propertyWriter.DefaultValue != null && propertyWriter.DefaultValue.Equals(propertyValue))) + && !Serializer.IncludeNullValues) + || (propertyValue != null && SerializationVisitorTracker.HasVisited(writer,propertyValue)) ) + continue; if (i++ > 0) writer.Write(JsWriter.ItemSeperator); @@ -298,6 +301,7 @@ public static void WriteProperties(TextWriter writer, object value) } else { + SerializationVisitorTracker.Track(writer, propertyValue); propertyWriter.WriteFn(writer, propertyValue); } if (typeof(TSerializer) == typeof(JsonTypeSerializer)) JsState.IsWritingValue = false; diff --git a/src/ServiceStack.Text/ServiceStack.Text.csproj b/src/ServiceStack.Text/ServiceStack.Text.csproj index d1b95a193..309020b3b 100644 --- a/src/ServiceStack.Text/ServiceStack.Text.csproj +++ b/src/ServiceStack.Text/ServiceStack.Text.csproj @@ -139,6 +139,7 @@ Code + Code diff --git a/src/ServiceStack.Text/TypeSerializer.cs b/src/ServiceStack.Text/TypeSerializer.cs index e84f9717e..e91545569 100644 --- a/src/ServiceStack.Text/TypeSerializer.cs +++ b/src/ServiceStack.Text/TypeSerializer.cs @@ -91,7 +91,15 @@ public static string SerializeToString(T value) var sb = new StringBuilder(); using (var writer = new StringWriter(sb, CultureInfo.InvariantCulture)) { - JsvWriter.WriteObject(writer, value); + try + { + SerializationVisitorTracker.TrackSerialization(writer); + JsvWriter.WriteObject(writer, value); + } + finally + { + SerializationVisitorTracker.UnTrackSerialization(writer); + } } return sb.ToString(); } @@ -104,7 +112,15 @@ public static string SerializeToString(object value, Type type) var sb = new StringBuilder(); using (var writer = new StringWriter(sb, CultureInfo.InvariantCulture)) { - JsvWriter.GetWriteFn(type)(writer, value); + try + { + SerializationVisitorTracker.TrackSerialization(writer); + JsvWriter.GetWriteFn(type)(writer, value); + } + finally + { + SerializationVisitorTracker.UnTrackSerialization(writer); + } } return sb.ToString(); } @@ -125,8 +141,16 @@ public static void SerializeToWriter(T value, TextWriter writer) return; } - JsvWriter.WriteObject(writer, value); - } + try + { + SerializationVisitorTracker.TrackSerialization(writer); + JsvWriter.WriteObject(writer, value); + } + finally + { + SerializationVisitorTracker.UnTrackSerialization(writer); + } + } public static void SerializeToWriter(object value, Type type, TextWriter writer) { @@ -137,7 +161,15 @@ public static void SerializeToWriter(object value, Type type, TextWriter writer) return; } - JsvWriter.GetWriteFn(type)(writer, value); + try + { + SerializationVisitorTracker.TrackSerialization(writer); + JsvWriter.GetWriteFn(type)(writer, value); + } + finally + { + SerializationVisitorTracker.UnTrackSerialization(writer); + } } public static void SerializeToStream(T value, Stream stream) @@ -152,15 +184,31 @@ public static void SerializeToStream(T value, Stream stream) } var writer = new StreamWriter(stream, UTF8EncodingWithoutBom); - JsvWriter.WriteObject(writer, value); - writer.Flush(); + try + { + SerializationVisitorTracker.TrackSerialization(writer); + JsvWriter.WriteObject(writer, value); + } + finally + { + SerializationVisitorTracker.UnTrackSerialization(writer); + } + writer.Flush(); } public static void SerializeToStream(object value, Type type, Stream stream) { var writer = new StreamWriter(stream, UTF8EncodingWithoutBom); - JsvWriter.GetWriteFn(type)(writer, value); - writer.Flush(); + try + { + SerializationVisitorTracker.TrackSerialization(writer); + JsvWriter.GetWriteFn(type)(writer, value); + } + finally + { + SerializationVisitorTracker.UnTrackSerialization(writer); + } + writer.Flush(); } public static T Clone(T value) From 95f123d4ce3f9122084e71e78078e1f381abf183 Mon Sep 17 00:00:00 2001 From: Brian Ritchie Date: Tue, 21 May 2013 07:03:57 -0400 Subject: [PATCH 2/2] Added locking to make it thread safe --- .../Common/SerializationVisitorTracker.cs | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/ServiceStack.Text/Common/SerializationVisitorTracker.cs b/src/ServiceStack.Text/Common/SerializationVisitorTracker.cs index 91a474ea9..a6e236127 100644 --- a/src/ServiceStack.Text/Common/SerializationVisitorTracker.cs +++ b/src/ServiceStack.Text/Common/SerializationVisitorTracker.cs @@ -12,22 +12,34 @@ internal static class SerializationVisitorTracker internal static void TrackSerialization(TextWriter writer) { - VisitedCache.Add(writer, new List()); + lock (VisitedCache) + { + VisitedCache.Add(writer, new List()); + } } internal static void UnTrackSerialization(TextWriter writer) { - VisitedCache.Remove(writer); + lock (VisitedCache) + { + VisitedCache.Remove(writer); + } } internal static bool HasVisited(TextWriter writer, object o) { - return VisitedCache[writer].Exists(x => Object.ReferenceEquals(x, o)); + lock (VisitedCache) + { + return VisitedCache[writer].Exists(x => Object.ReferenceEquals(x, o)); + } } internal static void Track(TextWriter writer, object o) { - VisitedCache[writer].Add(o); + lock (VisitedCache) + { + VisitedCache[writer].Add(o); + } } }