Fix ShallowCopy DateFormatString side effect overriding DateFormatHandling

[kubaflo]

🤖 AI Summary

🔍 Automated Fix Report

---

🔍 Pre-Flight — Context & Validation

Issue: #29532 — Newtonsoft ShallowCopy sets DateFormatString side effect overriding DateFormatHandling

Area: src/Mvc/Mvc.NewtonsoftJson/

Root Cause: NewtonsoftJsonOutputFormatter.ShallowCopy copies DateFormatString via the property setter, which triggers an internal _dateFormatStringSet flag in Newtonsoft.Json's JsonSerializerSettings. When this flag is set, DateFormatString takes precedence over DateFormatHandling — so users who set DateFormatHandling.MicrosoftDateFormat still get ISO 8601 output.

Classification: Bug — unintended side effect from property copy

---

🧪 Test — Bug Reproduction

Test File: src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs

Test Added: DateFormatHandling_IsRespected_WhenDateFormatStringNotExplicitlySet — verifies that DateFormatHandling.MicrosoftDateFormat produces \/Date(...)\/ output when DateFormatString is not explicitly set.

Strategy: Created settings with only DateFormatHandling = MicrosoftDateFormat, formatted a date object, and asserted the output contains the Microsoft date format pattern.

---

🚦 Gate — Test Verification & Regression

Gate Result: ✅ All 250 NewtonsoftJson tests pass

Test Command:

dotnet test src/Mvc/Mvc.NewtonsoftJson/test/Microsoft.AspNetCore.Mvc.NewtonsoftJson.Test.csproj --no-restore -v q

Regression: No failures in existing test suite.

---

🔧 Fix — Analysis & Comparison (✅ 3 passed)

Fix: Avoid triggering _dateFormatStringSet flag during ShallowCopy when user hasn't explicitly set DateFormatString.

Attempt	Approach	Result
0	Reflection check on _dateFormatStringSet field	✅ Pass
1	Omit DateFormatString from copy entirely	✅ Pass (but loses custom values)
2	Compare to cached default, only copy when different	✅ Pass

✅ Attempt 0: PASS

Approach: Reflection check on _dateFormatStringSet field, conditionally copy DateFormatString.

Added static FieldInfo for the internal _dateFormatStringSet field. In ShallowCopy, removed DateFormatString from initializer and only copy it when the field is true (user explicitly set it).

📄 Diff

diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
index faa2909506..3deb9ec8c7 100644
--- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
+++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Buffers;
+using System.Reflection;
 using System.Text;
 using Microsoft.AspNetCore.Mvc.NewtonsoftJson;
 using Microsoft.AspNetCore.WebUtilities;
@@ -190,6 +191,9 @@ public partial class NewtonsoftJsonOutputFormatter : TextOutputFormatter
         }
     }
 
+    private static readonly FieldInfo? DateFormatStringSetField =
+        typeof(JsonSerializerSettings).GetField("_dateFormatStringSet", BindingFlags.Instance | BindingFlags.NonPublic);
+
     private static JsonSerializerSettings ShallowCopy(JsonSerializerSettings settings)
     {
         var copiedSettings = new JsonSerializerSettings
@@ -201,7 +205,6 @@ public partial class NewtonsoftJsonOutputFormatter : TextOutputFormatter
             DateFormatHandling = settings.DateFormatHandling,
             Formatting = settings.Formatting,
             MaxDepth = settings.MaxDepth,
-            DateFormatString = settings.DateFormatString,
             Context = settings.Context,
             Error = settings.Error,
             SerializationBinder = settings.SerializationBinder,
@@ -225,6 +228,15 @@ public partial class NewtonsoftJsonOutputFormatter : TextOutputFormatter
             StringEscapeHandling = settings.StringEscapeHandling,
         };
 
+        // Only copy DateFormatString when it was explicitly set by the user.
+        // The JsonSerializerSettings.DateFormatString setter marks an internal
+        // _dateFormatStringSet flag that causes DateFormatString to take precedence
+        // over DateFormatHandling, even when the value is the default.
+        if (DateFormatStringSetField?.GetValue(settings) is true)
+        {
+            copiedSettings.DateFormatString = settings.DateFormatString;
+        }
+
         return copiedSettings;
     }
 

✅ Attempt 1: PASS

Approach: Simply omit DateFormatString from ShallowCopy entirely.

Remove DateFormatString = settings.DateFormatString from the initializer. The default value is the same ISO 8601 format, so this works for the common case. Caveat: if a user explicitly sets a custom DateFormatString, it won't be copied to the shallow copy. All 250 tests pass.

📄 Diff

diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
index faa2909506..a303a6894c 100644
--- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
+++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
@@ -201,7 +201,9 @@ public partial class NewtonsoftJsonOutputFormatter : TextOutputFormatter
             DateFormatHandling = settings.DateFormatHandling,
             Formatting = settings.Formatting,
             MaxDepth = settings.MaxDepth,
-            DateFormatString = settings.DateFormatString,
+            // DateFormatString intentionally omitted to avoid triggering
+            // the internal _dateFormatStringSet flag which overrides DateFormatHandling.
+            // The default value is already "yyyy'-'MM'-'dd'T'HH':'mm':'ss.FFFFFFFK".
             Context = settings.Context,
             Error = settings.Error,
             SerializationBinder = settings.SerializationBinder,

✅ Attempt 2: PASS

Approach: Compare DateFormatString to a cached default value, only copy when different.

Created DefaultDateFormatString static field from new JsonSerializerSettings().DateFormatString. In ShallowCopy, only copy DateFormatString via property setter when it differs from the default. This avoids reflection while correctly preserving custom format strings.

📄 Diff

diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
index faa2909506..574e23b82c 100644
--- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
+++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs
@@ -190,6 +190,8 @@ public partial class NewtonsoftJsonOutputFormatter : TextOutputFormatter
         }
     }
 
+    private static readonly string DefaultDateFormatString = new JsonSerializerSettings().DateFormatString;
+
     private static JsonSerializerSettings ShallowCopy(JsonSerializerSettings settings)
     {
         var copiedSettings = new JsonSerializerSettings
@@ -201,7 +203,6 @@ public partial class NewtonsoftJsonOutputFormatter : TextOutputFormatter
             DateFormatHandling = settings.DateFormatHandling,
             Formatting = settings.Formatting,
             MaxDepth = settings.MaxDepth,
-            DateFormatString = settings.DateFormatString,
             Context = settings.Context,
             Error = settings.Error,
             SerializationBinder = settings.SerializationBinder,
@@ -225,6 +226,14 @@ public partial class NewtonsoftJsonOutputFormatter : TextOutputFormatter
             StringEscapeHandling = settings.StringEscapeHandling,
         };
 
+        // Only copy DateFormatString when it differs from the default.
+        // Using the property setter triggers an internal _dateFormatStringSet flag
+        // that causes DateFormatString to override DateFormatHandling.
+        if (!string.Equals(settings.DateFormatString, DefaultDateFormatString, StringComparison.Ordinal))
+        {
+            copiedSettings.DateFormatString = settings.DateFormatString;
+        }
+
         return copiedSettings;
     }
 

[dotnet-policy-service]

Thanks for your PR, @@kubaflo. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[Copilot]

Pull request overview

This PR fixes a long-standing bug (issue #29532) in NewtonsoftJsonOutputFormatter.ShallowCopy: assigning DateFormatString through the property setter (even to its default value) sets an internal _dateFormatStringSet flag inside Newtonsoft.Json's JsonSerializerSettings, which causes DateFormatString to silently override DateFormatHandling. As a result, users who configure DateFormatHandling.MicrosoftDateFormat would get ISO 8601 output instead. The fix uses reflection to only copy DateFormatString when it was explicitly set. The PR also bundles several new agentic skill definition files for GitHub Actions automation.

Changes:

• NewtonsoftJsonOutputFormatter.cs: Uses reflection to check _dateFormatStringSet flag before copying DateFormatString in ShallowCopy
• NewtonsoftJsonOutputFormatterTest.cs: Adds regression test verifying DateFormatHandling.MicrosoftDateFormat is respected when DateFormatString is not set
• .github/skills/ files: Adds multiple new agentic workflow skill definitions and scripts for automated issue fixing

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs	Core fix: reflection-based conditional copy of DateFormatString in ShallowCopy
src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs	Regression test verifying DateFormatHandling.MicrosoftDateFormat produces Microsoft date format output
.github/skills/fix-issue/SKILL.md	New 5-phase agentic workflow skill for end-to-end issue fixing
.github/skills/write-tests/SKILL.md	New skill for creating unit tests for issues
.github/skills/verify-tests/SKILL.md	New skill for verifying tests catch bugs
.github/skills/try-fix/SKILL.md	New skill for attempting alternative fixes
.github/skills/ai-summary-comment/SKILL.md	New skill for posting automated progress comments on PRs
.github/skills/ai-summary-comment/scripts/post-ai-summary-comment.sh	Shell script that reads phase outputs and posts/updates a unified AI summary comment
.github/skills/fix-issue/tests/test-skill-definition.sh	Bash test script validating the fix-issue skill definition
.github/skills/fix-issue/tests/test-ai-summary-comment.sh	Bash test script validating the ai-summary-comment scripts