fix: Avoid mutating live network requests

Copy the task's current request before adding trace propagation headers so CFNetwork never observes concurrent in-place mutations.

Fixes #8519

Author: philprime

CHANGELOG.md

@@ -17,6 +17,7 @@
 
 - Fix incorrect `duration` sent for active sessions (#8612)
   - Session `duration` is now set only when the session ends. Active sessions (including on error increments) no longer emit a bogus `duration`.
+- Fix a race caused by mutating `URLSessionTask.currentRequest` during trace header propagation (#8519)
 - Fix a race that could prevent consecutive app hangs from being reported (#8627)
 - Fix malformed itms-services URL in SentryDistribution updater (#8567)
 

Sources/Sentry/SentryTracePropagation.m

@@ -38,25 +38,18 @@ + (void)addBaggageHeader:(SentryBaggage *)baggage
         }
     }
 
-    if ([request isKindOfClass:[NSMutableURLRequest class]]) {
-        NSMutableURLRequest *mutableRequest = (NSMutableURLRequest *)request;
-        [SentryTracePropagation addHeaderFieldsToRequest:mutableRequest
+    // CFNetwork may read currentRequest concurrently, so never mutate it in place.
+    SEL setCurrentRequestSelector = NSSelectorFromString(@"setCurrentRequest:");
+    if ([sessionTask respondsToSelector:setCurrentRequestSelector]) {
+        NSMutableURLRequest *newRequest = [request mutableCopy];
+        [SentryTracePropagation addHeaderFieldsToRequest:newRequest
                                              traceHeader:traceHeader
                                            baggageHeader:baggageHeader
                                     propagateTraceparent:propagateTraceparent];
-    } else {
-        SEL setCurrentRequestSelector = NSSelectorFromString(@"setCurrentRequest:");
-        if ([sessionTask respondsToSelector:setCurrentRequestSelector]) {
-            NSMutableURLRequest *newRequest = [request mutableCopy];
-            [SentryTracePropagation addHeaderFieldsToRequest:newRequest
-                                                 traceHeader:traceHeader
-                                               baggageHeader:baggageHeader
-                                        propagateTraceparent:propagateTraceparent];
 
-            void (*func)(id, SEL, id param)
-                = (void *)[sessionTask methodForSelector:setCurrentRequestSelector];
-            func(sessionTask, setCurrentRequestSelector, newRequest);
-        }
+        void (*func)(id, SEL, id param)
+            = (void *)[sessionTask methodForSelector:setCurrentRequestSelector];
+        func(sessionTask, setCurrentRequestSelector, newRequest);
     }
 }
 

Tests/SentryTests/Integrations/Performance/Network/SentryTracePropagationTests.swift

@@ -170,11 +170,36 @@ final class SentryTracePropagationTests: XCTestCase {
         XCTAssertNil(task.currentRequest?.value(forHTTPHeaderField: "sentry-trace"))
     }
 
-    private func createSessionTask(method: String = "GET") throws -> URLSessionDownloadTaskMock {
+    func testAddBaggageHeader_whenCurrentRequestIsMutable_shouldNotMutateItInPlace() throws {
+        // -- Arrange --
+        let request = URLRequest(url: try XCTUnwrap(URL(string: "https://www.domain.com/api")))
+        let task = MutableRequestTaskMock(request: request)
+        let traceHeader = TraceHeader(
+            trace: SentryId(),
+            spanId: SpanId(),
+            sampled: .yes
+        )
+
+        // -- Act --
+        SentryTracePropagation.addBaggageHeader(
+            Baggage(),
+            traceHeader: traceHeader,
+            propagateTraceparent: true,
+            tracePropagationTargets: [try XCTUnwrap(NSRegularExpression(pattern: ".*"))],
+            toRequest: task
+        )
+
+        // -- Assert --
+        XCTAssertEqual(task.setCurrentRequestCallCount, 1)
+        XCTAssertNil(task.initialCurrentRequest.value(forHTTPHeaderField: "sentry-trace"))
+        XCTAssertEqual(task.currentRequest?.value(forHTTPHeaderField: "sentry-trace"), traceHeader.value())
+    }
+
+    private func createSessionTask(method: String = "GET") throws -> URLSessionDataTaskMock {
         let url = try XCTUnwrap(URL(string: "https://www.domain.com/api?query=value&query2=value2#fragment"))
         var request = URLRequest(url: url)
         request.httpMethod = method
-        return URLSessionDownloadTaskMock(request: request)
+        return URLSessionDataTaskMock(request: request)
     }
 
 }

Tests/SentryTests/Integrations/Performance/Network/URLSessionTaskMock.h

@@ -86,4 +86,11 @@ static int64_t const DATA_BYTES_SENT = 652;
 
 @end
 
+@interface MutableRequestTaskMock : URLSessionDataTaskMock
+
+@property (nonatomic, readonly) NSMutableURLRequest *initialCurrentRequest;
+@property (nonatomic, readonly) NSUInteger setCurrentRequestCallCount;
+
+@end
+
 NS_ASSUME_NONNULL_END

Tests/SentryTests/Integrations/Performance/Network/URLSessionTaskMock.m

@@ -325,3 +325,37 @@ - (NSURLRequest *)currentRequest
 }
 
 @end
+
+@implementation MutableRequestTaskMock {
+    NSMutableURLRequest *_initialCurrentRequest;
+    NSUInteger _setCurrentRequestCallCount;
+}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+- (instancetype)initWithRequest:(NSURLRequest *)request
+{
+    if (self = [super initWithRequest:request]) {
+        _initialCurrentRequest = (NSMutableURLRequest *)[super currentRequest];
+    }
+    return self;
+}
+#pragma clang diagnostic pop
+
+- (NSMutableURLRequest *)initialCurrentRequest
+{
+    return _initialCurrentRequest;
+}
+
+- (NSUInteger)setCurrentRequestCallCount
+{
+    return _setCurrentRequestCallCount;
+}
+
+- (void)setCurrentRequest:(NSURLRequest *)request
+{
+    _setCurrentRequestCallCount++;
+    [super setCurrentRequest:request];
+}
+
+@end