Surrogate-safe JSON serialization for payloads, responses, and RPC bodies#54
Conversation
…odies Follow-up to the lenient string extraction: System.Text.Json's WRITER also throws on lone-surrogate strings (with or without a custom encoder), so trees containing them failed at serialization time — surfaced in production as pipe() errors on /search-api/similar when clients sent titles/bodies with mangled unicode. Serialize upstream payloads, client response bodies, and Hive RPC request bodies with JsJson.Stringify, which tolerates lone surrogates by construction and byte-matches the JSON.stringify output the Node service produced. The Solana raw-text reconstruction keeps ToJsonString deliberately: it needs raw u64 preservation and RPC responses carry no user strings.
Greptile SummaryThis PR makes JSON serialization match the Node service for surrogate-safe proxying. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "docs: codify JS-double number semantics ..." | Re-trigger Greptile |
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughInfrastructure JSON serialization now uses ChangesJSON serialization updates
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
…s invariants Both have now been flagged (in both directions) by automated review across multiple PRs; write them down where review tooling and contributors pick up repo context.
|
@greptile-apps Repository convention to remember for future reviews of this repo — this class of finding has now been raised (in both directions) on several PRs, and it is a designed invariant, not a defect: Number precision on proxied JSON is intentionally JavaScript-double semantics. This service is a behavior-preserving replacement for a Node/Express proxy: every request body and upstream response historically went through Likewise, lone-surrogate Both invariants are now codified in |
|
Understood and saved. View rule Going forward, findings on this repo that contradict either invariant will be treated as false positives:
Both are gated by the golden-vector tests in |
Follow-up to #52. Path-attributed diagnostics showed the surrogate failure surfacing in a second place:
pipe(): error on /search-api/similar. Root cause: System.Text.Json's writer also throws on lone-surrogate strings (verified with both default and relaxed-encoderToJsonString), so any JsonNode tree containing one failed at serialization — upstream payloads built from client bodies, response bodies echoing upstream data, and Hive RPC request bodies carrying client-derived names.All three now serialize with
JsJson.Stringify, which tolerates lone surrogates by construction (re-emitting them as escapes, like well-formedJSON.stringify) and byte-matches what the Node service sent. The Solana raw-text reconstruction deliberately keepsToJsonString: it depends on raw u64 preservation and those RPC responses contain no user-controlled strings.Verified: 43 tests green; end-to-end smoke of the exact failing shape (
/search-api/similarwith a lone-surrogate title) now proxies 200 with the payload delivered upstream and zero exceptions.Summary by CodeRabbit