Add TraceContextSerializationBinder to constrain deserialization#1346
Add TraceContextSerializationBinder to constrain deserialization#1346
Conversation
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens JSON deserialization for trace-context payloads carried with orchestration messages by introducing a dedicated serialization binder that constrains allowed $type values.
Changes:
- Add
TraceContextSerializationBinderto allowlistTraceContextBase(and subclasses) during (de)serialization. - Wire the new binder into
TraceContextBase’sJsonSerializerSettings. - Add tests intended to validate rejection of non-allowlisted
$typevalues and a valid W3C trace-context round-trip.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/DurableTask.Core/TraceContextBase.cs | Configures the trace-context serializer to use the new constrained binder. |
| src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs | Implements a strict binder allowlisting TraceContextBase/subclasses (and Stack<TraceContextBase>). |
| Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs | Adds tests for allowlist enforcement and valid deserialization paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| namespace DurableTask.Core.Tests | ||
| { | ||
| using System; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
|
|
||
| [TestClass] | ||
| public class TraceContextSerializationBinderTests | ||
| { | ||
| [TestMethod] |
There was a problem hiding this comment.
This test file is added under the legacy Test/ folder, but the solution and active test project are under test/DurableTask.Core.Tests (lowercase) and do not include sources from Test/. As a result, these tests won't be compiled/executed in CI. Move this file into test/DurableTask.Core.Tests/ (or update the referenced test csproj to include it) so the new binder behavior is actually covered by the test suite.
Resolves https://msazure.visualstudio.com/Antares/_workitems/edit/37181650