Skip to content

Conversation

Rick-Anderson
Copy link
Contributor

@Rick-Anderson Rick-Anderson commented May 11, 2025

@Rick-Anderson Rick-Anderson requested a review from mikekistler May 12, 2025 00:11
@Rick-Anderson
Copy link
Contributor Author

@mikekistler pls search for and review <!-- comments. I'll delete them after you approve. Once this is approved I'll apply most of it to #35411

Copy link
Contributor

@mikekistler mikekistler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Pls check the one wonky line I noted -- otherwise this is good to go!

dotnet add package Microsoft.AspNetCore.JsonPatch.SystemTextJson --prerelease
```

This package provides a `JsonPatchDocument<T>` class to represent a JSON Patch document for objects of type `T` and custom logic for serializing and deserializing JSON Patch documents using `System.Text.Json`. The key method of the `JsonPatchDocument<T>` class is `ApplyTo`, which applies the patch operations to a target object of type `T`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the reference docs for Preview 4 are published, I think we should change the "code-style" references to .NET types to link to the ref docs.

> [!IMPORTANT]
> ***This is not an exhaustive list of threats.*** app developers must conduct their own threat model reviews to determine an app-specific comprehensive list and come up with appropriate mitigations as needed. For example, apps which expose collections to patch operations should consider the potential for algorithmic complexity attacks if those operations insert or remove elements at the beginning of the collection.

By running comprehensive threat models for their own apps and addressing identified threats while following the recommended mitigations below, consumers of these packages can <!-- review removing safely --> integrate JSON Patch functionality into their apps while minimizing security risks.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

* **Scenario**: A malicious client submits a `copy` operation that duplicates large object graphs multiple times, leading to excessive memory consumption.
* **Impact**: Potential Out-Of-Memory (OOM) conditions, causing service disruptions.
* **Mitigation**:
* Validate incoming JSON Patch documents for size and structure <!-- review my removing: before applying the document --> before calling `ApplyTo`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@Rick-Anderson Rick-Anderson enabled auto-merge (squash) May 12, 2025 15:01
@Rick-Anderson Rick-Anderson merged commit 0164e63 into main May 12, 2025
3 checks passed
@Rick-Anderson Rick-Anderson deleted the JasonPatch/Sys.txt/3 branch May 12, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WN 10.P4 New JsonPatch Implementation with System.Text.Json

2 participants