dd-sdk-ios icon indicating copy to clipboard operation
dd-sdk-ios copied to clipboard

RUM-3183 fix: Fix crash on accessing `request.allHTTPHeaderFields`

Open ncreated opened this issue 1 year ago • 1 comments

What and why?

📦 🧯 Fixes https://github.com/DataDog/dd-sdk-ios/issues/1638

As we observed in https://github.com/DataDog/dd-sdk-ios/issues/1638, accessing request.allHTTPHeaderFields is not safe and leads to crashes with undefined root cause. To avoid this, instead we use request.value(forHTTPHeaderField:) to only read headers known by the SDK.

How?

Removed usage of .allHTTPHeaderFields by replacing it with higher-level .value(forHTTPHeaderField:) API.

Suspected problem with .allHTTPHeaderFields safety

I failed to reproduce the crash, although by exploring swift-corelibs-foundation code, I suspect it to relate to the Obj-c → Swift bridging, somewhere in this code. Symbols ran during the bridging correspond to some reported in https://github.com/DataDog/dd-sdk-ios/issues/1638 stack traces. This may invalidate our earlier assumption on the problem being caused by lack of thread safety in .allHTTPHeaderFields implementation.

Review checklist

  • [x] Feature or bugfix MUST have appropriate tests (unit, integration)
  • [x] Make sure each commit and the PR mention the Issue number or JIRA reference
  • [x] Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • [ ] Run unit tests for Core, RUM, Trace, Logs, CR and WVT
  • [ ] Run unit tests for Session Replay
  • [ ] Run integration tests
  • [ ] Run smoke tests
  • [ ] Run tests for tools/

ncreated avatar May 17 '24 09:05 ncreated

Datadog Report

Branch report: ncreated/RUM-3183/fix-crash-on-accessing-allHTTPHeaderFields Commit report: fa145d1 Test service: dd-sdk-ios

:white_check_mark: 0 Failed, 3222 Passed, 0 Skipped, 2m 31.53s Total Time :small_red_triangle_down: Test Sessions change in coverage: 9 decreased, 4 increased

:small_red_triangle_down: Code Coverage Decreases vs Default Branch (9)

This report shows up to 5 code coverage decreases.

  • test DatadogCrashReportingTests tvOS 27.39% (-0.32%) - Details
  • test DatadogCrashReportingTests iOS 27.36% (-0.3%) - Details
  • test DatadogLogsTests tvOS 46.14% (-0.26%) - Details
  • test DatadogLogsTests iOS 46.09% (-0.26%) - Details
  • test DatadogWebViewTrackingTests iOS 18.31% (-0.21%) - Details