Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Session replay transformed view masking #4529

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

### Fixes

- Make `Scope.span` fully thread safe (#4519)
- Keep PropagationContext when cloning scope (#4518)
- Session replay transformed view masking (#4529)
- UIViewController with Xcode 16 in debug (#4523). The Xcode 16 build setting [ENABLE_DEBUG_DYLIB](https://developer.apple.com/documentation/xcode/build-settings-reference#Enable-Debug-Dylib-Support), which is turned on by default only in debug, could lead to missing UIViewController traces.
- Concurrency crash with Swift 6 (#4512)
- Make `Scope.span` fully thread safe (#4519)
Expand Down
2 changes: 1 addition & 1 deletion Samples/iOS-Swift/iOS-Swift/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
options.debug = true

if #available(iOS 16.0, *), !args.contains("--disable-session-replay") {
options.experimental.sessionReplay = SentryReplayOptions(sessionSampleRate: 1, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true)
options.experimental.sessionReplay = SentryReplayOptions(sessionSampleRate: 0, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true)
brustolin marked this conversation as resolved.
Show resolved Hide resolved
options.experimental.sessionReplay.quality = .high
}

Expand Down
9 changes: 5 additions & 4 deletions Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="23094" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES" initialViewController="5CD-RQ-aBU">
<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="23504" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES" initialViewController="5CD-RQ-aBU">
<device id="retina4_0" orientation="portrait" appearance="light"/>
<dependencies>
<deployment identifier="iOS"/>
<plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="23084"/>
<plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="23506"/>
<capability name="Image references" minToolsVersion="12.0"/>
<capability name="Safe area layout guides" minToolsVersion="9.0"/>
<capability name="System colors in document resources" minToolsVersion="11.0"/>
Expand Down Expand Up @@ -1194,7 +1194,8 @@
<navigationItem key="navigationItem" id="IKp-8n-e0m"/>
<connections>
<outlet property="label" destination="QLx-Ff-Zy1" id="gnc-AD-TCJ"/>
<outlet property="notRedactedView" destination="Nch-qj-FJO" id="eTE-nC-OQb"/>
<outlet property="notRedactedLabel" destination="zye-8M-uzl" id="uri-pC-cHU"/>
<outlet property="notRedactedView" destination="Nch-qj-FJO" id="ZMj-72-PkX"/>
</connections>
</viewController>
<placeholder placeholderIdentifier="IBFirstResponder" id="wpE-MN-0Ua" userLabel="First Responder" customClass="UIResponder" sceneMemberID="firstResponder"/>
Expand Down Expand Up @@ -1279,7 +1280,7 @@
<color white="1" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/>
</systemColor>
<systemColor name="systemGray5Color">
<color red="0.8980392157" green="0.8980392157" blue="0.91764705879999997" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
<color red="0.89803921568627454" green="0.89803921568627454" blue="0.91764705882352937" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
</systemColor>
</resources>
</document>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Foundation
class SRRedactSampleViewController: UIViewController {

@IBOutlet var notRedactedView: UIView!
@IBOutlet var notRedactedLabel: UILabel!

@IBOutlet var label: UILabel!

Expand All @@ -11,7 +12,6 @@ class SRRedactSampleViewController: UIViewController {

notRedactedView.backgroundColor = .green
notRedactedView.transform = CGAffineTransform(rotationAngle: 45 * .pi / 180.0)

SentrySDK.replay.maskView(notRedactedView)
SentrySDK.replay.unmaskView(notRedactedLabel)
}
}
38 changes: 28 additions & 10 deletions Sources/Swift/Tools/SentryViewPhotographer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class SentryViewPhotographer: NSObject, SentryViewScreenshotProvider {
self.renderer = DefaultViewRenderer()
self.redactBuilder = UIRedactBuilder(options: redactOptions)
}

func image(view: UIView, options: SentryRedactOptions, onComplete: @escaping ScreenshotCallback ) {
let image = renderer.render(view: view)

Expand All @@ -45,6 +45,9 @@ class SentryViewPhotographer: NSObject, SentryViewScreenshotProvider {
dispatchQueue.dispatchAsync {
let screenshot = UIGraphicsImageRenderer(size: imageSize, format: .init(for: .init(displayScale: 1))).image { context in

let clipOutPath = CGMutablePath(rect: CGRect(origin: .zero, size: imageSize), transform: nil)
brustolin marked this conversation as resolved.
Show resolved Hide resolved
var clipPaths = [CGPath]()

let imageRect = CGRect(origin: .zero, size: imageSize)
context.cgContext.addRect(CGRect(origin: CGPoint.zero, size: imageSize))
context.cgContext.clip(using: .evenOdd)
Expand All @@ -62,30 +65,45 @@ class SentryViewPhotographer: NSObject, SentryViewScreenshotProvider {
defer { latestRegion = region }

guard latestRegion?.canReplace(as: region) != true && imageRect.intersects(path.boundingBoxOfPath) else { continue }

switch region.type {
case .redact, .redactSwiftUI:
(region.color ?? UIImageHelper.averageColor(of: context.currentImage, at: rect.applying(region.transform))).setFill()
context.cgContext.addPath(path)
context.cgContext.fillPath()
case .clipOut:
context.cgContext.addRect(context.cgContext.boundingBoxOfClipPath)
context.cgContext.addPath(path)
context.cgContext.clip(using: .evenOdd)
clipOutPath.addPath(path)
self.updateClipping(for: context.cgContext,
clipPaths: clipPaths,
clipOutPath: clipOutPath)
case .clipBegin:
context.cgContext.saveGState()
context.cgContext.resetClip()
context.cgContext.addPath(path)
context.cgContext.clip()
clipPaths.append(path)
self.updateClipping(for: context.cgContext,
clipPaths: clipPaths,
clipOutPath: clipOutPath)
case .clipEnd:
context.cgContext.restoreGState()
clipPaths.removeLast()
self.updateClipping(for: context.cgContext,
clipPaths: clipPaths,
clipOutPath: clipOutPath)
}
}
}
onComplete(screenshot)
}
}

private func updateClipping(for context: CGContext, clipPaths: [CGPath], clipOutPath: CGPath) {
context.resetClip()
clipPaths.reversed().forEach {
context.addPath($0)
context.clip()
}

context.addPath(clipOutPath)
context.clip(using: .evenOdd)
}

@objc(addIgnoreClasses:)
func addIgnoreClasses(classes: [AnyClass]) {
redactBuilder.addIgnoreClasses(classes)
Expand Down
17 changes: 10 additions & 7 deletions Sources/Swift/Tools/UIRedactBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ class UIRedactBuilder {
self.mapRedactRegion(fromView: view,
relativeTo: nil,
redacting: &redactingRegions,
rootFrame: view.frame)
rootFrame: view.frame,
transform: .identity)

var swiftUIRedact = [RedactRegion]()
var otherRegions = [RedactRegion]()
Expand Down Expand Up @@ -237,12 +238,12 @@ class UIRedactBuilder {
return image.imageAsset?.value(forKey: "_containingBundle") == nil
}

private func mapRedactRegion(fromView view: UIView, relativeTo parentLayer: CALayer?, redacting: inout [RedactRegion], rootFrame: CGRect, forceRedact: Bool = false) {
private func mapRedactRegion(fromView view: UIView, relativeTo parentLayer: CALayer?, 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 = getTranform(from: layer, withParent: parentLayer)
let newTransform = concatenateTranform(transform, from: layer, withParent: parentLayer)

let ignore = !forceRedact && shouldIgnore(view: view)
let swiftUI = SentryRedactViewHelper.shouldRedactSwiftUI(view)
Expand Down Expand Up @@ -272,7 +273,7 @@ class UIRedactBuilder {
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, relativeTo: layer, redacting: &redacting, rootFrame: rootFrame, forceRedact: enforceRedact)
mapRedactRegion(fromView: subview, relativeTo: layer, redacting: &redacting, rootFrame: rootFrame, transform: newTransform, forceRedact: enforceRedact)
}
if view.clipsToBounds {
redacting.append(RedactRegion(size: layer.bounds.size, transform: newTransform, type: .clipBegin))
Expand All @@ -282,12 +283,14 @@ class UIRedactBuilder {
/**
Gets a transform that represents the layer global position.
*/
private func getTranform(from layer: CALayer, withParent parentLayer: CALayer?) -> CGAffineTransform {
private func concatenateTranform(_ transform: CGAffineTransform, from layer: CALayer, withParent parentLayer: CALayer?) -> CGAffineTransform {
let size = layer.bounds.size
let anchorPoint = CGPoint(x: size.width * layer.anchorPoint.x, y: size.height * layer.anchorPoint.y)
let position = parentLayer?.convert(layer.position, to: nil) ?? layer.position

var newTransform = CGAffineTransform(translationX: position.x, y: position.y)

var newTransform = transform
newTransform.tx = position.x
newTransform.ty = position.y
newTransform = CATransform3DGetAffineTransform(layer.transform).concatenating(newTransform)
return newTransform.translatedBy(x: -anchorPoint.x, y: -anchorPoint.y)
}
Expand Down
65 changes: 65 additions & 0 deletions Tests/SentryTests/SentryViewPhotographerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,33 @@ class SentryViewPhotographerTests: XCTestCase {
assertColor(pixel2, .green)
}

func testRedactLabelWithParentTransformed() throws {
let label = UILabel(frame: CGRect(x: 0, y: 0, width: 50, height: 25))
label.text = "Test"

let parentView = UIView(frame: CGRect(x: 0, y: 12.5, width: 50, height: 25))
parentView.backgroundColor = .green
parentView.transform = CGAffineTransform(rotationAngle: .pi / 2)
parentView.addSubview(label)

let image = try XCTUnwrap(prepare(views: [parentView] ))
assertColor(.white, in: image, at: [
CGPoint(x: 2, y: 2),
CGPoint(x: 10, y: 2),
CGPoint(x: 2, y: 47),
CGPoint(x: 10, y: 47),
CGPoint(x: 39, y: 2),
CGPoint(x: 39, y: 47)
])

assertColor(.black, in: image, at: [
CGPoint(x: 13, y: 2),
CGPoint(x: 35, y: 2),
CGPoint(x: 13, y: 47),
CGPoint(x: 35, y: 47)
])
}

func testDontRedactClippedLabel() throws {
let label = UILabel(frame: CGRect(x: 0, y: 25, width: 50, height: 25))
label.text = "Test"
Expand Down Expand Up @@ -210,6 +237,44 @@ class SentryViewPhotographerTests: XCTestCase {
assertColor(pixel1, .green)
}

func testNotMaskingLabelInsideclippedViewHiddenByAnOpaqueExternalView() throws {
brustolin marked this conversation as resolved.
Show resolved Hide resolved
let topView = UIView(frame: CGRect(x: 25, y: 0, width: 25, height: 25))
topView.backgroundColor = .green

let label1 = UILabel(frame: CGRect(x: 0, y: 0, width: 50, height: 25))
label1.text = "Test"
label1.textColor = .black

let parentView = UIView(frame: CGRect(x: 0, y: 0, width: 50, height: 25))
parentView.addSubview(label1)
parentView.clipsToBounds = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: If we want to test clipsToBounds, shouldn't the label1 be bigger than the parentView. Currently, they are the same size, and clipsToBounds shouldn't make a difference. Or is there another reason to use it?

I think if you set the background color of the parentView to red and increase the height of the label1 to 30, no red color should be visible, cause the label and the topView should be drawn over the parentView.

Copy link
Contributor Author

@brustolin brustolin Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this test. I just need the parent to clip its bounds because it changes the flow of masking.


let image = try XCTUnwrap(prepare(views: [parentView, topView]))
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved

assertColor(.green, in: image, at: [
CGPoint(x: 27, y: 3),
CGPoint(x: 27, y: 22),
CGPoint(x: 35, y: 12),
CGPoint(x: 47, y: 3),
CGPoint(x: 47, y: 22)
])

assertColor(.black, in: image, at: [
CGPoint(x: 3, y: 3),
CGPoint(x: 3, y: 22),
CGPoint(x: 12, y: 12),
CGPoint(x: 22, y: 3),
CGPoint(x: 22, y: 22)
])
}

private func assertColor(_ color: UIColor, in image: UIImage, at points: [CGPoint]) {
points.forEach {
let pixel = self.color(at: $0, in: image)
assertColor(color, pixel)
}
}

private func assertColor(_ color1: UIColor, _ color2: UIColor) {
let sRGBColor1 = color1.cgColor.converted(to: CGColorSpace(name: CGColorSpace.sRGB)!, intent: .defaultIntent, options: nil)
let sRGBColor2 = color2.cgColor.converted(to: CGColorSpace(name: CGColorSpace.sRGB)!, intent: .defaultIntent, options: nil)
Expand Down
Loading