Skip to content
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

fix: Crash when reading corrupted envelope #4297

Merged
merged 14 commits into from
Aug 26, 2024

Conversation

brustolin
Copy link
Contributor

📜 Description

Added a safe guard to prevent crashing when reading corrupted envelope

💡 Motivation and Context

This was mentioned in #4280

💚 How did you test it?

Unit tests

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link

github-actions bot commented Aug 26, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1210.73 ms 1245.63 ms 34.90 ms
Size 21.58 KiB 706.70 KiB 685.12 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
135dec6 1236.67 ms 1255.13 ms 18.45 ms
42f4107 1224.52 ms 1244.77 ms 20.25 ms
25bcc50 1237.69 ms 1258.40 ms 20.71 ms
253bb71 1221.62 ms 1250.82 ms 29.20 ms
699d76f 1217.12 ms 1236.33 ms 19.20 ms
4b1113e 1239.04 ms 1250.45 ms 11.41 ms
7cd187e 1239.39 ms 1258.02 ms 18.63 ms
e758449 1243.41 ms 1246.71 ms 3.31 ms
aa4eddf 1228.21 ms 1236.72 ms 8.51 ms
06bac56 1213.62 ms 1236.27 ms 22.65 ms

App size

Revision Plain With Sentry Diff
135dec6 21.58 KiB 615.19 KiB 593.61 KiB
42f4107 21.58 KiB 614.92 KiB 593.34 KiB
25bcc50 20.76 KiB 427.23 KiB 406.46 KiB
253bb71 20.76 KiB 393.37 KiB 372.60 KiB
699d76f 21.58 KiB 631.82 KiB 610.24 KiB
4b1113e 21.58 KiB 697.83 KiB 676.25 KiB
7cd187e 20.76 KiB 401.65 KiB 380.89 KiB
e758449 22.85 KiB 407.62 KiB 384.77 KiB
aa4eddf 21.58 KiB 544.86 KiB 523.28 KiB
06bac56 22.84 KiB 403.23 KiB 380.39 KiB

Previous results on branch: fix/unserialize-envelope-item

Startup times

Revision Plain With Sentry Diff
83979c7 1230.82 ms 1248.88 ms 18.06 ms

App size

Revision Plain With Sentry Diff
83979c7 21.58 KiB 706.78 KiB 685.20 KiB

Copy link

codecov bot commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.696%. Comparing base (a0cc9d6) to head (8966958).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4297       +/-   ##
=============================================
+ Coverage   91.684%   91.696%   +0.011%     
=============================================
  Files          617       617               
  Lines        50194     50205       +11     
  Branches     18116     18119        +3     
=============================================
+ Hits         46020     46036       +16     
+ Misses        4083      4076        -7     
- Partials        91        93        +2     
Files Coverage Δ
Sources/Sentry/SentrySerialization.m 92.018% <100.000%> (+0.191%) ⬆️
.../SentryTests/Helper/SentrySerializationTests.swift 98.507% <100.000%> (+0.045%) ⬆️

... and 9 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0cc9d6...8966958. Read the comment docs.

@brustolin brustolin merged commit b18a037 into main Aug 26, 2024
65 checks passed
@brustolin brustolin deleted the fix/unserialize-envelope-item branch August 26, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash during SDK initialization, presumably due to corrupted envelope data from a previous run
3 participants