diff --git a/src/WebJobs.Extensions.DurableTask/ErrorSerializerSettingsFactory.cs b/src/WebJobs.Extensions.DurableTask/ErrorSerializerSettingsFactory.cs index e77bffeb5..ee51d1b14 100644 --- a/src/WebJobs.Extensions.DurableTask/ErrorSerializerSettingsFactory.cs +++ b/src/WebJobs.Extensions.DurableTask/ErrorSerializerSettingsFactory.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. See LICENSE in the project root for license information. using System; +using System.Collections.Generic; using System.Reflection; using Newtonsoft.Json; using Newtonsoft.Json.Serialization; @@ -30,6 +31,12 @@ public JsonSerializerSettings CreateJsonSerializerSettings() ContractResolver = new ExceptionResolver(), TypeNameHandling = TypeNameHandling.Objects, ReferenceLoopHandling = ReferenceLoopHandling.Ignore, + + // Limit the serialization depth to prevent StackOverflowException + // when serializing exceptions with complex/deeply nested structures. + // When this depth is exceeded, a JsonSerializationException is thrown + // which can be caught by the caller (unlike StackOverflowException). + MaxDepth = 64, }; } @@ -38,15 +45,33 @@ public JsonSerializerSettings CreateJsonSerializerSettings() private class ExceptionResolver : DefaultContractResolver { + // These are the well-known safe properties from System.Exception that we want to serialize. + // Other properties (especially from derived exception types like CsvHelper.FieldValidationException) + // may contain complex object graphs that can cause StackOverflowException during serialization. + private static readonly HashSet AllowedExceptionProperties = new HashSet(StringComparer.OrdinalIgnoreCase) + { + nameof(Exception.Message), + nameof(Exception.StackTrace), + nameof(Exception.Source), + nameof(Exception.HelpLink), + nameof(Exception.HResult), + nameof(Exception.Data), + nameof(Exception.InnerException), + }; + protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization) { JsonProperty property = base.CreateProperty(member, memberSerialization); - // Strip the TargetSite property from all exceptions - if (typeof(Exception).IsAssignableFrom(property.DeclaringType) && - property.PropertyName == nameof(Exception.TargetSite)) + // For Exception types and their derived classes, only serialize properties + // that are known to be safe. This prevents serialization of complex properties + // like CsvHelper's Context/ReadingContext that can cause StackOverflowException. + if (typeof(Exception).IsAssignableFrom(property.DeclaringType)) { - property.ShouldSerialize = _ => false; + if (!AllowedExceptionProperties.Contains(property.PropertyName)) + { + property.ShouldSerialize = _ => false; + } } return property; diff --git a/test/Common/ErrorSerializerSettingsFactoryTests.cs b/test/Common/ErrorSerializerSettingsFactoryTests.cs new file mode 100644 index 000000000..1400d0be1 --- /dev/null +++ b/test/Common/ErrorSerializerSettingsFactoryTests.cs @@ -0,0 +1,214 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +using System; +using Newtonsoft.Json; +using Xunit; + +namespace Microsoft.Azure.WebJobs.Extensions.DurableTask.Tests +{ + /// + /// Tests for ErrorSerializerSettingsFactory. + /// + public class ErrorSerializerSettingsFactoryTests + { + /// + /// Verifies that the error serializer settings have MaxDepth configured + /// to prevent StackOverflowException when serializing complex exceptions. + /// + [Fact] + public void CreateJsonSerializerSettings_HasMaxDepthConfigured() + { + // Arrange + var factory = new ErrorSerializerSettingsFactory(); + + // Act + var settings = factory.CreateJsonSerializerSettings(); + + // Assert + Assert.NotNull(settings.MaxDepth); + Assert.True(settings.MaxDepth > 0, "MaxDepth should be a positive value"); + } + + /// + /// Verifies that simple exceptions can still be serialized successfully. + /// + [Fact] + public void Serialize_SimpleException_Succeeds() + { + // Arrange + var factory = new ErrorSerializerSettingsFactory(); + var settings = factory.CreateJsonSerializerSettings(); + var simpleException = new InvalidOperationException("Test exception message"); + + // Act + var serialized = JsonConvert.SerializeObject(simpleException, settings); + + // Assert + Assert.NotNull(serialized); + Assert.Contains("Test exception message", serialized); + } + + /// + /// Verifies that exceptions with inner exceptions at a reasonable depth can be serialized. + /// + [Fact] + public void Serialize_ExceptionWithInnerExceptions_Succeeds() + { + // Arrange + var factory = new ErrorSerializerSettingsFactory(); + var settings = factory.CreateJsonSerializerSettings(); + + // Create an exception with a few levels of inner exceptions + var innerMost = new ArgumentException("Inner most exception"); + var middle = new InvalidOperationException("Middle exception", innerMost); + var outer = new Exception("Outer exception", middle); + + // Act + var serialized = JsonConvert.SerializeObject(outer, settings); + + // Assert + Assert.NotNull(serialized); + Assert.Contains("Outer exception", serialized); + Assert.Contains("Middle exception", serialized); + Assert.Contains("Inner most exception", serialized); + } + + /// + /// Verifies that the TargetSite property is excluded from serialization. + /// + [Fact] + public void Serialize_Exception_ExcludesTargetSite() + { + // Arrange + var factory = new ErrorSerializerSettingsFactory(); + var settings = factory.CreateJsonSerializerSettings(); + var exceptionWithTargetSite = CreateThrownException("Test exception"); + + // Verify that the exception has a TargetSite before serialization + Assert.NotNull(exceptionWithTargetSite.TargetSite); + + // Act + var serialized = JsonConvert.SerializeObject(exceptionWithTargetSite, settings); + + // Assert - TargetSite should not appear as a property key in the JSON + Assert.NotNull(serialized); + + // The serialized JSON shouldn't have "TargetSite" followed by a colon (as a property) + Assert.DoesNotContain("\"TargetSite\":", serialized); + } + + /// + /// Verifies that ReferenceLoopHandling is set to Ignore. + /// + [Fact] + public void CreateJsonSerializerSettings_HasReferenceLoopHandlingIgnore() + { + // Arrange + var factory = new ErrorSerializerSettingsFactory(); + + // Act + var settings = factory.CreateJsonSerializerSettings(); + + // Assert + Assert.Equal(ReferenceLoopHandling.Ignore, settings.ReferenceLoopHandling); + } + + /// + /// Verifies that TypeNameHandling is set to Objects for proper exception type deserialization. + /// + [Fact] + public void CreateJsonSerializerSettings_HasTypeNameHandlingObjects() + { + // Arrange + var factory = new ErrorSerializerSettingsFactory(); + + // Act + var settings = factory.CreateJsonSerializerSettings(); + + // Assert + Assert.Equal(TypeNameHandling.Objects, settings.TypeNameHandling); + } + + /// + /// Verifies that core exception properties are serialized. + /// + [Fact] + public void Serialize_Exception_IncludesAllowedProperties() + { + // Arrange + var factory = new ErrorSerializerSettingsFactory(); + var settings = factory.CreateJsonSerializerSettings(); + var exceptionWithStackTrace = CreateThrownException("Test message"); + + // Act + var serialized = JsonConvert.SerializeObject(exceptionWithStackTrace, settings); + + // Assert - core properties should be present + Assert.Contains("Message", serialized); + Assert.Contains("Test message", serialized); + } + + /// + /// Verifies that exceptions with custom properties that could cause + /// serialization issues are safely serialized. + /// + [Fact] + public void Serialize_ExceptionWithDangerousProperty_SucceedsWithoutIncludingDangerousProperty() + { + // Arrange + var factory = new ErrorSerializerSettingsFactory(); + var settings = factory.CreateJsonSerializerSettings(); + + // Create an exception with a property that could cause serialization issues + var dangerousException = new ExceptionWithProblematicProperty("Test exception"); + + // Act - this should succeed without StackOverflowException + var serialized = JsonConvert.SerializeObject(dangerousException, settings); + + // Assert + Assert.NotNull(serialized); + Assert.Contains("Test exception", serialized); + + // The dangerous property should NOT be serialized as a JSON property (look for the colon pattern) + Assert.DoesNotContain("\"SelfReference\":", serialized); + Assert.DoesNotContain("\"ProblematicProperty\":", serialized); + } + + /// + /// Creates an exception that has been thrown and caught, ensuring it has + /// populated TargetSite and StackTrace properties. + /// + /// The exception message. + /// An exception with populated stack trace information. + private static Exception CreateThrownException(string message) + { + try + { + throw new InvalidOperationException(message); + } + catch (Exception ex) + { + return ex; + } + } + + /// + /// A test exception with a property that would cause serialization issues. + /// + private class ExceptionWithProblematicProperty : Exception + { + public ExceptionWithProblematicProperty(string message) + : base(message) + { + // Self-referential property that would cause infinite recursion + this.SelfReference = this; + } + + public Exception SelfReference { get; set; } + + // A property with a complex object graph + public object ProblematicProperty => new { Nested = new { More = new { AndMore = this } } }; + } + } +}