From 35c786c5bea9723bc18d33654332e72b01332204 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 22:26:32 +0000 Subject: [PATCH 1/3] Initial plan From f0a0b610503f92fc50f4d8199772ff3e394232dc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 22:41:27 +0000 Subject: [PATCH 2/3] Fix StackOverflowException when serializing complex exceptions Modified ErrorSerializerSettingsFactory to only serialize safe, well-known exception properties (Message, StackTrace, Source, HelpLink, HResult, Data, InnerException). This prevents serialization of complex properties from derived exception types like CsvHelper.FieldValidationException that can cause infinite recursion and StackOverflowException during JSON serialization. Also added MaxDepth=64 as an additional safety measure. Fixes #550 Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com> --- .../ErrorSerializerSettingsFactory.cs | 33 ++- .../ErrorSerializerSettingsFactoryTests.cs | 215 ++++++++++++++++++ 2 files changed, 244 insertions(+), 4 deletions(-) create mode 100644 test/Common/ErrorSerializerSettingsFactoryTests.cs 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..8090eda0b --- /dev/null +++ b/test/Common/ErrorSerializerSettingsFactoryTests.cs @@ -0,0 +1,215 @@ +// 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(); + + // Create an exception that has a TargetSite + Exception exceptionWithTargetSite; + try + { + throw new InvalidOperationException("Test exception"); + } + catch (Exception ex) + { + exceptionWithTargetSite = ex; + } + + // 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(); + + Exception exceptionWithStackTrace; + try + { + throw new InvalidOperationException("Test message"); + } + catch (Exception ex) + { + exceptionWithStackTrace = ex; + } + + // 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); + } + + /// + /// 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 } } }; + } + } +} From 59a59661a58b04ed5e677ec93f2024a7618477df Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 22:47:04 +0000 Subject: [PATCH 3/3] Refactor test code to extract helper method Extracted CreateThrownException helper method to reduce code duplication in ErrorSerializerSettingsFactoryTests. Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com> --- .../ErrorSerializerSettingsFactoryTests.cs | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/test/Common/ErrorSerializerSettingsFactoryTests.cs b/test/Common/ErrorSerializerSettingsFactoryTests.cs index 8090eda0b..1400d0be1 100644 --- a/test/Common/ErrorSerializerSettingsFactoryTests.cs +++ b/test/Common/ErrorSerializerSettingsFactoryTests.cs @@ -83,17 +83,7 @@ public void Serialize_Exception_ExcludesTargetSite() // Arrange var factory = new ErrorSerializerSettingsFactory(); var settings = factory.CreateJsonSerializerSettings(); - - // Create an exception that has a TargetSite - Exception exceptionWithTargetSite; - try - { - throw new InvalidOperationException("Test exception"); - } - catch (Exception ex) - { - exceptionWithTargetSite = ex; - } + var exceptionWithTargetSite = CreateThrownException("Test exception"); // Verify that the exception has a TargetSite before serialization Assert.NotNull(exceptionWithTargetSite.TargetSite); @@ -149,16 +139,7 @@ public void Serialize_Exception_IncludesAllowedProperties() // Arrange var factory = new ErrorSerializerSettingsFactory(); var settings = factory.CreateJsonSerializerSettings(); - - Exception exceptionWithStackTrace; - try - { - throw new InvalidOperationException("Test message"); - } - catch (Exception ex) - { - exceptionWithStackTrace = ex; - } + var exceptionWithStackTrace = CreateThrownException("Test message"); // Act var serialized = JsonConvert.SerializeObject(exceptionWithStackTrace, settings); @@ -194,6 +175,24 @@ public void Serialize_ExceptionWithDangerousProperty_SucceedsWithoutIncludingDan 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. ///