-
-
Notifications
You must be signed in to change notification settings - Fork 279
fix(types): improve JSON decoding resilience #1301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add explicit type casts and validation for JSON parsing across all SDK packages. Support both int and num numeric types for better compatibility with different JSON decoders. Add unknown enum values for forward compatibility with new factor types and statuses. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughHardened JSON deserialization across multiple packages by replacing direct parsing/force-unwrapping with factories/private parsers, adding runtime type checks and casts, introducing unknown enum variants, validating numeric/date fields, and improving error messages via FormatException (no public API removals). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/gotrue/lib/src/types/session.dart`:
- Around line 34-46: The FormatException constructors in the session parsing
(the userJson check and the User.fromJson null branch) currently include
json.toString(), which can leak tokens/PII; update these throws to omit the full
payload or supply a redacted summary instead (e.g., no second argument or a
small redacted map/string), altering the two places that construct
FormatException around userJson and User.fromJson to avoid embedding the
original json contents.
🧹 Nitpick comments (1)
packages/realtime_client/lib/src/realtime_presence.dart (1)
18-26: Good implementation that correctly avoids input mutation.The shallow copy with
Map<String, dynamic>.from(map)before removing'presence_ref'properly addresses the mutation concern mentioned in the PR objectives. The null-coalescing to empty string provides sensible resilient decoding.Minor note: In
deepClone()(line 30-31), ifpayloadsomehow contains a'presence_ref'key, it would shadow the explicit assignment due to spread order. This is unlikely givenfromJsonremoves it, but you could swap the order for defensive coding:return Presence.fromJson({ ...payload, 'presence_ref': presenceRef, // Ensure this takes precedence });
Avoid leaking tokens and PII by removing json.toString() from FormatException constructors in Session.fromJson. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Check if the completer is already completed before calling completeError in dispose() to avoid "Bad state: Future already completed" errors during concurrent dispose operations. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Summary
Improve JSON parsing resilience across all Supabase Flutter SDK packages with explicit type validation and robust error handling. Support both
intandnumnumeric types to handle different JSON decoder behaviors. Addunknownenum values for forward compatibility.Changes
as String,as Map<String, dynamic>) for all JSON fieldsis! inttois! numwith.toInt()conversionunknownenum values toFactorTypeandFactorStatusfor forward compatibilityPresence.fromJsonandPostgresChangePayload.fromPayload🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.