feat: protect final deserialization in PatchToDate with skip conditions#151
Open
feat: protect final deserialization in PatchToDate with skip conditions#151
Conversation
Problem: PatchToDate has two steps: (1) ApplyPatchesToDate applies patches one-by-one to a JToken — each protected by skip conditions, and (2) _deserialize() converts the accumulated JToken back to TEntity. Step 2 was NOT protected by skip conditions. Symptom: When the initial entity has a null navigation property (e.g. due to a missing EF Core Include), jsondiffpatch.net's Unpatch silently produces a garbled JToken — no error is thrown during patching. The configured skip conditions (e.g. IgnoreEverythingSkipCondition) are never invoked because no individual patch fails. The unprotected _deserialize() then crashes with a JsonException because the garbled JToken cannot be deserialized back into the target type (e.g. List<T> at a path that became a non-array). This was discovered in production (DEV-107694). Fix: - Wrap _deserialize() and _populateEntity() calls in both PatchToDate overloads with try-catch that consults skip conditions - When skip conditions allow skipping: return initialEntity as-is (the unpatched entity at +/- infinity) and set FinalDeserializationFailed = true - When no skip conditions match: rethrow as before (no behavior change) Consequences for servers using this library: - Without this fix: _deserialize() throws → unhandled exception → HTTP 500. - With this fix: _deserialize() is caught → initialEntity is returned. The server no longer crashes, but the returned entity is the CURRENT state (at +/- infinity), NOT the state at the requested keyDate. This is fachlich incorrect — the caller gets the wrong time slice. - The entity cannot be partially rescued: System.Text.Json deserializes the entire object or not at all, even though only one property path (e.g. konfigurationsprodukte) is garbled while the rest was correctly unpatched. - Servers SHOULD check chain.FinalDeserializationFailed after PatchToDate and log a warning, because the returned data is silently wrong. - The correct fix remains on the consumer side: ensure all navigation properties are properly loaded (e.g. via EF Core Include) so that the Unpatch operates on valid arrays instead of null. New API: - FinalDeserializationFailed (bool): true when the final deserialization was skipped. The returned entity is NOT the state at keyDate but the unpatched initial entity. - PatchesHaveBeenSkipped now also returns true when FinalDeserializationFailed is true. BREAKING CHANGE: ISkipCondition<TEntity>.ShouldSkipPatch parameter failedPatch changed from TimeRangePatch to TimeRangePatch? (nullable). The parameter is null when the error occurred during final deserialization (no single patch failed). If your skip condition only inspects the exception type, just update the method signature. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
JoschaMetze
approved these changes
Feb 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem:
PatchToDate has two steps: (1) ApplyPatchesToDate applies patches one-by-one to a JToken — each protected by skip conditions, and (2) _deserialize() converts the accumulated JToken back to TEntity. Step 2 was NOT protected by skip conditions.
Symptom:
When the initial entity has a null navigation property (e.g. due to a missing EF Core Include), jsondiffpatch.net's Unpatch silently produces a garbled JToken — no error is thrown during patching. The configured skip conditions (e.g. IgnoreEverythingSkipCondition) are never invoked because no individual patch fails. The unprotected _deserialize() then crashes with a JsonException because the garbled JToken cannot be deserialized back into the target type (e.g. List at a path that became a non-array). This was discovered in production (DEV-107694).
Fix:
Consequences for servers using this library:
New API:
BREAKING CHANGE: ISkipCondition.ShouldSkipPatch parameter failedPatch changed from TimeRangePatch to TimeRangePatch? (nullable). The parameter is null when the error occurred during final deserialization (no single patch failed). If your skip condition only inspects the exception type, just update the method signature.