fix: Crash deserializing empty envelope length>0

[kahest]

📜 Description

Deserializing empty envelope with length != 0 crashes SDK during init. Unsure what the root cause (creating this kind of invalid payload) is.

However, it seems we already had code for that exact case in place but it didn't catch it because the iterator was incremented at the beginning of the loop, so the if checking for whether end of envelope is reached didn't work.

Least intrusive solution is to move incrementing to right before we use the iterator to avoid side effects.

💡 Motivation and Context

fix GH-4280

💚 How did you test it?

Unit test

📝 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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.597%. Comparing base (02671d8) to head (706c985).

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #4281       +/-   ##
=============================================
- Coverage   91.601%   91.597%   -0.004%     
=============================================
  Files          616       616               
  Lines        49995     49999        +4     
  Branches     18080     18091       +11     
=============================================
+ Hits         45796     45798        +2     
+ Misses        4108      4107        -1     
- Partials        91        94        +3     

Files	Coverage Δ
Sources/Sentry/SentrySerialization.m	91.891% <100.000%> (+4.054%)	⬆️
Tests/SentryTests/PrivateSentrySDKOnlyTests.swift	99.629% <100.000%> (+0.004%)	⬆️

... and 7 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 02671d8...706c985. Read the comment docs.

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1222.81 ms	1244.74 ms	21.93 ms
Size	21.58 KiB	705.40 KiB	683.82 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
340fb46	1236.45 ms	1244.38 ms	7.93 ms
9f12001	1235.61 ms	1246.80 ms	11.18 ms
7512778	1200.78 ms	1206.33 ms	5.55 ms
97de6c8	1226.78 ms	1240.98 ms	14.20 ms
feba2be	1246.67 ms	1254.64 ms	7.97 ms
643853e	1225.75 ms	1247.00 ms	21.25 ms
2deb275	1224.04 ms	1239.37 ms	15.33 ms
b2c9166	1246.86 ms	1255.28 ms	8.42 ms
6e31b7c	1230.80 ms	1246.29 ms	15.49 ms
25bcc50	1209.53 ms	1224.86 ms	15.33 ms

App size

Revision	Plain	With Sentry	Diff
340fb46	21.58 KiB	418.78 KiB	397.20 KiB
9f12001	21.58 KiB	694.47 KiB	672.89 KiB
7512778	21.58 KiB	539.87 KiB	518.29 KiB
97de6c8	21.58 KiB	613.99 KiB	592.41 KiB
feba2be	20.76 KiB	414.45 KiB	393.69 KiB
643853e	21.58 KiB	655.73 KiB	634.15 KiB
2deb275	21.58 KiB	681.72 KiB	660.14 KiB
b2c9166	21.58 KiB	630.26 KiB	608.68 KiB
6e31b7c	21.58 KiB	614.65 KiB	593.07 KiB
25bcc50	20.76 KiB	427.22 KiB	406.46 KiB

[philipphofmann]

Thank you @kahest 💯

[philipphofmann]

We see two occurrences of this in our internal SDK crash detection. So this bug occurs rarely.

[MrMage]

Thank you for merging this so quickly! However, I would have recommended to also perform a greater-than-or-equals check. Otherwise, if an attachment ever gets written partially, a crash could still occur, because currently you are checking only for no attachment being present at all.

I would also recommend investigating how this can occur in the first place — maybe the attachment file is being written incrementally rather than atomically, so a crash or power outage after writing the header but before writing the attachment itself could lead to this condition?

[philipphofmann]

@MrMage, yes, that definitely makes sense. This was just a quick fix; we will investigate this problem further, as SentrySerialization.dataWithEnvelope does a couple of calls to NSMakeRange that can easily lead to a crash if not handled carefully. I reopened the issue.

sentry-cocoa/Sources/Sentry/SentrySerialization.m

Line 95 in 464117d

+ (SentryEnvelope *_Nullable)envelopeWithData:(NSData *)data

[philipphofmann]

This is related to #4338.