fix: toJSONwithObjects leaks ParseACL instances when afterFind hooks modify objects with directAccess#10244
Conversation
…oks modify objects When `directAccess` is enabled, `toJSONwithObjects` iterates pending ops and calls `object.get(key)` for each dirty field. For ACL, this returns a live `ParseACL` instance. Since `ParseACL` lacks `_toFullJSON`, the instance was assigned directly to the response JSON. With `directAccess`, the response bypasses HTTP serialization, so the raw `ParseACL` leaks to the client SDK. The client's `ParseObject._finishFetch` then calls `new ParseACL(aclData)` expecting plain JSON but receiving a `ParseACL` instance, throwing: "TypeError: Tried to create an ACL with an invalid permission type." The fix calls `val.toJSON()` on values that support it before assigning, ensuring SDK types like `ParseACL` and `ParseGeoPoint` are serialized to plain JSON — consistent with what `object.toJSON()` already does. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughAdded test cases for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10244 +/- ##
=======================================
Coverage 92.60% 92.60%
=======================================
Files 192 192
Lines 16358 16358
Branches 201 201
=======================================
Hits 15148 15148
Misses 1193 1193
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Issue
Closes #8473
When
directAccessis enabled,toJSONwithObjectsinsrc/triggers.jsleaks liveParseACLinstances into the response instead of plain JSON objects. This causes the client-side Parse SDK to throw:The bug occurs when an
afterFindhook modifies objects via.setACL()or.set(). SinceParseACLlacks_toFullJSON, the raw instance is assigned directly to the response JSON. WithdirectAccess, the response bypasses HTTP serialization (JSON.stringify), so theParseACLinstance leaks through toParseObject._finishFetch, which expects plain JSON.Approach
In
toJSONwithObjects, when a pending op value lacks_toFullJSON, check if it has atoJSONmethod and call it before assigning. This serializes SDK types likeParseACLandParseGeoPointto plain JSON — consistent with whatobject.toJSON()already does in the same function.Change in
src/triggers.js:198:Tasks