ref: Changing the redact logic (#4327)

Before this PR, redactAllText was wrongly used as a "redact is enabled" flag.

Now we're using redactAllText and redactAllImages as an indicator to how start the redact list

CHANGELOG.md

@@ -19,6 +19,7 @@
 - Don't redact clipped views (#4325)
 - Session replay for crash not created because of a race condition (#4314)
 - Double-quoted include, expected angle-bracketed instead (#4298)
+- Stop using `redactAllText` as an indicator tha redact is enabled (#4327)
 
 ### Improvements
 

Sources/Sentry/PrivateSentrySDKOnly.mm

@@ -11,7 +11,7 @@
 #import "SentryOptions.h"
 #import "SentrySDK+Private.h"
 #import "SentrySerialization.h"
-#import "SentrySessionReplayIntegration.h"
+#import "SentrySessionReplayIntegration+Private.h"
 #import "SentrySwift.h"
 #import "SentryThreadHandle.hpp"
 #import "SentryUser+Private.h"
@@ -21,7 +21,6 @@
 #import <SentryFramesTracker.h>
 #import <SentryScope+Private.h>
 #import <SentryScreenshot.h>
-#import <SentrySessionReplayIntegration.h>
 #import <SentryUser.h>
 
 #if SENTRY_TARGET_PROFILING_SUPPORTED
@@ -351,12 +350,12 @@ + (NSString *__nullable)getReplayId
 
 + (void)addReplayIgnoreClasses:(NSArray<Class> *_Nonnull)classes
 {
- [SentryViewPhotographer.shared addIgnoreClasses:classes];
+ [[PrivateSentrySDKOnly getReplayIntegration].viewPhotographer addIgnoreClasses:classes];
 }
 
 + (void)addReplayRedactClasses:(NSArray<Class> *_Nonnull)classes
 {
- [SentryViewPhotographer.shared addRedactClasses:classes];
+ [[PrivateSentrySDKOnly getReplayIntegration].viewPhotographer addRedactClasses:classes];
 }
 #endif
 

Sources/Sentry/SentrySessionReplayIntegration.m

@@ -62,6 +62,8 @@ - (BOOL)installWithOptions:(nonnull SentryOptions *)options
  }
 
  _replayOptions = options.experimental.sessionReplay;
+ _viewPhotographer =
+ [[SentryViewPhotographer alloc] initWithRedactOptions:options.experimental.sessionReplay];
 
  if (options.enableSwizzling) {
  _touchTracker = [[SentryTouchTracker alloc]
@@ -85,8 +87,6 @@ - (BOOL)installWithOptions:(nonnull SentryOptions *)options
  }];
 
  [SentryDependencyContainer.sharedInstance.reachability addObserver:self];
- [SentryViewPhotographer.shared addIgnoreClasses:_replayOptions.ignoreRedactViewTypes];
- [SentryViewPhotographer.shared addRedactClasses:_replayOptions.redactViewTypes];
 
  _installedInstance = self;
  return YES;
@@ -235,7 +235,7 @@ - (void)startWithOptions:(SentryReplayOptions *)replayOptions
  fullSession:(BOOL)shouldReplayFullSession
 {
  [self startWithOptions:replayOptions
- screenshotProvider:SentryViewPhotographer.shared
+ screenshotProvider:_viewPhotographer
  breadcrumbConverter:[[SentrySRDefaultBreadcrumbConverter alloc] init]
  fullSession:shouldReplayFullSession];
 }

Sources/Sentry/include/SentrySessionReplayIntegration+Private.h

@@ -5,12 +5,15 @@
 #if SENTRY_TARGET_REPLAY_SUPPORTED
 
 @class SentrySessionReplay;
+@class SentryViewPhotographer;
 
 @interface SentrySessionReplayIntegration () <SentryIntegrationProtocol, SentrySessionListener,
  SentrySessionReplayDelegate>
 
 @property (nonatomic, strong) SentrySessionReplay *sessionReplay;
 
+@property (nonatomic, strong) SentryViewPhotographer *viewPhotographer;
+
 @end
 
 #endif

Sources/Swift/Extensions/UIViewExtensions.swift

@@ -7,7 +7,7 @@ public extension UIView {
 
  /**
  * Marks this view to be redacted during replays.
- * - warning: This is an experimental feature and may still have bugs.
+ * - experiment: This is an experimental feature and may still have bugs.
  */
  func sentryReplayRedact() {
  SentryRedactViewHelper.redactView(self)
@@ -16,7 +16,7 @@ public extension UIView {
  /**
  * Marks this view to be ignored during redact step
  * of session replay. All its content will be visible in the replay.
- * - warning: This is an experimental feature and may still have bugs.
+ * - experiment: This is an experimental feature and may still have bugs.
  */
  func sentryReplayIgnore() {
  SentryRedactViewHelper.ignoreView(self)

Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift

@@ -70,17 +70,18 @@ public class SentryReplayOptions: NSObject, SentryRedactOptions {
  /**
  * A list of custom UIView subclasses that need 
  * to be masked during session replay.
- * By default Sentry already mask text elements from UIKit
+ * By default Sentry already mask text and image elements from UIKit
+ * Every child of a view that is redacted will also be redacted.
  */
- public var redactViewTypes = [AnyClass]()
+ public var redactViewClasses = [AnyClass]()
 
  /**
  * A list of custom UIView subclasses to be ignored
  * during masking step of the session replay.
- * The view itself and any child will be ignored and not masked.
+ * The views of given classes will not be redacted but their children may be.
  * This property has precedence over `redactViewTypes`.
  */
- public var ignoreRedactViewTypes = [AnyClass]()
+ public var ignoreViewClasses = [AnyClass]()
 
  /**
  * Defines the quality of the session replay.

Sources/Swift/Protocol/SentryRedactOptions.swift

@@ -4,4 +4,6 @@ import Foundation
 protocol SentryRedactOptions {
  var redactAllText: Bool { get }
  var redactAllImages: Bool { get }
+ var redactViewClasses: [AnyClass] { get }
+ var ignoreViewClasses: [AnyClass] { get }
 }

Sources/Swift/Tools/SentryViewPhotographer.swift

@@ -21,25 +21,26 @@ class DefaultViewRenderer: ViewRenderer {
 
 @objcMembers
 class SentryViewPhotographer: NSObject, SentryViewScreenshotProvider {
- static let shared = SentryViewPhotographer()
- private let redactBuilder = UIRedactBuilder()
+ private let redactBuilder: UIRedactBuilder
  private let dispatchQueue = SentryDispatchQueueWrapper()
 
  var renderer: ViewRenderer
 
- init(renderer: ViewRenderer) {
+ init(renderer: ViewRenderer, redactOptions: SentryRedactOptions) {
  self.renderer = renderer
+ redactBuilder = UIRedactBuilder(options: redactOptions)
  super.init()
  }
 
- private convenience override init() {
- self.init(renderer: DefaultViewRenderer())
+ init(redactOptions: SentryRedactOptions) {
+ self.renderer = DefaultViewRenderer()
+ self.redactBuilder = UIRedactBuilder(options: redactOptions)
  }
 
  func image(view: UIView, options: SentryRedactOptions, onComplete: @escaping ScreenshotCallback ) {
  let image = renderer.render(view: view)
 
- let redact = redactBuilder.redactRegionsFor(view: view, options: options)
+ let redact = redactBuilder.redactRegionsFor(view: view)
  let imageSize = view.bounds.size
  dispatchQueue.dispatchAsync {
  let screenshot = UIGraphicsImageRenderer(size: imageSize, format: .init(for: .init(displayScale: 1))).image { context in

Sources/Swift/Tools/UIRedactBuilder.swift

@@ -44,23 +44,58 @@ class UIRedactBuilder {
  private var ignoreClassesIdentifiers: Set<ObjectIdentifier>
  ///This is a list of UIView subclasses that need to be redacted from screenshot
  private var redactClassesIdentifiers: Set<ObjectIdentifier>
-
- init() {
 
- var redactClasses = [ UILabel.self, UITextView.self, UITextField.self ] +
- //this classes are used by SwiftUI to display images.
- ["_TtCOCV7SwiftUI11DisplayList11ViewUpdater8Platform13CGDrawingView",
- "_TtC7SwiftUIP33_A34643117F00277B93DEBAB70EC0697122_UIShapeHitTestingView",
- "SwiftUI._UIGraphicsView", "SwiftUI.ImageLayer", "UIWebView"
- ].compactMap { NSClassFromString($0) }
+ /**
+ Initializes a new instance of the redaction process with the specified options.
+
+ This initializer configures which `UIView` subclasses should be redacted from screenshots and which should be ignored during the redaction process.
+
+ - parameter options: A `SentryRedactOptions` object that specifies the configuration for the redaction process.
+ 
+ - If `options.redactAllText` is `true`, common text-related views such as `UILabel`, `UITextView`, and `UITextField` are redacted.
+ - If `options.redactAllImages` is `true`, common image-related views such as `UIImageView` and various internal `SwiftUI` image views are redacted.
+ - The `options.ignoreRedactViewTypes` allows specifying custom view types to be ignored during the redaction process.
+ - The `options.redactViewTypes` allows specifying additional custom view types to be redacted.
+
+ - note: On iOS, views such as `WKWebView` and `UIWebView` are automatically redacted, and controls like `UISlider` and `UISwitch` are ignored.
+ */
+ init(options: SentryRedactOptions) {
+ var redactClasses = [AnyClass]()
+
+ if options.redactAllText {
+ redactClasses += [ UILabel.self, UITextView.self, UITextField.self ]
+ }
+
+ if options.redactAllImages {
+ //this classes are used by SwiftUI to display images.
+ redactClasses += ["_TtCOCV7SwiftUI11DisplayList11ViewUpdater8Platform13CGDrawingView",
+ "_TtC7SwiftUIP33_A34643117F00277B93DEBAB70EC0697122_UIShapeHitTestingView",
+ "SwiftUI._UIGraphicsView", "SwiftUI.ImageLayer"
+ ].compactMap(NSClassFromString(_:))
+
+ redactClasses.append(UIImageView.self)
+ }
 
 #if os(iOS)
  redactClasses += [ WKWebView.self ]
+
+ //If we try to use 'UIWebView.self' it will not compile for macCatalyst, but the class does exists.
+ redactClasses += [ "UIWebView" ].compactMap(NSClassFromString(_:))
+
  ignoreClassesIdentifiers = [ ObjectIdentifier(UISlider.self), ObjectIdentifier(UISwitch.self) ]
 #else
  ignoreClassesIdentifiers = []
 #endif
+
  redactClassesIdentifiers = Set(redactClasses.map({ ObjectIdentifier($0) }))
+
+ for type in options.ignoreViewClasses {
+ self.ignoreClassesIdentifiers.insert(ObjectIdentifier(type))
+ }
+
+ for type in options.redactViewClasses {
+ self.redactClassesIdentifiers.insert(ObjectIdentifier(type))
+ }
  }
 
  func containsIgnoreClass(_ ignoreClass: AnyClass) -> Bool {
@@ -112,13 +147,12 @@ class UIRedactBuilder {
  
  This function returns the redaction regions in reverse order from what was found in the view hierarchy, allowing the processing of regions from top to bottom. This ensures that clip regions are applied first before drawing a redact mask on lower views.
  */
- func redactRegionsFor(view: UIView, options: SentryRedactOptions?) -> [RedactRegion] {
+ func redactRegionsFor(view: UIView) -> [RedactRegion] {
  var redactingRegions = [RedactRegion]()
 
  self.mapRedactRegion(fromView: view,
  redacting: &redactingRegions,
  rootFrame: view.frame,
- redactOptions: options ?? SentryReplayOptions(),
  transform: CGAffineTransform.identity)
 
  return redactingRegions.reversed()
@@ -128,14 +162,14 @@ class UIRedactBuilder {
  return SentryRedactViewHelper.shouldIgnoreView(view) || containsIgnoreClass(type(of: view))
  }
 
- private func shouldRedact(view: UIView, redactOptions: SentryRedactOptions) -> Bool {
+ private func shouldRedact(view: UIView) -> Bool {
  if SentryRedactViewHelper.shouldRedactView(view) {
  return true
  }
- if redactOptions.redactAllImages, let imageView = view as? UIImageView {
+ if let imageView = view as? UIImageView, containsRedactClass(UIImageView.self) {
  return shouldRedact(imageView: imageView)
  }
- return redactOptions.redactAllText && containsRedactClass(type(of: view))
+ return containsRedactClass(type(of: view))
  }
 
  private func shouldRedact(imageView: UIImageView) -> Bool {
@@ -145,22 +179,23 @@ class UIRedactBuilder {
  return image.imageAsset?.value(forKey: "_containingBundle") == nil
  }
 
- private func mapRedactRegion(fromView view: UIView, redacting: inout [RedactRegion], rootFrame: CGRect, redactOptions: SentryRedactOptions, transform: CGAffineTransform) {
- guard (redactOptions.redactAllImages || redactOptions.redactAllText) && !view.isHidden && view.alpha != 0 else { return }
+ private func mapRedactRegion(fromView view: UIView, redacting: inout [RedactRegion], rootFrame: CGRect, transform: CGAffineTransform, forceRedact: Bool = false) {
+ guard !redactClassesIdentifiers.isEmpty && !view.isHidden && view.alpha != 0 else { return }
 
  let layer = view.layer.presentation() ?? view.layer
 
  let newTransform = concatenateTranform(transform, with: layer)
 
- let ignore = shouldIgnore(view: view)
- let redact = shouldRedact(view: view, redactOptions: redactOptions)
+ let ignore = !forceRedact && shouldIgnore(view: view)
+ let redact = forceRedact || shouldRedact(view: view)
+ var enforceRedact = forceRedact
 
  if !ignore && redact {
  redacting.append(RedactRegion(size: layer.bounds.size, transform: newTransform, type: .redact, color: self.color(for: view)))
- return
- }
-
- if isOpaque(view) {
+
+  guard !view.clipsToBounds else { return }
+  enforceRedact = true
+ } else if isOpaque(view) {
  let finalViewFrame = CGRect(origin: .zero, size: layer.bounds.size).applying(newTransform)
  if isAxisAligned(newTransform) && finalViewFrame == rootFrame {
  //Because the current view is covering everything we found so far we can clear `redacting` list
@@ -170,15 +205,15 @@ class UIRedactBuilder {
  }
  }
 
- guard !ignore else { return }
+ guard view.subviews.count > 0 else { return }
 
  if view.clipsToBounds {
  /// Because the order in which we process the redacted regions is reversed, we add the end of the clip region first.
  /// The beginning will be added after all the subviews have been mapped.
  redacting.append(RedactRegion(size: layer.bounds.size, transform: newTransform, type: .clipEnd))
  }
  for subview in view.subviews.sorted(by: { $0.layer.zPosition < $1.layer.zPosition }) {
- mapRedactRegion(fromView: subview, redacting: &redacting, rootFrame: rootFrame, redactOptions: redactOptions, transform: newTransform)
+ mapRedactRegion(fromView: subview, redacting: &redacting, rootFrame: rootFrame, transform: newTransform, forceRedact: enforceRedact)
  }
  if view.clipsToBounds {
  redacting.append(RedactRegion(size: layer.bounds.size, transform: newTransform, type: .clipBegin))

Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift

@@ -282,28 +282,30 @@ class SentrySessionReplayIntegrationTests: XCTestCase {
  (sut as? SentryReachabilityObserver)?.connectivityChanged(true, typeDescription: "")
  XCTAssertFalse(sut.sessionReplay.isSessionPaused)
  }
-  
- func testMaskViewFromSDK() {
+
+ func testMaskViewFromSDK() throws {
  class AnotherLabel: UILabel {
  }
 
  startSDK(sessionSampleRate: 1, errorSampleRate: 1) { options in
- options.experimental.sessionReplay.redactViewTypes = [AnotherLabel.self]
+ options.experimental.sessionReplay.redactViewClasses = [AnotherLabel.self]
  }
-
- let redactBuilder = SentryViewPhotographer.shared.getRedactBuild()
+
+ let sut = try getSut()
+ let redactBuilder = sut.viewPhotographer.getRedactBuild()
  XCTAssertTrue(redactBuilder.containsRedactClass(AnotherLabel.self))
  }
 
- func testIgnoreViewFromSDK() {
+ func testIgnoreViewFromSDK() throws {
  class AnotherLabel: UILabel {
  }
 
  startSDK(sessionSampleRate: 1, errorSampleRate: 1) { options in
- options.experimental.sessionReplay.ignoreRedactViewTypes = [AnotherLabel.self]
+ options.experimental.sessionReplay.ignoreViewClasses = [AnotherLabel.self]
  }
 
- let redactBuilder = SentryViewPhotographer.shared.getRedactBuild()
+ let sut = try getSut()
+ let redactBuilder = sut.viewPhotographer.getRedactBuild()
  XCTAssertTrue(redactBuilder.containsIgnoreClass(AnotherLabel.self))
  }
 

Tests/SentryTests/SentryViewPhotographerTests.swift

@@ -15,18 +15,8 @@ class SentryViewPhotographerTests: XCTestCase {
  }
  }
 
- private class RedactOptions: SentryRedactOptions {
- var redactAllText: Bool
- var redactAllImages: Bool
-
- init(redactAllText: Bool = true, redactAllImages: Bool = true) {
- self.redactAllText = redactAllText
- self.redactAllImages = redactAllImages
- }
- }
-
  func sut() -> SentryViewPhotographer {
- return SentryViewPhotographer(renderer: TestViewRenderer())
+ return SentryViewPhotographer(renderer: TestViewRenderer(), redactOptions: RedactOptions())
  }
 
  private func prepare(views: [UIView], options: any SentryRedactOptions = RedactOptions()) -> UIImage? {