fix(normalizr): Use Object.keys() in deepClone to avoid inherited properties#3875
Merged
fix(normalizr): Use Object.keys() in deepClone to avoid inherited properties#3875
Conversation
…perties for...in iterates inherited properties, which could copy polluted Object.prototype entries. Object.keys() restricts to own enumerable properties only. Made-with: Cursor
🦋 Changeset detectedLatest commit: ad23e37 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Made-with: Cursor
Contributor
|
Size Change: 0 B Total Size: 80.6 kB ℹ️ View Unchanged
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Benchmark React
Details
| Benchmark suite | Current: ad23e37 | Previous: be8f8d0 | Ratio |
|---|---|---|---|
data-client: getlist-100 |
169.54 ops/s (± 4.8%) |
170.95 ops/s (± 5.4%) |
1.01 |
data-client: getlist-500 |
45.66 ops/s (± 5.4%) |
48.78 ops/s (± 5.6%) |
1.07 |
data-client: update-entity |
444.66 ops/s (± 6.5%) |
500 ops/s (± 3.8%) |
1.12 |
data-client: update-user |
370.37 ops/s (± 5.3%) |
500 ops/s (± 6.0%) |
1.35 |
data-client: getlist-500-sorted |
50.76 ops/s (± 2.7%) |
49.26 ops/s (± 7.4%) |
0.97 |
data-client: update-entity-sorted |
370.37 ops/s (± 6.2%) |
384.62 ops/s (± 6.5%) |
1.04 |
data-client: update-entity-multi-view |
377.49 ops/s (± 6.5%) |
370.37 ops/s (± 5.1%) |
0.98 |
data-client: list-detail-switch-10 |
13.55 ops/s (± 9.7%) |
12.87 ops/s (± 6.1%) |
0.95 |
data-client: update-user-10000 |
90.91 ops/s (± 0.7%) |
104.17 ops/s (± 5.9%) |
1.15 |
data-client: invalidate-and-resolve |
50.51 ops/s (± 5.2%) |
50.38 ops/s (± 5.9%) |
1.00 |
data-client: unshift-item |
303.03 ops/s (± 4.1%) |
281.75 ops/s (± 5.3%) |
0.93 |
data-client: delete-item |
370.37 ops/s (± 5.4%) |
400 ops/s (± 3.3%) |
1.08 |
data-client: move-item |
208.33 ops/s (± 8.8%) |
232.56 ops/s (± 5.8%) |
1.12 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3875 +/- ##
=======================================
Coverage 98.10% 98.10%
=======================================
Files 153 153
Lines 2899 2899
Branches 564 564
=======================================
Hits 2844 2844
Misses 11 11
Partials 44 44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: ad23e37 | Previous: 53c7091 | Ratio |
|---|---|---|---|
normalizeLong |
434 ops/sec (±2.38%) |
459 ops/sec (±0.91%) |
1.06 |
normalizeLong Values |
403 ops/sec (±0.20%) |
407 ops/sec (±0.42%) |
1.01 |
denormalizeLong |
277 ops/sec (±2.71%) |
275 ops/sec (±3.25%) |
0.99 |
denormalizeLong Values |
249 ops/sec (±2.39%) |
242 ops/sec (±2.61%) |
0.97 |
denormalizeLong donotcache |
967 ops/sec (±0.19%) |
1023 ops/sec (±0.15%) |
1.06 |
denormalizeLong Values donotcache |
710 ops/sec (±0.31%) |
731 ops/sec (±0.44%) |
1.03 |
denormalizeShort donotcache 500x |
1569 ops/sec (±0.12%) |
1562 ops/sec (±0.10%) |
1.00 |
denormalizeShort 500x |
825 ops/sec (±2.46%) |
833 ops/sec (±2.01%) |
1.01 |
denormalizeShort 500x withCache |
6322 ops/sec (±0.23%) |
5711 ops/sec (±0.18%) |
0.90 |
queryShort 500x withCache |
2719 ops/sec (±0.09%) |
2666 ops/sec (±0.11%) |
0.98 |
buildQueryKey All |
53921 ops/sec (±0.51%) |
54203 ops/sec (±1.19%) |
1.01 |
query All withCache |
6332 ops/sec (±0.18%) |
6508 ops/sec (±0.22%) |
1.03 |
denormalizeLong with mixin Entity |
267 ops/sec (±2.18%) |
276 ops/sec (±2.50%) |
1.03 |
denormalizeLong withCache |
6490 ops/sec (±0.17%) |
8046 ops/sec (±0.22%) |
1.24 |
denormalizeLong Values withCache |
5003 ops/sec (±0.41%) |
5066 ops/sec (±0.36%) |
1.01 |
denormalizeLong All withCache |
6246 ops/sec (±0.19%) |
6175 ops/sec (±0.14%) |
0.99 |
denormalizeLong Query-sorted withCache |
6511 ops/sec (±0.13%) |
6487 ops/sec (±0.18%) |
1.00 |
denormalizeLongAndShort withEntityCacheOnly |
1621 ops/sec (±0.23%) |
1794 ops/sec (±0.18%) |
1.11 |
denormalize bidirectional 50 |
5676 ops/sec (±2.04%) |
5716 ops/sec (±1.95%) |
1.01 |
denormalize bidirectional 50 donotcache |
41117 ops/sec (±0.66%) |
40068 ops/sec (±0.51%) |
0.97 |
getResponse |
4616 ops/sec (±0.60%) |
4638 ops/sec (±0.56%) |
1.00 |
getResponse (null) |
10250207 ops/sec (±0.59%) |
10565574 ops/sec (±1.77%) |
1.03 |
getResponse (clear cache) |
255 ops/sec (±2.07%) |
261 ops/sec (±2.10%) |
1.02 |
getSmallResponse |
3346 ops/sec (±0.14%) |
3295 ops/sec (±0.13%) |
0.98 |
getSmallInferredResponse |
2419 ops/sec (±0.39%) |
2468 ops/sec (±0.09%) |
1.02 |
getResponse Collection |
4485 ops/sec (±0.38%) |
4545 ops/sec (±0.59%) |
1.01 |
get Collection |
4461 ops/sec (±0.27%) |
4617 ops/sec (±0.21%) |
1.03 |
get Query-sorted |
5299 ops/sec (±0.14%) |
5261 ops/sec (±0.29%) |
0.99 |
setLong |
451 ops/sec (±0.24%) |
468 ops/sec (±0.23%) |
1.04 |
setLongWithMerge |
242 ops/sec (±0.34%) |
261 ops/sec (±0.23%) |
1.08 |
setLongWithSimpleMerge |
254 ops/sec (±0.48%) |
276 ops/sec (±0.22%) |
1.09 |
setSmallResponse 500x |
925 ops/sec (±0.06%) |
939 ops/sec (±0.15%) |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
This was referenced Apr 6, 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
1 participant
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.
Motivation
deepClonein the immutable normalize path usesfor...in, which iterates both own and inherited enumerable properties. IfObject.prototypewere ever polluted (e.g., by a third-party library), those properties would be silently copied into cloned store state, leading to subtle data corruption.Solution
Replace
for (const key in obj)withfor (const key of Object.keys(obj))to restrict iteration to own enumerable properties only. This is a defensive hardening with no behavioral change under normal conditions.Open questions
N/A
Made with Cursor
Note
Low Risk
Small defensive change limited to
deepCloneiteration semantics plus a changeset; low risk with behavior only differing when inherited enumerable properties exist (e.g., prototype pollution).Overview
Hardens the immutable normalize path by updating
deepCloneto iterate viaObject.keys()instead offor...in, so only own enumerable properties are cloned and inherited properties can’t leak into normalized state.Adds a patch changeset for
@data-client/normalizrdocumenting the fix.Reviewed by Cursor Bugbot for commit ad23e37. Bugbot is set up for automated code reviews on this repo. Configure here.