From 153db3dda25e15127048c889ca2bfbb7e1675f3a Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 7 Oct 2024 09:40:22 +0200 Subject: [PATCH 1/2] fix: Edge case for swizzleClassNameExclude Skip creating transactions for UIViewControllers ignored for swizzling via the option swizzleClassNameExclude. Due to some edge cases with nib files, the SDK doesn't swizzle the loadView method of the UIViewController subclasses, but instead, it swizzles the UIViewController.loadView method directly. Although the SDK doesn't swizzle the classes specified in swizzleClassNameExclude, it created transactions. Now, this is fixed. Fixes GH-4386 --- CHANGELOG.md | 2 + Sentry.xcodeproj/project.pbxproj | 4 ++ Sources/Sentry/Public/SentryOptions.h | 8 ++-- Sources/Sentry/SentrySubClassFinder.m | 11 ++---- ...SentryUIViewControllerPerformanceTracker.m | 12 ++++++ .../Performance/SwizzleClassNameExclude.swift | 13 +++++++ ...iewControllerPerformanceTrackerTests.swift | 39 ++++++++++++++----- 7 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 Sources/Swift/Integrations/Performance/SwizzleClassNameExclude.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 32c8fd3070a..052099ec8e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ - Fix the versioning to support app release with Beta versions (#4368) - Linking ongoing trace to crash event (#4393) +- Edge case for swizzleClassNameExclude (#4405): Skip creating transactions for UIViewControllers ignored for swizzling +via the option `swizzleClassNameExclude`. ## 8.37.0-beta.1 diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index 9a77b69818e..67fd3a876d0 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -103,6 +103,7 @@ 62872B5F2BA1B7F300A4FA7D /* NSLock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62872B5E2BA1B7F300A4FA7D /* NSLock.swift */; }; 62872B632BA1B86100A4FA7D /* NSLockTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62872B622BA1B86100A4FA7D /* NSLockTests.swift */; }; 62885DA729E946B100554F38 /* TestConncurrentModifications.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62885DA629E946B100554F38 /* TestConncurrentModifications.swift */; }; + 629428802CB3BF69002C454C /* SwizzleClassNameExclude.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6294287F2CB3BF4E002C454C /* SwizzleClassNameExclude.swift */; }; 6294774C2C6F255F00846CBC /* SentryANRTrackerV2Delegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6294774B2C6F255F00846CBC /* SentryANRTrackerV2Delegate.swift */; }; 62950F1029E7FE0100A42624 /* SentryTransactionContextTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62950F0F29E7FE0100A42624 /* SentryTransactionContextTests.swift */; }; 629690532AD3E060000185FA /* SentryReachabilitySwiftTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 629690522AD3E060000185FA /* SentryReachabilitySwiftTests.swift */; }; @@ -1113,6 +1114,7 @@ 62872B5E2BA1B7F300A4FA7D /* NSLock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSLock.swift; sourceTree = ""; }; 62872B622BA1B86100A4FA7D /* NSLockTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSLockTests.swift; sourceTree = ""; }; 62885DA629E946B100554F38 /* TestConncurrentModifications.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestConncurrentModifications.swift; sourceTree = ""; }; + 6294287F2CB3BF4E002C454C /* SwizzleClassNameExclude.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SwizzleClassNameExclude.swift; sourceTree = ""; }; 6294774B2C6F255F00846CBC /* SentryANRTrackerV2Delegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryANRTrackerV2Delegate.swift; sourceTree = ""; }; 62950F0F29E7FE0100A42624 /* SentryTransactionContextTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryTransactionContextTests.swift; sourceTree = ""; }; 629690522AD3E060000185FA /* SentryReachabilitySwiftTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryReachabilitySwiftTests.swift; sourceTree = ""; }; @@ -3838,6 +3840,7 @@ D8739CF72BECFF92007D2F66 /* Performance */ = { isa = PBXGroup; children = ( + 6294287F2CB3BF4E002C454C /* SwizzleClassNameExclude.swift */, D8739CF82BECFFB5007D2F66 /* SentryTransactionNameSource.swift */, ); path = Performance; @@ -4744,6 +4747,7 @@ 7B2A70DD27D6083D008B0D15 /* SentryThreadWrapper.m in Sources */, 62E146D02BAAE47600ED34FD /* LocalMetricsAggregator.swift in Sources */, D8ACE3C72762187200F5A213 /* SentryNSDataSwizzling.m in Sources */, + 629428802CB3BF69002C454C /* SwizzleClassNameExclude.swift in Sources */, 638DC9A11EBC6B6400A66E41 /* SentryRequestOperation.m in Sources */, 63AA767A1EB8D20500D153DE /* SentryLogC.m in Sources */, 6344DDBA1EC3115C00D9160D /* SentryCrashReportConverter.m in Sources */, diff --git a/Sources/Sentry/Public/SentryOptions.h b/Sources/Sentry/Public/SentryOptions.h index a98d8d64cc8..c7dd8e36c2c 100644 --- a/Sources/Sentry/Public/SentryOptions.h +++ b/Sources/Sentry/Public/SentryOptions.h @@ -446,17 +446,17 @@ NS_SWIFT_NAME(Options) @property (nonatomic, assign) BOOL enableSwizzling; /** - * An array of class names to ignore for swizzling. + * A set of class names to ignore for swizzling. * * @discussion The SDK checks if a class name of a class to swizzle contains a class name of this * array. For example, if you add MyUIViewController to this list, the SDK excludes the following * classes from swizzling: YourApp.MyUIViewController, YourApp.MyUIViewControllerA, * MyApp.MyUIViewController. - * We can't use an @c NSArray here because we use this as a workaround for which users have + * We can't use an @c NSSet here because we use this as a workaround for which users have * to pass in class names that aren't available on specific iOS versions. By using @c - * NSArray, users can specify unavailable class names. + * NSSet, users can specify unavailable class names. * - * @note Default is an empty array. + * @note Default is an empty set. */ @property (nonatomic, strong) NSSet *swizzleClassNameExcludes; diff --git a/Sources/Sentry/SentrySubClassFinder.m b/Sources/Sentry/SentrySubClassFinder.m index 2960b540ef1..b730f01f687 100644 --- a/Sources/Sentry/SentrySubClassFinder.m +++ b/Sources/Sentry/SentrySubClassFinder.m @@ -2,6 +2,7 @@ #import "SentryDispatchQueueWrapper.h" #import "SentryLog.h" #import "SentryObjCRuntimeWrapper.h" +#import "SentrySwift.h" #import #import @@ -61,13 +62,9 @@ - (void)actOnSubclassesOfViewControllerInImage:(NSString *)imageName block:(void for (int i = 0; i < count; i++) { NSString *className = [NSString stringWithUTF8String:classes[i]]; - BOOL shouldExcludeClassFromSwizzling = NO; - for (NSString *swizzleClassNameExclude in self.swizzleClassNameExcludes) { - if ([className containsString:swizzleClassNameExclude]) { - shouldExcludeClassFromSwizzling = YES; - break; - } - } + BOOL shouldExcludeClassFromSwizzling = [SentrySwizzleClassNameExclude + shouldExcludeClassWithClassName:className + swizzleClassNameExcludes:self.swizzleClassNameExcludes]; // It is vital to avoid calling NSClassFromString for the excluded classes because we // had crashes for specific classes when calling NSClassFromString, such as diff --git a/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m b/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m index 51be581e7d6..200a6f933df 100644 --- a/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m +++ b/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m @@ -68,6 +68,18 @@ - (void)viewControllerLoadView:(UIViewController *)controller return; } + SentryOptions *options = [SentrySDK options]; + + if ([SentrySwizzleClassNameExclude + shouldExcludeClassWithClassName:NSStringFromClass([controller class]) + swizzleClassNameExcludes:options.swizzleClassNameExcludes]) { + SENTRY_LOG_DEBUG(@"Won't track view controller because it's excluded with the option " + @"swizzleClassNameExcludes: %@", + controller); + callbackToOrigin(); + return; + } + [self limitOverride:@"loadView" target:controller callbackToOrigin:callbackToOrigin diff --git a/Sources/Swift/Integrations/Performance/SwizzleClassNameExclude.swift b/Sources/Swift/Integrations/Performance/SwizzleClassNameExclude.swift new file mode 100644 index 00000000000..9b60ae87ff1 --- /dev/null +++ b/Sources/Swift/Integrations/Performance/SwizzleClassNameExclude.swift @@ -0,0 +1,13 @@ +import Foundation + +@objcMembers +class SentrySwizzleClassNameExclude: NSObject { + static func shouldExcludeClass(className: String, swizzleClassNameExcludes: Set) -> Bool { + for exclude in swizzleClassNameExcludes { + if className.contains(exclude) { + return true + } + } + return false + } +} diff --git a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift index dac6d9af223..24fa222d4a3 100644 --- a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift @@ -26,15 +26,7 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase { private class Fixture { - var options: Options { - let options = Options.noIntegrations() - let imageName = String( - cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!, - encoding: .utf8)! as NSString - options.add(inAppInclude: imageName.lastPathComponent) - options.debug = true - return options - } + var options: Options let viewController = TestViewController() let tracker = SentryPerformanceTracker.shared @@ -50,6 +42,13 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase { } init() { + options = Options.noIntegrations() + let imageName = String( + cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!, + encoding: .utf8)! as NSString + options.add(inAppInclude: imageName.lastPathComponent) + options.debug = true + framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, dispatchQueueWrapper: TestSentryDispatchQueueWrapper(), notificationCenter: TestNSNotificationCenterWrapper(), keepDelayedFramesDuration: 0) SentryDependencyContainer.sharedInstance().framesTracker = framesTracker @@ -500,7 +499,7 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase { wait(for: [callbackExpectation], timeout: 0) } - func testLoadView_withUIViewController() { + func testLoadView_withNonInAppUIViewController_DoesNotStartTransaction() { let sut = fixture.getSut() let viewController = UIViewController() let tracker = fixture.tracker @@ -519,6 +518,26 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase { wait(for: [callbackExpectation], timeout: 0) } + func testLoadView_withIgnoreSwizzleUIViewController_DoesNotStartTransaction() { + fixture.options.swizzleClassNameExcludes = ["TestViewController"] + let sut = fixture.getSut() + let viewController = fixture.viewController + let tracker = fixture.tracker + var transactionSpan: Span! + let callbackExpectation = expectation(description: "Callback Expectation") + + XCTAssertTrue(getStack(tracker).isEmpty) + + sut.viewControllerLoadView(viewController) { + let spans = self.getStack(tracker) + transactionSpan = spans.first + callbackExpectation.fulfill() + } + + XCTAssertNil(transactionSpan, "Expected to transaction.") + wait(for: [callbackExpectation], timeout: 0) + } + func testSecondLoadView() throws { let sut = fixture.getSut() let viewController = fixture.viewController From ca857903419832f8b5e56d99fb0a5b3520576bb1 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 7 Oct 2024 11:03:56 +0200 Subject: [PATCH 2/2] fix: Swizzling RootUIViewController if ignored The SDK didn't exclude the RootViewController from swizzling when ignored by the option swizzleClassNameExclude. This is fixed now. Fixes GH-4385 --- CHANGELOG.md | 1 + .../Sentry/SentryUIViewControllerSwizzling.m | 11 ++++++++++ ...SentryUIViewControllerSwizzlingTests.swift | 21 +++++++++++++------ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 052099ec8e9..7ddd08f3126 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - Linking ongoing trace to crash event (#4393) - Edge case for swizzleClassNameExclude (#4405): Skip creating transactions for UIViewControllers ignored for swizzling via the option `swizzleClassNameExclude`. +- Swizzling RootUIViewController if ignored by `swizzleClassNameExclude` (#4407) ## 8.37.0-beta.1 diff --git a/Sources/Sentry/SentryUIViewControllerSwizzling.m b/Sources/Sentry/SentryUIViewControllerSwizzling.m index 0cd42b99119..456dd933e16 100644 --- a/Sources/Sentry/SentryUIViewControllerSwizzling.m +++ b/Sources/Sentry/SentryUIViewControllerSwizzling.m @@ -36,6 +36,7 @@ @interface UIApplication (SentryUIApplication) @interface SentryUIViewControllerSwizzling () +@property (nonatomic, strong) SentryOptions *options; @property (nonatomic, strong) SentryInAppLogic *inAppLogic; @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue; @property (nonatomic, strong) id objcRuntimeWrapper; @@ -56,6 +57,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options binaryImageCache:(SentryBinaryImageCache *)binaryImageCache { if (self = [super init]) { + self.options = options; self.inAppLogic = [[SentryInAppLogic alloc] initWithInAppIncludes:options.inAppIncludes inAppExcludes:options.inAppExcludes]; self.dispatchQueue = dispatchQueue; @@ -341,6 +343,15 @@ - (void)swizzleViewControllerSubClass:(Class)class */ - (BOOL)shouldSwizzleViewController:(Class)class { + NSString *className = NSStringFromClass(class); + + BOOL shouldExcludeClassFromSwizzling = [SentrySwizzleClassNameExclude + shouldExcludeClassWithClassName:className + swizzleClassNameExcludes:self.options.swizzleClassNameExcludes]; + if (shouldExcludeClassFromSwizzling) { + return NO; + } + return [self.inAppLogic isClassInApp:class]; } diff --git a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift index f11525dc60b..fa4a7cad04c 100644 --- a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift +++ b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift @@ -13,14 +13,13 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase { let subClassFinder: TestSubClassFinder let processInfoWrapper = SentryNSProcessInfoWrapper() let binaryImageCache: SentryBinaryImageCache + var options: Options init() { subClassFinder = TestSubClassFinder(dispatchQueue: dispatchQueue, objcRuntimeWrapper: objcRuntimeWrapper, swizzleClassNameExcludes: []) binaryImageCache = SentryDependencyContainer.sharedInstance().binaryImageCache - } - - var options: Options { - let options = Options.noIntegrations() + + options = Options.noIntegrations() let imageName = String( cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!, @@ -31,8 +30,6 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase { cString: class_getImageName(ExternalUIViewController.self)!, encoding: .utf8)! as NSString options.add(inAppInclude: externalImageName.lastPathComponent) - - return options } var sut: SentryUIViewControllerSwizzling { @@ -98,6 +95,18 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase { XCTAssertFalse(result) } + func testShouldNotSwizzle_UIViewControllerExcludedFromSwizzling() { + fixture.options.swizzleClassNameExcludes = ["TestViewController"] + + XCTAssertFalse(fixture.sut.shouldSwizzleViewController(TestViewController.self)) + } + + func testShouldSwizzle_UIViewControllerNotExcludedFromSwizzling() { + fixture.options.swizzleClassNameExcludes = ["TestViewController1"] + + XCTAssertTrue(fixture.sut.shouldSwizzleViewController(TestViewController.self)) + } + func testUIViewController_loadView_noTransactionBoundToScope() { fixture.sut.start() let controller = UIViewController()