diff --git a/CHANGELOG.md b/CHANGELOG.md index da3585d1077..d2d5e97bf2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - Use correct rendered frames timestamp for TTID/TTFD and app start (#3531) - Move header reference out of "extern C" (#3538) - Missing transactions when not calling `reportFullyDisplayed` (#3477) +- Don't override `sentry-trace` and `baggage` headers (#3540) ## 8.17.2 diff --git a/Sources/Sentry/SentryBaggage.m b/Sources/Sentry/SentryBaggage.m index 2978a97848c..60139dafba9 100644 --- a/Sources/Sentry/SentryBaggage.m +++ b/Sources/Sentry/SentryBaggage.m @@ -34,11 +34,6 @@ - (instancetype)initWithTraceId:(SentryId *)traceId return self; } -- (NSString *)toHTTPHeader -{ - return [self toHTTPHeaderWithOriginalBaggage:nil]; -} - - (NSString *)toHTTPHeaderWithOriginalBaggage:(NSDictionary *_Nullable)originalBaggage { NSMutableDictionary *information diff --git a/Sources/Sentry/SentryNetworkTracker.m b/Sources/Sentry/SentryNetworkTracker.m index 9fa754671b3..a33691d364d 100644 --- a/Sources/Sentry/SentryNetworkTracker.m +++ b/Sources/Sentry/SentryNetworkTracker.m @@ -239,11 +239,12 @@ - (void)addBaggageHeader:(SentryBaggage *)baggage NSString *baggageHeader = @""; if (baggage != nil) { - baggageHeader = - [baggage toHTTPHeaderWithOriginalBaggage: - [SentrySerialization - decodeBaggage:sessionTask.currentRequest - .allHTTPHeaderFields[SENTRY_BAGGAGE_HEADER]]]; + NSDictionary *originalBaggage = [SentrySerialization + decodeBaggage:sessionTask.currentRequest.allHTTPHeaderFields[SENTRY_BAGGAGE_HEADER]]; + + if (originalBaggage[@"sentry-trace_id"] == nil) { + baggageHeader = [baggage toHTTPHeaderWithOriginalBaggage:originalBaggage]; + } } // First we check if the current request is mutable, so we could easily add a new @@ -251,7 +252,10 @@ - (void)addBaggageHeader:(SentryBaggage *)baggage // header. if ([sessionTask.currentRequest isKindOfClass:[NSMutableURLRequest class]]) { NSMutableURLRequest *currentRequest = (NSMutableURLRequest *)sessionTask.currentRequest; - [currentRequest setValue:traceHeader.value forHTTPHeaderField:SENTRY_TRACE_HEADER]; + + if ([currentRequest valueForHTTPHeaderField:SENTRY_TRACE_HEADER] == nil) { + [currentRequest setValue:traceHeader.value forHTTPHeaderField:SENTRY_TRACE_HEADER]; + } if (baggageHeader.length > 0) { [currentRequest setValue:baggageHeader forHTTPHeaderField:SENTRY_BAGGAGE_HEADER]; @@ -265,7 +269,9 @@ - (void)addBaggageHeader:(SentryBaggage *)baggage if ([sessionTask respondsToSelector:setCurrentRequestSelector]) { NSMutableURLRequest *newRequest = [sessionTask.currentRequest mutableCopy]; - [newRequest setValue:traceHeader.value forHTTPHeaderField:SENTRY_TRACE_HEADER]; + if ([newRequest valueForHTTPHeaderField:SENTRY_TRACE_HEADER] == nil) { + [newRequest setValue:traceHeader.value forHTTPHeaderField:SENTRY_TRACE_HEADER]; + } if (baggageHeader.length > 0) { [newRequest setValue:baggageHeader forHTTPHeaderField:SENTRY_BAGGAGE_HEADER]; diff --git a/Sources/Sentry/include/SentryBaggage.h b/Sources/Sentry/include/SentryBaggage.h index dede549ae0e..1aa9a06b558 100644 --- a/Sources/Sentry/include/SentryBaggage.h +++ b/Sources/Sentry/include/SentryBaggage.h @@ -63,7 +63,6 @@ static NSString *const SENTRY_BAGGAGE_HEADER = @"baggage"; sampleRate:(nullable NSString *)sampleRate sampled:(nullable NSString *)sampled; -- (NSString *)toHTTPHeader; - (NSString *)toHTTPHeaderWithOriginalBaggage:(NSDictionary *_Nullable)originalBaggage; @end diff --git a/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerIntegrationTests.swift b/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerIntegrationTests.swift index 43d00f3aafc..bff5d482fa7 100644 --- a/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerIntegrationTests.swift @@ -164,7 +164,7 @@ class SentryNetworkTrackerIntegrationTests: XCTestCase { self.assertNetworkError(error) let response = String(data: data ?? Data(), encoding: .utf8) ?? "" - let expectedBaggageHeader = transaction.traceContext.toBaggage().toHTTPHeader() + let expectedBaggageHeader = transaction.traceContext.toBaggage().toHTTPHeader(withOriginalBaggage: nil) XCTAssertEqual(expectedBaggageHeader, response) expect.fulfill() diff --git a/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift index 23509274206..4039b0169fe 100644 --- a/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift @@ -565,9 +565,21 @@ class SentryNetworkTrackerTests: XCTestCase { let transaction = startTransaction() as! SentryTracer sut.urlSessionTaskResume(task) - let expectedBaggageHeader = transaction.traceContext.toBaggage().toHTTPHeader() + let expectedBaggageHeader = transaction.traceContext.toBaggage().toHTTPHeader(withOriginalBaggage: nil) XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", expectedBaggageHeader) } + + func testDontOverrideBaggageHeader() { + let sut = fixture.getSut() + let task = createDataTask { + var request = $0 + request.setValue("sentry-trace_id=something", forHTTPHeaderField: "baggage") + return request + } + sut.urlSessionTaskResume(task) + + XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", "sentry-trace_id=something") + } func testTraceHeader() { let sut = fixture.getSut() @@ -580,6 +592,18 @@ class SentryNetworkTrackerTests: XCTestCase { let expectedTraceHeader = networkSpan.toTraceHeader().value() XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", expectedTraceHeader) } + + func testDontOverrideTraceHeader() { + let sut = fixture.getSut() + let task = createDataTask { + var request = $0 + request.setValue("test", forHTTPHeaderField: "sentry-trace") + return request + } + sut.urlSessionTaskResume(task) + + XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", "test") + } func testDefaultHeadersWhenDisabled() { let sut = fixture.getSut() @@ -591,7 +615,7 @@ class SentryNetworkTrackerTests: XCTestCase { let expectedTraceHeader = SentrySDK.currentHub().scope.propagationContext.traceHeader.value() let traceContext = SentryTraceContext(trace: SentrySDK.currentHub().scope.propagationContext.traceId, options: self.fixture.options, userSegment: self.fixture.scope.userObject?.segment) - let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader() + let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader(withOriginalBaggage: nil) XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", expectedBaggageHeader) XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", expectedTraceHeader) } @@ -603,7 +627,7 @@ class SentryNetworkTrackerTests: XCTestCase { let expectedTraceHeader = SentrySDK.currentHub().scope.propagationContext.traceHeader.value() let traceContext = SentryTraceContext(trace: SentrySDK.currentHub().scope.propagationContext.traceId, options: self.fixture.options, userSegment: self.fixture.scope.userObject?.segment) - let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader() + let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader(withOriginalBaggage: nil) XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", expectedBaggageHeader) XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", expectedTraceHeader) } @@ -897,9 +921,12 @@ class SentryNetworkTrackerTests: XCTestCase { XCTAssertEqual(duration, expectedDuration) } - func createDataTask(method: String = "GET") -> URLSessionDataTaskMock { + func createDataTask(method: String = "GET", modifyRequest: ((URLRequest) -> (URLRequest))? = nil) -> URLSessionDataTaskMock { var request = URLRequest(url: SentryNetworkTrackerTests.fullUrl) request.httpMethod = method + if let modifyRequest = modifyRequest { + request = modifyRequest(request) + } return URLSessionDataTaskMock(request: request) } diff --git a/Tests/SentryTests/Transaction/SentryBaggageTests.swift b/Tests/SentryTests/Transaction/SentryBaggageTests.swift index a728d8e9c1b..88891a42894 100644 --- a/Tests/SentryTests/Transaction/SentryBaggageTests.swift +++ b/Tests/SentryTests/Transaction/SentryBaggageTests.swift @@ -3,12 +3,6 @@ import Sentry import XCTest class SentryBaggageTests: XCTestCase { - func test_baggageToHeader() { - let header = SentryBaggage(trace: SentryId.empty, publicKey: "publicKey", releaseName: "release name", environment: "teste", transaction: "transaction", userSegment: "test user", sampleRate: "0.49", sampled: "true").toHTTPHeader() - - XCTAssertEqual(header, "sentry-environment=teste,sentry-public_key=publicKey,sentry-release=release%20name,sentry-sample_rate=0.49,sentry-sampled=true,sentry-trace_id=00000000000000000000000000000000,sentry-transaction=transaction,sentry-user_segment=test%20user") - } - func test_baggageToHeader_AppendToOriginal() { let header = SentryBaggage(trace: SentryId.empty, publicKey: "publicKey", releaseName: "release name", environment: "teste", transaction: "transaction", userSegment: "test user", sampleRate: "0.49", sampled: "true").toHTTPHeader(withOriginalBaggage: ["a": "a", "sentry-trace_id": "to-be-overwritten"]) @@ -16,7 +10,7 @@ class SentryBaggageTests: XCTestCase { } func test_baggageToHeader_onlyTrace_ignoreNils() { - let header = SentryBaggage(trace: SentryId.empty, publicKey: "publicKey", releaseName: nil, environment: nil, transaction: nil, userSegment: nil, sampleRate: nil, sampled: nil).toHTTPHeader() + let header = SentryBaggage(trace: SentryId.empty, publicKey: "publicKey", releaseName: nil, environment: nil, transaction: nil, userSegment: nil, sampleRate: nil, sampled: nil).toHTTPHeader(withOriginalBaggage: nil) XCTAssertEqual(header, "sentry-public_key=publicKey,sentry-trace_id=00000000000000000000000000000000") } diff --git a/scripts/no-changes-in-high-risk-files.sh b/scripts/no-changes-in-high-risk-files.sh index a110a7ce4eb..376122b987d 100755 --- a/scripts/no-changes-in-high-risk-files.sh +++ b/scripts/no-changes-in-high-risk-files.sh @@ -5,7 +5,7 @@ set -euo pipefail ACTUAL=$(shasum -a 256 ./Sources/Sentry/SentryNSURLSessionTaskSearch.m ./Sources/Sentry/SentryNetworkTracker.m ./Sources/Sentry/SentryUIViewControllerSwizzling.m ./Sources/Sentry/SentryNSDataSwizzling.m ./Sources/Sentry/SentrySubClassFinder.m ./Sources/Sentry/SentryCoreDataSwizzling.m ./Sources/Sentry/SentrySwizzleWrapper.m ./Sources/Sentry/include/SentrySwizzle.h ./Sources/Sentry/SentrySwizzle.m) EXPECTED="819d5ca5e3db2ac23c859b14c149b7f0754d3ae88bea1dba92c18f49a81da0e1 ./Sources/Sentry/SentryNSURLSessionTaskSearch.m -2f9ea6984ff32b53fc03c386b648f4f1bdf8737ce69050d25ce6d1307f8d1672 ./Sources/Sentry/SentryNetworkTracker.m +bee886f6eaa3ec0ba999fe0dbe38537a47cb328a3432b908231326a65a9fb2d3 ./Sources/Sentry/SentryNetworkTracker.m 40f476800b32cf885dba9ac3de75d93b6f536e819fe8e51d071b4610c879b416 ./Sources/Sentry/SentryUIViewControllerSwizzling.m e95e62ec7363984f20c78643bb7d992a41a740f97e1befb71525ac34caf88b37 ./Sources/Sentry/SentryNSDataSwizzling.m cc3849725bd1733515c71742872bed94ca47d2c115ef9d8c98383eae2e171925 ./Sources/Sentry/SentrySubClassFinder.m