fix: Harden float JSON encoding and validate battery level#7802
fix: Harden float JSON encoding and validate battery level#7802
Conversation
Ports numeric encoding guards from upstream KSCrash (kstenerud/KSCrash#526 (kstenerud/KSCrash#526)): - Encode ±infinity as 1e999/-1e999 instead of the invalid JSON literal inf - Validate snprintf return before emitting data (format before beginElement so errors can't corrupt the JSON stream) Also validates battery level in SentrySystemEventBreadcrumbs — rejects non-finite and out of-range values before they reach the crash-scope JSON encoder. Relates to #4580
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Internal Changes 🔧Deps
🤖 This preview updates automatically when you update the PR. |
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Fixes
- Harden float JSON encoding and validate battery level ([#7802](https://github.com/getsentry/sentry-cocoa/pull/7802))If none of the above apply, you can opt out of this check by adding |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7802 +/- ##
=============================================
- Coverage 85.416% 85.117% -0.300%
=============================================
Files 487 487
Lines 29190 29208 +18
Branches 12625 12618 -7
=============================================
- Hits 24933 24861 -72
- Misses 4207 4296 +89
- Partials 50 51 +1
... and 15 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
denrase
requested review from
itaybre,
noahsmartin,
philipphofmann and
philprime
as code owners
| // W3C spec says level must be null if it is unknown. | ||
| // Also guard against non-finite or out-of-range values so they never | ||
| // reach the crash-scope JSON encoder as invalid floats. | ||
| if currentState != .unknown && currentLevel.isFinite && (0...1).contains(currentLevel) { |
There was a problem hiding this comment.
l: Does apple even report non finite values?
| if (isinf(value)) { | ||
| int result = sentrycrashjson_beginElement(context, name); | ||
| unlikely_if(result != SentryCrashJSON_OK) { return result; } | ||
| return value > 0 ? addJSONData(context, "1e999", 5) : addJSONData(context, "-1e999", 6); |
There was a problem hiding this comment.
m: are these the expected values at Sentry or just replicating KSCrash?
2 participants
📜 Description
Ports numeric encoding guards from upstream KSCrash (kstenerud/KSCrash#526 (kstenerud/KSCrash#526)):
Also validates battery level in SentrySystemEventBreadcrumbs — rejects non-finite and out of-range values before they reach the crash-scope JSON encoder.
💡 Motivation and Context
Relates to #4580
💚 How did you test it?
Unit tests.
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.