From fa684554d1bf84b9de32fd9095b3a8d6aa0680a1 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 25 Sep 2024 21:24:23 +0200 Subject: [PATCH 01/21] refactor: change redaction logic to custom filters --- flutter/lib/src/replay/recorder.dart | 7 +- flutter/lib/src/replay/widget_filter.dart | 75 ++++++++++++++------- flutter/lib/src/sentry_replay_options.dart | 68 ++++++++++++++++++- flutter/test/replay/widget_filter_test.dart | 39 ++++++----- 4 files changed, 140 insertions(+), 49 deletions(-) diff --git a/flutter/lib/src/replay/recorder.dart b/flutter/lib/src/replay/recorder.dart index 847c3a75f6..916fa778e4 100644 --- a/flutter/lib/src/replay/recorder.dart +++ b/flutter/lib/src/replay/recorder.dart @@ -22,11 +22,8 @@ class ScreenshotRecorder { ScreenshotRecorder(this.config, this.options) { final replayOptions = options.experimental.replay; - if (replayOptions.redactAllText || replayOptions.redactAllImages) { - _widgetFilter = WidgetFilter( - redactText: replayOptions.redactAllText, - redactImages: replayOptions.redactAllImages, - logger: options.logger); + if (replayOptions.maskingConfig.isNotEmpty) { + _widgetFilter = WidgetFilter(replayOptions.maskingConfig, options.logger); } } diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index 1f66a42f1a..0b5bf9cab7 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -1,5 +1,4 @@ import 'package:flutter/rendering.dart'; -import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; @@ -10,20 +9,13 @@ import '../sentry_asset_bundle.dart'; class WidgetFilter { final items = []; final SentryLogger logger; - final bool redactText; - final bool redactImages; + final Map config; static const _defaultColor = Color.fromARGB(255, 0, 0, 0); late double _pixelRatio; late Rect _bounds; final _warnedWidgets = {}; - final AssetBundle _rootAssetBundle; - WidgetFilter( - {required this.redactText, - required this.redactImages, - required this.logger, - @visibleForTesting AssetBundle? rootAssetBundle}) - : _rootAssetBundle = rootAssetBundle ?? rootBundle; + WidgetFilter(this.config, this.logger); void obscure(BuildContext context, double pixelRatio, Rect bounds) { _pixelRatio = pixelRatio; @@ -55,21 +47,20 @@ class WidgetFilter { @pragma('vm:prefer-inline') bool _obscureIfNeeded(Element element, Widget widget) { - Color? color; + final maskingConfig = config[widget.runtimeType]; + if (maskingConfig == null) { + return false; + } else if (!maskingConfig.shouldMask(element, widget)) { + logger(SentryLevel.debug, "WidgetFilter skipping: $widget"); + return false; + } - if (redactText && widget is Text) { + Color? color; + if (widget is Text) { color = widget.style?.color; - } else if (redactText && widget is EditableText) { + } else if (widget is EditableText) { color = widget.style.color; - } else if (redactImages && widget is Image) { - if (widget.image is AssetBundleImageProvider) { - final image = widget.image as AssetBundleImageProvider; - if (isBuiltInAssetImage(image)) { - logger(SentryLevel.debug, - "WidgetFilter skipping asset: $widget ($image)."); - return false; - } - } + } else if (widget is Image) { color = widget.color; } else { // No other type is currently obscured. @@ -128,9 +119,10 @@ class WidgetFilter { return true; } - @visibleForTesting + @internal @pragma('vm:prefer-inline') - bool isBuiltInAssetImage(AssetBundleImageProvider image) { + static bool isBuiltInAssetImage( + AssetBundleImageProvider image, AssetBundle rootAssetBundle) { late final AssetBundle? bundle; if (image is AssetImage) { bundle = image.bundle; @@ -140,8 +132,8 @@ class WidgetFilter { return false; } return (bundle == null || - bundle == _rootAssetBundle || - (bundle is SentryAssetBundle && bundle.bundle == _rootAssetBundle)); + bundle == rootAssetBundle || + (bundle is SentryAssetBundle && bundle.bundle == rootAssetBundle)); } @pragma('vm:prefer-inline') @@ -165,9 +157,40 @@ class WidgetFilter { } } +@internal class WidgetFilterItem { final Color color; final Rect bounds; const WidgetFilterItem(this.color, this.bounds); } + +@internal +class WidgetFilterMaskingConfig { + static const mask = WidgetFilterMaskingConfig._(1, 'mask'); + static const show = WidgetFilterMaskingConfig._(2, 'mask'); + + final int index; + final String _name; + final bool Function(Element, Widget)? _shouldMask; + + const WidgetFilterMaskingConfig._(this.index, this._name) + : _shouldMask = null; + const WidgetFilterMaskingConfig.custom(this._shouldMask) + : index = 3, + _name = 'custom'; + + @override + String toString() => "$WidgetFilterMaskingConfig.$_name"; + + bool shouldMask(Element element, Widget widget) { + switch (this) { + case mask: + return true; + case show: + return false; + default: + return _shouldMask!(element, widget); + } + } +} diff --git a/flutter/lib/src/sentry_replay_options.dart b/flutter/lib/src/sentry_replay_options.dart index e52fbb2877..33b03f6001 100644 --- a/flutter/lib/src/sentry_replay_options.dart +++ b/flutter/lib/src/sentry_replay_options.dart @@ -1,7 +1,16 @@ +import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; +import 'replay/widget_filter.dart'; + /// Configuration of the experimental replay feature. class SentryReplayOptions { + SentryReplayOptions() { + redactAllText = true; + redactAllImages = true; + } + double? _sessionSampleRate; /// A percentage of sessions in which a replay will be created. @@ -27,12 +36,67 @@ class SentryReplayOptions { /// Redact all text content. Draws a rectangle of text bounds with text color /// on top. Currently, only [Text] and [EditableText] Widgets are redacted. /// Default is enabled. - var redactAllText = true; + set redactAllText(bool value) { + if (value) { + mask(Text); + mask(EditableText); + } else { + unmask(Text); + unmask(EditableText); + } + } /// Redact all image content. Draws a rectangle of image bounds with image's /// dominant color on top. Currently, only [Image] widgets are redacted. /// Default is enabled. - var redactAllImages = true; + set redactAllImages(bool value) { + if (value) { + maskIfTrue(Image, (element, widget) { + widget as Image; + if (widget.image is AssetBundleImageProvider) { + final image = widget.image as AssetBundleImageProvider; + if (WidgetFilter.isBuiltInAssetImage(image, rootBundle)) { + return false; + } + } + return true; + }); + } else { + unmask(Image); + } + } + + Map _maskingConfig = {}; + bool _finished = false; + + /// Once accessed, masking confing cannot change anymore. + @internal + Map get maskingConfig { + if (_finished) { + return _maskingConfig; + } + _finished = true; + final result = + Map.unmodifiable(_maskingConfig); + _maskingConfig = result; + return result; + } + + /// Mask given widget type in the replay. + void mask(Type type) { + _maskingConfig[type] = WidgetFilterMaskingConfig.mask; + } + + /// Unmask given widget type in the replay. + void unmask(Type type) { + _maskingConfig.remove(type); + } + + /// Unmask given widget type in the replay if it's masked by default rules + /// [redactAllText] or [redactAllImages]. + void maskIfTrue(Type type, bool Function(Element, Widget) shouldMask) { + _maskingConfig[type] = WidgetFilterMaskingConfig.custom(shouldMask); + } @internal bool get isEnabled => diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index e5787431bd..9c440b3a41 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -13,13 +13,13 @@ void main() async { final rootBundle = TestAssetBundle(); final otherBundle = TestAssetBundle(); - final createSut = - ({bool redactImages = false, bool redactText = false}) => WidgetFilter( - logger: (level, message, {exception, logger, stackTrace}) {}, - redactImages: redactImages, - redactText: redactText, - rootAssetBundle: rootBundle, - ); + final createSut = ({bool redactImages = false, bool redactText = false}) { + final replayOptions = SentryReplayOptions(); + replayOptions.redactAllImages = redactImages; + replayOptions.redactAllText = redactText; + return WidgetFilter(replayOptions.maskingConfig, + (level, message, {exception, logger, stackTrace}) {}); + }; boundsRect(WidgetFilterItem item) => '${item.bounds.width.floor()}x${item.bounds.height.floor()}'; @@ -75,20 +75,27 @@ void main() async { testWidgets( 'recognizes ${newAssetImage('').runtimeType} from the root bundle', (tester) async { - final sut = createSut(redactImages: true); - - expect(sut.isBuiltInAssetImage(newAssetImage('')), isTrue); - expect(sut.isBuiltInAssetImage(newAssetImage('', bundle: rootBundle)), + expect(WidgetFilter.isBuiltInAssetImage(newAssetImage(''), rootBundle), + isTrue); + expect( + WidgetFilter.isBuiltInAssetImage( + newAssetImage('', bundle: rootBundle), rootBundle), isTrue); - expect(sut.isBuiltInAssetImage(newAssetImage('', bundle: otherBundle)), + expect( + WidgetFilter.isBuiltInAssetImage( + newAssetImage('', bundle: otherBundle), rootBundle), isFalse); expect( - sut.isBuiltInAssetImage(newAssetImage('', - bundle: SentryAssetBundle(bundle: rootBundle))), + WidgetFilter.isBuiltInAssetImage( + newAssetImage('', + bundle: SentryAssetBundle(bundle: rootBundle)), + rootBundle), isTrue); expect( - sut.isBuiltInAssetImage(newAssetImage('', - bundle: SentryAssetBundle(bundle: otherBundle))), + WidgetFilter.isBuiltInAssetImage( + newAssetImage('', + bundle: SentryAssetBundle(bundle: otherBundle)), + rootBundle), isFalse); }); } From ec669f47322836a3df0759ea344c5a8087b58177 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 26 Sep 2024 14:31:30 +0200 Subject: [PATCH 02/21] refactor widget filter to handle errors gracefully --- flutter/lib/src/replay/widget_filter.dart | 116 ++++++++++++++-------- 1 file changed, 77 insertions(+), 39 deletions(-) diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index 0b5bf9cab7..1d4cbca1c3 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -22,13 +22,13 @@ class WidgetFilter { _bounds = bounds; items.clear(); if (context is Element) { - _obscure(context); + _process(context); } else { - context.visitChildElements(_obscure); + context.visitChildElements(_process); } } - void _obscure(Element element) { + void _process(Element element) { final widget = element.widget; if (!_isVisible(widget)) { @@ -39,46 +39,73 @@ class WidgetFilter { return; } - final obscured = _obscureIfNeeded(element, widget); - if (!obscured) { - element.visitChildElements(_obscure); + if (!_shouldObscure(element, widget)) { + // If this element should not be obscured, visit and check its children. + element.visitChildElements(_process); + } else { + final item = _obscureElementOrParent(element, widget); + if (item != null) { + items.add(item); + } } } @pragma('vm:prefer-inline') - bool _obscureIfNeeded(Element element, Widget widget) { + bool _shouldObscure(Element element, Widget widget) { + // Check if we should mask this widget based on the configuration. final maskingConfig = config[widget.runtimeType]; if (maskingConfig == null) { return false; } else if (!maskingConfig.shouldMask(element, widget)) { - logger(SentryLevel.debug, "WidgetFilter skipping: $widget"); - return false; - } - - Color? color; - if (widget is Text) { - color = widget.style?.color; - } else if (widget is EditableText) { - color = widget.style.color; - } else if (widget is Image) { - color = widget.color; - } else { - // No other type is currently obscured. + assert(() { + logger(SentryLevel.debug, "WidgetFilter skipping: $widget"); + return true; + }()); return false; } + return true; + } - final renderObject = element.renderObject; - if (renderObject is! RenderBox) { - _cantObscure(widget, "its renderObject is not a RenderBox"); - return false; + /// Determine the color and bounding box of the widget. + /// If the widget is offscreen, returns null. + /// If the widget cannot be obscured, obscures the parent. + @pragma('vm:prefer-inline') + WidgetFilterItem? _obscureElementOrParent(Element element, Widget widget) { + while (true) { + try { + return _obscure(element, widget); + } catch (e, stackTrace) { + final parent = element.parent; + if (!_warnedWidgets.contains(widget.hashCode)) { + _warnedWidgets.add(widget.hashCode); + logger( + SentryLevel.warning, + 'WidgetFilter cannot obscure widget $widget: $e.' + 'Obscuring the parent instead: ${parent?.widget}.', + stackTrace: stackTrace); + } + if (parent == null) { + return WidgetFilterItem(_defaultColor, _bounds); + } + element = parent; + widget = element.widget; + } } + } - var rect = _boundingBox(renderObject); + /// Determine the color and bounding box of the widget. + /// If the widget is offscreen, returns null. + /// This function may throw in which case the caller is responsible for + /// calling it again on the parent element. + @pragma('vm:prefer-inline') + WidgetFilterItem? _obscure(Element element, Widget widget) { + final RenderBox renderBox = element.renderObject as RenderBox; + var rect = _boundingBox(renderBox); // If it's a clipped render object, use parent's offset and size. // This helps with text fields which often have oversized render objects. - if (renderObject.parent is RenderStack) { - final renderStack = (renderObject.parent as RenderStack); + if (renderBox.parent is RenderStack) { + final renderStack = (renderBox.parent as RenderStack); final clipBehavior = renderStack.clipBehavior; if (clipBehavior == Clip.hardEdge || clipBehavior == Clip.antiAlias || @@ -93,19 +120,28 @@ class WidgetFilter { logger(SentryLevel.debug, "WidgetFilter skipping offscreen: $widget"); return true; }()); - return false; + return null; } - items.add(WidgetFilterItem(color ?? _defaultColor, rect)); assert(() { logger(SentryLevel.debug, "WidgetFilter obscuring: $widget"); return true; }()); - return true; + Color? color; + if (widget is Text) { + color = (widget).style?.color; + } else if (widget is EditableText) { + color = (widget).style.color; + } else if (widget is Image) { + color = (widget).color; + } + + return WidgetFilterItem(color ?? _defaultColor, rect); } // We cut off some widgets early because they're not visible at all. + @pragma('vm:prefer-inline') bool _isVisible(Widget widget) { if (widget is Visibility) { return widget.visible; @@ -136,15 +172,6 @@ class WidgetFilter { (bundle is SentryAssetBundle && bundle.bundle == rootAssetBundle)); } - @pragma('vm:prefer-inline') - void _cantObscure(Widget widget, String message) { - if (!_warnedWidgets.contains(widget.hashCode)) { - _warnedWidgets.add(widget.hashCode); - logger(SentryLevel.warning, - "WidgetFilter cannot obscure widget $widget: $message"); - } - } - @pragma('vm:prefer-inline') Rect _boundingBox(RenderBox box) { final offset = box.localToGlobal(Offset.zero); @@ -194,3 +221,14 @@ class WidgetFilterMaskingConfig { } } } + +extension on Element { + Element? get parent { + Element? result; + visitAncestorElements((el) { + result = el; + return false; + }); + return result; + } +} From ad0f33ceada4be4b2bced08ee7c3368629ea3856 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 26 Sep 2024 16:42:47 +0200 Subject: [PATCH 03/21] cleanup --- flutter/lib/src/replay/widget_filter.dart | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index 1d4cbca1c3..1d0032db82 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -195,7 +195,6 @@ class WidgetFilterItem { @internal class WidgetFilterMaskingConfig { static const mask = WidgetFilterMaskingConfig._(1, 'mask'); - static const show = WidgetFilterMaskingConfig._(2, 'mask'); final int index; final String _name; @@ -204,22 +203,15 @@ class WidgetFilterMaskingConfig { const WidgetFilterMaskingConfig._(this.index, this._name) : _shouldMask = null; const WidgetFilterMaskingConfig.custom(this._shouldMask) - : index = 3, + : index = 2, _name = 'custom'; @override String toString() => "$WidgetFilterMaskingConfig.$_name"; - bool shouldMask(Element element, Widget widget) { - switch (this) { - case mask: - return true; - case show: - return false; - default: - return _shouldMask!(element, widget); - } - } + @pragma('vm:prefer-inline') + bool shouldMask(Element element, Widget widget) => + this == mask ? true : _shouldMask!(element, widget); } extension on Element { From 665d01dbd186fa5696d4103b77ce7cd154fbdd6b Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 26 Sep 2024 17:59:35 +0200 Subject: [PATCH 04/21] add an option to disable masking asset images --- flutter/lib/src/sentry_replay_options.dart | 48 +++++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/flutter/lib/src/sentry_replay_options.dart b/flutter/lib/src/sentry_replay_options.dart index 33b03f6001..f1801d66d5 100644 --- a/flutter/lib/src/sentry_replay_options.dart +++ b/flutter/lib/src/sentry_replay_options.dart @@ -48,19 +48,33 @@ class SentryReplayOptions { /// Redact all image content. Draws a rectangle of image bounds with image's /// dominant color on top. Currently, only [Image] widgets are redacted. - /// Default is enabled. + /// Default is enabled (except for asset images, see [maskAssetImages]). + bool _maskImages = true; set redactAllImages(bool value) { + _maskImages = value; + if (value) { + if (_maskAssetImages) { + mask(Image); + } else { + maskIfTrue(Image, _maskImagesExceptAssets); + } + } else { + unmask(Image); + } + } + + /// Redact asset iamges. + bool _maskAssetImages = false; + set maskAssetImages(bool value) { + if (_maskAssetImages == value) { + return; + } + _maskAssetImages = value; if (value) { - maskIfTrue(Image, (element, widget) { - widget as Image; - if (widget.image is AssetBundleImageProvider) { - final image = widget.image as AssetBundleImageProvider; - if (WidgetFilter.isBuiltInAssetImage(image, rootBundle)) { - return false; - } - } - return true; - }); + assert(_maskImages); + mask(Image); + } else if (_maskImages) { + maskIfTrue(Image, _maskImagesExceptAssets); } else { unmask(Image); } @@ -102,3 +116,15 @@ class SentryReplayOptions { bool get isEnabled => ((sessionSampleRate ?? 0) > 0) || ((onErrorSampleRate ?? 0) > 0); } + +bool _maskImagesExceptAssets(Element element, Widget widget) { + if (widget is Image) { + final image = widget.image; + if (image is AssetBundleImageProvider) { + if (WidgetFilter.isBuiltInAssetImage(image, rootBundle)) { + return false; + } + } + } + return true; +} From bd5d026038544e53a3cf122d799b11231d9a343f Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 30 Sep 2024 17:47:49 +0200 Subject: [PATCH 05/21] add new masking config class --- flutter/lib/src/replay/masking_config.dart | 48 +++++++++ flutter/test/replay/masking_config_test.dart | 108 +++++++++++++++++++ flutter/test/replay/test_widget.dart | 14 ++- 3 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 flutter/lib/src/replay/masking_config.dart create mode 100644 flutter/test/replay/masking_config_test.dart diff --git a/flutter/lib/src/replay/masking_config.dart b/flutter/lib/src/replay/masking_config.dart new file mode 100644 index 0000000000..9d5727738a --- /dev/null +++ b/flutter/lib/src/replay/masking_config.dart @@ -0,0 +1,48 @@ +import 'package:flutter/widgets.dart'; +import 'package:meta/meta.dart'; + +@internal +class SentryMaskingConfig { + final List rules; + final int length; + + SentryMaskingConfig(List rules) + // Note: fixed-size list has performance benefits over growable list. + : rules = List.of(rules, growable: false), + length = rules.length; + + bool shouldMask(Element element, T widget) { + for (int i = 0; i < length; i++) { + if (rules[i].appliesTo(widget) && rules[i].shouldMask(element, widget)) { + return true; + } + } + return false; + } +} + +@internal +abstract class SentryMaskingRule { + bool appliesTo(Widget widget) => widget is T; + bool shouldMask(Element element, T widget); + + const SentryMaskingRule(); +} + +@internal +class SentryMaskingCustomRule extends SentryMaskingRule { + final bool Function(Element element, T widget) callback; + + const SentryMaskingCustomRule(this.callback); + + @override + bool shouldMask(Element element, T widget) => callback(element, widget); +} + +@internal +class SentryMaskingTruthyRule extends SentryMaskingRule { + const SentryMaskingTruthyRule(); + + @override + bool shouldMask(Element element, T widget) => true; +} diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart new file mode 100644 index 0000000000..2410e26bdb --- /dev/null +++ b/flutter/test/replay/masking_config_test.dart @@ -0,0 +1,108 @@ +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:sentry_flutter/src/replay/masking_config.dart'; + +import 'test_widget.dart'; + +void main() async { + TestWidgetsFlutterBinding.ensureInitialized(); + + testWidgets('will not mask if there are no rules', (tester) async { + final sut = SentryMaskingConfig([]); + final element = await pumpTestElement(tester); + expect(sut.rules, isEmpty); + expect(sut.shouldMask(element, element.widget), isFalse); + }); + + group('$SentryMaskingTruthyRule', () { + testWidgets('will mask widget by type', (tester) async { + final sut = SentryMaskingConfig([SentryMaskingTruthyRule()]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), isTrue); + }); + + testWidgets('will mask subtype widget by type', (tester) async { + final sut = SentryMaskingConfig([SentryMaskingTruthyRule()]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), isTrue); + }); + + testWidgets('will not mask widget of a different type', (tester) async { + final sut = SentryMaskingConfig([SentryMaskingTruthyRule()]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), isFalse); + }); + }); + + group('$SentryMaskingCustomRule', () { + testWidgets('only called for specified type', (tester) async { + final called = {}; + final sut = SentryMaskingConfig([ + SentryMaskingCustomRule((e, w) { + called[w.runtimeType] = (called[w.runtimeType] ?? 0) + 1; + return false; + }) + ]); + final rootElement = await pumpTestElement(tester); + for (final element in rootElement.findAllChildren()) { + expect(sut.shouldMask(element, element.widget), isFalse); + } + // Note: there are actually 5 Image widgets in the tree but when it's + // inside an `Visibility(visible: false)`, it won't be visited. + expect(called, {Image: 4, CustomImageWidget: 1}); + }); + + testWidgets('stops iteration on first truthy rule', (tester) async { + var called = 0; + final sut = SentryMaskingConfig([ + SentryMaskingCustomRule((e, w) => ++called == -1), + SentryMaskingCustomRule((e, w) => ++called == 2), + SentryMaskingCustomRule((e, w) => fail('should not be called')) + ]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), isTrue); + }); + + testWidgets('retuns false if no rule matches', (tester) async { + final sut = SentryMaskingConfig([ + SentryMaskingCustomRule((e, w) => false), + SentryMaskingCustomRule((e, w) => false), + ]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), isFalse); + }); + }); +} + +extension on Element { + Element findFirstOfType() { + late Element result; + late void Function(Element) visitor; + visitor = (Element element) { + if (element.widget is T) { + result = element; + } else { + element.visitChildElements(visitor); + } + }; + visitChildren((visitor)); + assert(result.widget is T); + return result; + } + + List findAllChildren() { + final result = []; + late void Function(Element) visitor; + visitor = (Element element) { + result.add(element); + element.visitChildElements(visitor); + }; + visitChildren((visitor)); + return result; + } +} diff --git a/flutter/test/replay/test_widget.dart b/flutter/test/replay/test_widget.dart index d800a3ef12..e3dbf62607 100644 --- a/flutter/test/replay/test_widget.dart +++ b/flutter/test/replay/test_widget.dart @@ -25,7 +25,7 @@ Future pumpTestElement(WidgetTester tester, onPressed: () {}, child: Text('Button title'), ), - newImage(), + newCustomImage(), // Invisible widgets won't be obscured. Visibility(visible: false, child: Text('Invisible text')), Visibility(visible: false, child: newImage()), @@ -77,4 +77,16 @@ Image newImage({double width = 1, double height = 1}) => Image.memory( height: height, ); +CustomImageWidget newCustomImage({double width = 1, double height = 1}) => + CustomImageWidget.memory( + testImageData, + width: width, + height: height, + ); + const dummyText = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.'; + +class CustomImageWidget extends Image { + CustomImageWidget.memory(super.bytes, {super.key, super.width, super.height}) + : super.memory(); +} From 37586fa43ede9cbc8ac19f8c4bd27cb4df292f2c Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 30 Sep 2024 19:11:55 +0200 Subject: [PATCH 06/21] update widget filter to use the masking config --- flutter/lib/src/replay/masking_config.dart | 7 +- flutter/lib/src/replay/recorder.dart | 6 +- flutter/lib/src/replay/widget_filter.dart | 49 ++------- flutter/lib/src/sentry_replay_options.dart | 104 ++++++++----------- flutter/test/replay/masking_config_test.dart | 43 ++++---- flutter/test/replay/widget_filter_test.dart | 2 +- 6 files changed, 84 insertions(+), 127 deletions(-) diff --git a/flutter/lib/src/replay/masking_config.dart b/flutter/lib/src/replay/masking_config.dart index 9d5727738a..2e31ee97f5 100644 --- a/flutter/lib/src/replay/masking_config.dart +++ b/flutter/lib/src/replay/masking_config.dart @@ -40,9 +40,10 @@ class SentryMaskingCustomRule extends SentryMaskingRule { } @internal -class SentryMaskingTruthyRule extends SentryMaskingRule { - const SentryMaskingTruthyRule(); +class SentryMaskingConstantRule extends SentryMaskingRule { + final bool _value; + const SentryMaskingConstantRule(this._value); @override - bool shouldMask(Element element, T widget) => true; + bool shouldMask(Element element, T widget) => _value; } diff --git a/flutter/lib/src/replay/recorder.dart b/flutter/lib/src/replay/recorder.dart index 916fa778e4..f15b79a072 100644 --- a/flutter/lib/src/replay/recorder.dart +++ b/flutter/lib/src/replay/recorder.dart @@ -21,9 +21,9 @@ class ScreenshotRecorder { bool warningLogged = false; ScreenshotRecorder(this.config, this.options) { - final replayOptions = options.experimental.replay; - if (replayOptions.maskingConfig.isNotEmpty) { - _widgetFilter = WidgetFilter(replayOptions.maskingConfig, options.logger); + final maskingConfig = options.experimental.replay.buildMaskingConfig(); + if (maskingConfig.length > 0) { + _widgetFilter = WidgetFilter(maskingConfig, options.logger); } } diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index 1d0032db82..a99668050f 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -4,12 +4,13 @@ import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; import '../sentry_asset_bundle.dart'; +import 'masking_config.dart'; @internal class WidgetFilter { final items = []; final SentryLogger logger; - final Map config; + final SentryMaskingConfig config; static const _defaultColor = Color.fromARGB(255, 0, 0, 0); late double _pixelRatio; late Rect _bounds; @@ -39,33 +40,17 @@ class WidgetFilter { return; } - if (!_shouldObscure(element, widget)) { - // If this element should not be obscured, visit and check its children. - element.visitChildElements(_process); - } else { + if (config.shouldMask(element, widget)) { final item = _obscureElementOrParent(element, widget); if (item != null) { items.add(item); } + } else { + // If this element should not be obscured, visit and check its children. + element.visitChildElements(_process); } } - @pragma('vm:prefer-inline') - bool _shouldObscure(Element element, Widget widget) { - // Check if we should mask this widget based on the configuration. - final maskingConfig = config[widget.runtimeType]; - if (maskingConfig == null) { - return false; - } else if (!maskingConfig.shouldMask(element, widget)) { - assert(() { - logger(SentryLevel.debug, "WidgetFilter skipping: $widget"); - return true; - }()); - return false; - } - return true; - } - /// Determine the color and bounding box of the widget. /// If the widget is offscreen, returns null. /// If the widget cannot be obscured, obscures the parent. @@ -192,28 +177,6 @@ class WidgetFilterItem { const WidgetFilterItem(this.color, this.bounds); } -@internal -class WidgetFilterMaskingConfig { - static const mask = WidgetFilterMaskingConfig._(1, 'mask'); - - final int index; - final String _name; - final bool Function(Element, Widget)? _shouldMask; - - const WidgetFilterMaskingConfig._(this.index, this._name) - : _shouldMask = null; - const WidgetFilterMaskingConfig.custom(this._shouldMask) - : index = 2, - _name = 'custom'; - - @override - String toString() => "$WidgetFilterMaskingConfig.$_name"; - - @pragma('vm:prefer-inline') - bool shouldMask(Element element, Widget widget) => - this == mask ? true : _shouldMask!(element, widget); -} - extension on Element { Element? get parent { Element? result; diff --git a/flutter/lib/src/sentry_replay_options.dart b/flutter/lib/src/sentry_replay_options.dart index f1801d66d5..6c45607726 100644 --- a/flutter/lib/src/sentry_replay_options.dart +++ b/flutter/lib/src/sentry_replay_options.dart @@ -2,6 +2,7 @@ import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; +import 'replay/masking_config.dart'; import 'replay/widget_filter.dart'; /// Configuration of the experimental replay feature. @@ -33,85 +34,72 @@ class SentryReplayOptions { _onErrorSampleRate = value; } - /// Redact all text content. Draws a rectangle of text bounds with text color + /// Mask all text content. Draws a rectangle of text bounds with text color /// on top. Currently, only [Text] and [EditableText] Widgets are redacted. /// Default is enabled. - set redactAllText(bool value) { - if (value) { - mask(Text); - mask(EditableText); - } else { - unmask(Text); - unmask(EditableText); - } - } + bool maskAllText = true; - /// Redact all image content. Draws a rectangle of image bounds with image's + @Deprecated('Use maskAllText instead') + bool get redactAllText => maskAllText; + set redactAllText(bool value) => maskAllText = value; + + /// Mask content of all images. Draws a rectangle of image bounds with image's /// dominant color on top. Currently, only [Image] widgets are redacted. /// Default is enabled (except for asset images, see [maskAssetImages]). - bool _maskImages = true; - set redactAllImages(bool value) { - _maskImages = value; - if (value) { - if (_maskAssetImages) { - mask(Image); - } else { - maskIfTrue(Image, _maskImagesExceptAssets); - } - } else { - unmask(Image); - } - } + bool maskAllImages = true; - /// Redact asset iamges. - bool _maskAssetImages = false; - set maskAssetImages(bool value) { - if (_maskAssetImages == value) { - return; - } - _maskAssetImages = value; - if (value) { - assert(_maskImages); - mask(Image); - } else if (_maskImages) { - maskIfTrue(Image, _maskImagesExceptAssets); - } else { - unmask(Image); - } - } + @Deprecated('Use maskAllImages instead') + bool get redactAllImages => maskAllImages; + set redactAllImages(bool value) => maskAllImages = value; - Map _maskingConfig = {}; - bool _finished = false; + /// Redact asset images coming from the root asset bundle. + bool maskAssetImages = false; + + final _userMaskingRules = []; - /// Once accessed, masking confing cannot change anymore. @internal - Map get maskingConfig { - if (_finished) { - return _maskingConfig; + SentryMaskingConfig buildMaskingConfig() { + final rules = _userMaskingRules.toList(); + if (maskAllImages) { + if (maskAssetImages) { + rules.add(const SentryMaskingConstantRule(true)); + } else { + rules + .add(const SentryMaskingCustomRule(_maskImagesExceptAssets)); + } + } else { + assert(!maskAssetImages, + "maskAssetImages can't be true if maskAllImages is false"); } - _finished = true; - final result = - Map.unmodifiable(_maskingConfig); - _maskingConfig = result; - return result; + if (maskAllText) { + rules.add(const SentryMaskingConstantRule(true)); + rules.add(const SentryMaskingConstantRule(true)); + } + return SentryMaskingConfig(rules); } /// Mask given widget type in the replay. - void mask(Type type) { - _maskingConfig[type] = WidgetFilterMaskingConfig.mask; + void mask() { + _removeMaskingConstantRule(); + _userMaskingRules.add(SentryMaskingConstantRule(true)); } /// Unmask given widget type in the replay. - void unmask(Type type) { - _maskingConfig.remove(type); + void unmask() { + _removeMaskingConstantRule(); + _userMaskingRules.add(SentryMaskingConstantRule(false)); } /// Unmask given widget type in the replay if it's masked by default rules - /// [redactAllText] or [redactAllImages]. - void maskIfTrue(Type type, bool Function(Element, Widget) shouldMask) { - _maskingConfig[type] = WidgetFilterMaskingConfig.custom(shouldMask); + /// [maskAllText] or [maskAllImages]. + void maskIfTrue(bool Function(Element, T) shouldMask) { + _removeMaskingConstantRule(); + _userMaskingRules.add(SentryMaskingCustomRule(shouldMask)); } + void _removeMaskingConstantRule() => _userMaskingRules + .removeWhere((rule) => rule is SentryMaskingConstantRule); + @internal bool get isEnabled => ((sessionSampleRate ?? 0) > 0) || ((onErrorSampleRate ?? 0) > 0); diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart index 2410e26bdb..cdd781e176 100644 --- a/flutter/test/replay/masking_config_test.dart +++ b/flutter/test/replay/masking_config_test.dart @@ -14,28 +14,33 @@ void main() async { expect(sut.shouldMask(element, element.widget), isFalse); }); - group('$SentryMaskingTruthyRule', () { - testWidgets('will mask widget by type', (tester) async { - final sut = SentryMaskingConfig([SentryMaskingTruthyRule()]); - final rootElement = await pumpTestElement(tester); - final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), isTrue); - }); + for (final value in [true, false]) { + group('$SentryMaskingConstantRule($value)', () { + testWidgets('will mask widget by type', (tester) async { + final sut = + SentryMaskingConfig([SentryMaskingConstantRule(value)]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), value); + }); - testWidgets('will mask subtype widget by type', (tester) async { - final sut = SentryMaskingConfig([SentryMaskingTruthyRule()]); - final rootElement = await pumpTestElement(tester); - final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), isTrue); - }); + testWidgets('will mask subtype widget by type', (tester) async { + final sut = + SentryMaskingConfig([SentryMaskingConstantRule(value)]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), value); + }); - testWidgets('will not mask widget of a different type', (tester) async { - final sut = SentryMaskingConfig([SentryMaskingTruthyRule()]); - final rootElement = await pumpTestElement(tester); - final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), isFalse); + testWidgets('will not mask widget of a different type', (tester) async { + final sut = + SentryMaskingConfig([SentryMaskingConstantRule(value)]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), !value); + }, skip: !value); }); - }); + } group('$SentryMaskingCustomRule', () { testWidgets('only called for specified type', (tester) async { diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index 9c440b3a41..5ae70f0336 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -17,7 +17,7 @@ void main() async { final replayOptions = SentryReplayOptions(); replayOptions.redactAllImages = redactImages; replayOptions.redactAllText = redactText; - return WidgetFilter(replayOptions.maskingConfig, + return WidgetFilter(replayOptions.buildMaskingConfig(), (level, message, {exception, logger, stackTrace}) {}); }; From 8135d2814eafcdc2a1aaf7d5929c0f742afb8610 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 30 Sep 2024 19:16:13 +0200 Subject: [PATCH 07/21] cleanup --- flutter/lib/src/replay/masking_config.dart | 2 ++ flutter/test/replay/masking_config_test.dart | 1 + 2 files changed, 3 insertions(+) diff --git a/flutter/lib/src/replay/masking_config.dart b/flutter/lib/src/replay/masking_config.dart index 2e31ee97f5..caa8356da8 100644 --- a/flutter/lib/src/replay/masking_config.dart +++ b/flutter/lib/src/replay/masking_config.dart @@ -3,7 +3,9 @@ import 'package:meta/meta.dart'; @internal class SentryMaskingConfig { + @visibleForTesting final List rules; + final int length; SentryMaskingConfig(List rules) diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart index cdd781e176..4efd0679b1 100644 --- a/flutter/test/replay/masking_config_test.dart +++ b/flutter/test/replay/masking_config_test.dart @@ -11,6 +11,7 @@ void main() async { final sut = SentryMaskingConfig([]); final element = await pumpTestElement(tester); expect(sut.rules, isEmpty); + expect(sut.length, 0); expect(sut.shouldMask(element, element.widget), isFalse); }); From 90273fced09f1920a1ce569f3bdafb4562401ebf Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 1 Oct 2024 11:26:16 +0200 Subject: [PATCH 08/21] masking tests --- flutter/lib/src/replay/masking_config.dart | 35 ++++- flutter/lib/src/sentry_replay_options.dart | 37 +++-- flutter/test/replay/masking_config_test.dart | 155 +++++++++++++++++-- 3 files changed, 189 insertions(+), 38 deletions(-) diff --git a/flutter/lib/src/replay/masking_config.dart b/flutter/lib/src/replay/masking_config.dart index caa8356da8..5d04e0f46c 100644 --- a/flutter/lib/src/replay/masking_config.dart +++ b/flutter/lib/src/replay/masking_config.dart @@ -15,37 +15,58 @@ class SentryMaskingConfig { bool shouldMask(Element element, T widget) { for (int i = 0; i < length; i++) { - if (rules[i].appliesTo(widget) && rules[i].shouldMask(element, widget)) { - return true; + if (rules[i].appliesTo(widget)) { + switch (rules[i].shouldMask(element, widget)) { + case MaskingDecision.mask: + return true; + case MaskingDecision.unmask: + return false; + case MaskingDecision.continueProcessing: + // Continue to the next matching rule. + } } } return false; } } +enum MaskingDecision { mask, unmask, continueProcessing } + @internal abstract class SentryMaskingRule { bool appliesTo(Widget widget) => widget is T; - bool shouldMask(Element element, T widget); + MaskingDecision shouldMask(Element element, T widget); const SentryMaskingRule(); } @internal class SentryMaskingCustomRule extends SentryMaskingRule { - final bool Function(Element element, T widget) callback; + final MaskingDecision Function(Element element, T widget) callback; const SentryMaskingCustomRule(this.callback); @override - bool shouldMask(Element element, T widget) => callback(element, widget); + MaskingDecision shouldMask(Element element, T widget) => + callback(element, widget); + + @override + String toString() => '$SentryMaskingCustomRule<$T>($callback)'; } @internal class SentryMaskingConstantRule extends SentryMaskingRule { - final bool _value; + final MaskingDecision _value; const SentryMaskingConstantRule(this._value); @override - bool shouldMask(Element element, T widget) => _value; + MaskingDecision shouldMask(Element element, T widget) { + // This rule only makes sense with true/false. Continue won't do anything. + assert(_value == MaskingDecision.mask || _value == MaskingDecision.unmask); + return _value; + } + + @override + String toString() => + '$SentryMaskingConstantRule<$T>(${_value == MaskingDecision.mask ? 'mask' : 'unmask'})'; } diff --git a/flutter/lib/src/sentry_replay_options.dart b/flutter/lib/src/sentry_replay_options.dart index 6c45607726..2588183b99 100644 --- a/flutter/lib/src/sentry_replay_options.dart +++ b/flutter/lib/src/sentry_replay_options.dart @@ -62,7 +62,7 @@ class SentryReplayOptions { final rules = _userMaskingRules.toList(); if (maskAllImages) { if (maskAssetImages) { - rules.add(const SentryMaskingConstantRule(true)); + rules.add(const SentryMaskingConstantRule(MaskingDecision.mask)); } else { rules .add(const SentryMaskingCustomRule(_maskImagesExceptAssets)); @@ -72,47 +72,46 @@ class SentryReplayOptions { "maskAssetImages can't be true if maskAllImages is false"); } if (maskAllText) { - rules.add(const SentryMaskingConstantRule(true)); - rules.add(const SentryMaskingConstantRule(true)); + rules.add(const SentryMaskingConstantRule(MaskingDecision.mask)); + rules.add( + const SentryMaskingConstantRule(MaskingDecision.mask)); } return SentryMaskingConfig(rules); } - /// Mask given widget type in the replay. + /// Mask given widget type (or it's subclasses) in the replay. + /// Note: masking rules are called in the order they're added. void mask() { - _removeMaskingConstantRule(); - _userMaskingRules.add(SentryMaskingConstantRule(true)); + _userMaskingRules.add(SentryMaskingConstantRule(MaskingDecision.mask)); } - /// Unmask given widget type in the replay. + /// Unmask given widget type (or it's subclasses) in the replay, if it would + /// have otherwise been masked by other rules, for example the default rules + /// [maskAllText] or [maskAllImages]. void unmask() { - _removeMaskingConstantRule(); - _userMaskingRules.add(SentryMaskingConstantRule(false)); + _userMaskingRules.add(SentryMaskingConstantRule(MaskingDecision.unmask)); } - /// Unmask given widget type in the replay if it's masked by default rules - /// [maskAllText] or [maskAllImages]. - void maskIfTrue(bool Function(Element, T) shouldMask) { - _removeMaskingConstantRule(); + /// Provide a custom callback to decide whether to mask the widget. + /// Note: masking rules are called in the order they're added. + void maskCallback( + MaskingDecision Function(Element, T) shouldMask) { _userMaskingRules.add(SentryMaskingCustomRule(shouldMask)); } - void _removeMaskingConstantRule() => _userMaskingRules - .removeWhere((rule) => rule is SentryMaskingConstantRule); - @internal bool get isEnabled => ((sessionSampleRate ?? 0) > 0) || ((onErrorSampleRate ?? 0) > 0); } -bool _maskImagesExceptAssets(Element element, Widget widget) { +MaskingDecision _maskImagesExceptAssets(Element element, Widget widget) { if (widget is Image) { final image = widget.image; if (image is AssetBundleImageProvider) { if (WidgetFilter.isBuiltInAssetImage(image, rootBundle)) { - return false; + return MaskingDecision.continueProcessing; } } } - return true; + return MaskingDecision.mask; } diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart index 4efd0679b1..1f96f51b38 100644 --- a/flutter/test/replay/masking_config_test.dart +++ b/flutter/test/replay/masking_config_test.dart @@ -1,5 +1,6 @@ import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/replay/masking_config.dart'; import 'test_widget.dart'; @@ -15,14 +16,15 @@ void main() async { expect(sut.shouldMask(element, element.widget), isFalse); }); - for (final value in [true, false]) { + for (final value in [MaskingDecision.mask, MaskingDecision.unmask]) { group('$SentryMaskingConstantRule($value)', () { testWidgets('will mask widget by type', (tester) async { final sut = SentryMaskingConfig([SentryMaskingConstantRule(value)]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), value); + expect(sut.shouldMask(element, element.widget), + value == MaskingDecision.mask); }); testWidgets('will mask subtype widget by type', (tester) async { @@ -30,7 +32,8 @@ void main() async { SentryMaskingConfig([SentryMaskingConstantRule(value)]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), value); + expect(sut.shouldMask(element, element.widget), + value == MaskingDecision.mask); }); testWidgets('will not mask widget of a different type', (tester) async { @@ -38,8 +41,8 @@ void main() async { SentryMaskingConfig([SentryMaskingConstantRule(value)]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), !value); - }, skip: !value); + expect(sut.shouldMask(element, element.widget), false); + }, skip: value == MaskingDecision.unmask); }); } @@ -49,7 +52,7 @@ void main() async { final sut = SentryMaskingConfig([ SentryMaskingCustomRule((e, w) { called[w.runtimeType] = (called[w.runtimeType] ?? 0) + 1; - return false; + return MaskingDecision.continueProcessing; }) ]); final rootElement = await pumpTestElement(tester); @@ -61,11 +64,11 @@ void main() async { expect(called, {Image: 4, CustomImageWidget: 1}); }); - testWidgets('stops iteration on first truthy rule', (tester) async { - var called = 0; + testWidgets('stops iteration on the first "mask" rule', (tester) async { final sut = SentryMaskingConfig([ - SentryMaskingCustomRule((e, w) => ++called == -1), - SentryMaskingCustomRule((e, w) => ++called == 2), + SentryMaskingCustomRule( + (e, w) => MaskingDecision.continueProcessing), + SentryMaskingCustomRule((e, w) => MaskingDecision.mask), SentryMaskingCustomRule((e, w) => fail('should not be called')) ]); final rootElement = await pumpTestElement(tester); @@ -73,16 +76,144 @@ void main() async { expect(sut.shouldMask(element, element.widget), isTrue); }); + testWidgets('stops iteration on first "unmask" rule', (tester) async { + final sut = SentryMaskingConfig([ + SentryMaskingCustomRule( + (e, w) => MaskingDecision.continueProcessing), + SentryMaskingCustomRule((e, w) => MaskingDecision.unmask), + SentryMaskingCustomRule((e, w) => fail('should not be called')) + ]); + final rootElement = await pumpTestElement(tester); + final element = rootElement.findFirstOfType(); + expect(sut.shouldMask(element, element.widget), isFalse); + }); + testWidgets('retuns false if no rule matches', (tester) async { final sut = SentryMaskingConfig([ - SentryMaskingCustomRule((e, w) => false), - SentryMaskingCustomRule((e, w) => false), + SentryMaskingCustomRule( + (e, w) => MaskingDecision.continueProcessing), + SentryMaskingCustomRule( + (e, w) => MaskingDecision.continueProcessing), ]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); expect(sut.shouldMask(element, element.widget), isFalse); }); }); + + group('$SentryReplayOptions.buildMaskingConfig()', () { + List stringRules(SentryReplayOptions options) { + final config = options.buildMaskingConfig(); + return config.rules + .map((rule) => rule.toString()) + .map((str) => str.replaceAll(RegExp(r'@[0-9]+'), '@')) + .toList(); + } + + test('defaults', () { + final sut = SentryReplayOptions(); + expect(stringRules(sut), [ + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function \'_maskImagesExceptAssets@\': static.)', + '$SentryMaskingConstantRule<$Text>(mask)', + '$SentryMaskingConstantRule<$EditableText>(mask)' + ]); + }); + + test('maskAllImages=true & maskAssetImages=true', () { + final sut = SentryReplayOptions() + ..maskAllText = false + ..maskAllImages = true + ..maskAssetImages = true; + expect(stringRules(sut), [ + '$SentryMaskingConstantRule<$Image>(mask)', + ]); + }); + + test('maskAllImages=true & maskAssetImages=false', () { + final sut = SentryReplayOptions() + ..maskAllText = false + ..maskAllImages = true + ..maskAssetImages = false; + expect(stringRules(sut), [ + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function \'_maskImagesExceptAssets@\': static.)', + ]); + }); + + test('maskAllText=true', () { + final sut = SentryReplayOptions() + ..maskAllText = true + ..maskAllImages = false + ..maskAssetImages = false; + expect(stringRules(sut), [ + '$SentryMaskingConstantRule<$Text>(mask)', + '$SentryMaskingConstantRule<$EditableText>(mask)' + ]); + }); + + test('maskAllText=false', () { + final sut = SentryReplayOptions() + ..maskAllText = false + ..maskAllImages = false + ..maskAssetImages = false; + expect(stringRules(sut), isEmpty); + }); + + group('user rules', () { + final defaultRules = [ + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function \'_maskImagesExceptAssets@\': static.)', + '$SentryMaskingConstantRule<$Text>(mask)', + '$SentryMaskingConstantRule<$EditableText>(mask)' + ]; + test('mask() takes precedence', () { + final sut = SentryReplayOptions(); + sut.mask(); + expect(stringRules(sut), + ['$SentryMaskingConstantRule<$Image>(mask)', ...defaultRules]); + }); + test('unmask() takes precedence', () { + final sut = SentryReplayOptions(); + sut.unmask(); + expect(stringRules(sut), + ['$SentryMaskingConstantRule<$Image>(unmask)', ...defaultRules]); + }); + test('are ordered in the call order', () { + var sut = SentryReplayOptions(); + sut.mask(); + sut.unmask(); + expect(stringRules(sut), [ + '$SentryMaskingConstantRule<$Image>(mask)', + '$SentryMaskingConstantRule<$Image>(unmask)', + ...defaultRules + ]); + sut = SentryReplayOptions(); + sut.unmask(); + sut.mask(); + expect(stringRules(sut), [ + '$SentryMaskingConstantRule<$Image>(unmask)', + '$SentryMaskingConstantRule<$Image>(mask)', + ...defaultRules + ]); + sut = SentryReplayOptions(); + sut.unmask(); + sut.maskCallback((_, Image widget) => MaskingDecision.mask); + sut.mask(); + expect(stringRules(sut), [ + '$SentryMaskingConstantRule<$Image>(unmask)', + '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $MaskingDecision)', + '$SentryMaskingConstantRule<$Image>(mask)', + ...defaultRules + ]); + }); + test('maskCallback() takes precedence', () { + final sut = SentryReplayOptions(); + sut.maskCallback((_, Image widget) => MaskingDecision.mask); + expect(stringRules(sut), [ + '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $MaskingDecision)', + ...defaultRules + ]); + }); + }); + }); } extension on Element { From 1179d12f893e2fc0247d73bc1ea6c81ad2324358 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 7 Oct 2024 15:18:04 +0200 Subject: [PATCH 09/21] cleanup --- flutter/lib/src/sentry_replay_options.dart | 11 +++-------- flutter/test/replay/widget_filter_test.dart | 3 +++ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/flutter/lib/src/sentry_replay_options.dart b/flutter/lib/src/sentry_replay_options.dart index 2588183b99..8d54fef866 100644 --- a/flutter/lib/src/sentry_replay_options.dart +++ b/flutter/lib/src/sentry_replay_options.dart @@ -7,11 +7,6 @@ import 'replay/widget_filter.dart'; /// Configuration of the experimental replay feature. class SentryReplayOptions { - SentryReplayOptions() { - redactAllText = true; - redactAllImages = true; - } - double? _sessionSampleRate; /// A percentage of sessions in which a replay will be created. @@ -37,7 +32,7 @@ class SentryReplayOptions { /// Mask all text content. Draws a rectangle of text bounds with text color /// on top. Currently, only [Text] and [EditableText] Widgets are redacted. /// Default is enabled. - bool maskAllText = true; + var maskAllText = true; @Deprecated('Use maskAllText instead') bool get redactAllText => maskAllText; @@ -46,14 +41,14 @@ class SentryReplayOptions { /// Mask content of all images. Draws a rectangle of image bounds with image's /// dominant color on top. Currently, only [Image] widgets are redacted. /// Default is enabled (except for asset images, see [maskAssetImages]). - bool maskAllImages = true; + var maskAllImages = true; @Deprecated('Use maskAllImages instead') bool get redactAllImages => maskAllImages; set redactAllImages(bool value) => maskAllImages = value; /// Redact asset images coming from the root asset bundle. - bool maskAssetImages = false; + var maskAssetImages = false; final _userMaskingRules = []; diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index 5ae70f0336..af29b49759 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -7,6 +7,9 @@ import 'package:sentry_flutter/src/replay/widget_filter.dart'; import 'test_widget.dart'; +// Note: these tests predate existance of `SentryMaskingConfig` which now +// takes care of the decision making whether something is masked or not. +// We'll keep these tests although they're not unit-tests anymore. void main() async { TestWidgetsFlutterBinding.ensureInitialized(); const defaultBounds = Rect.fromLTRB(0, 0, 1000, 1000); From a72c3b1ee747396b9a2533a8e8e3fb5ff3ff8599 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 7 Oct 2024 15:25:58 +0200 Subject: [PATCH 10/21] test masking editable text --- flutter/test/replay/test_widget.dart | 1 + flutter/test/replay/widget_filter_test.dart | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/flutter/test/replay/test_widget.dart b/flutter/test/replay/test_widget.dart index e3dbf62607..2e8d257a2a 100644 --- a/flutter/test/replay/test_widget.dart +++ b/flutter/test/replay/test_widget.dart @@ -34,6 +34,7 @@ Future pumpTestElement(WidgetTester tester, Offstage(offstage: true, child: Text('Offstage text')), Offstage(offstage: true, child: newImage()), Text(dummyText), + Material(child: TextFormField()), SizedBox( width: 100, height: 20, diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index af29b49759..44fd4b22af 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -32,7 +32,7 @@ void main() async { final sut = createSut(redactText: true); final element = await pumpTestElement(tester); sut.obscure(element, 1.0, defaultBounds); - expect(sut.items.length, 4); + expect(sut.items.length, 5); }); testWidgets('does not redact text when disabled', (tester) async { @@ -54,11 +54,12 @@ void main() async { final sut = createSut(redactText: true); final element = await pumpTestElement(tester); sut.obscure(element, 1.0, defaultBounds); - expect(sut.items.length, 4); + expect(sut.items.length, 5); expect(boundsRect(sut.items[0]), '624x48'); expect(boundsRect(sut.items[1]), '169x20'); expect(boundsRect(sut.items[2]), '800x192'); - expect(boundsRect(sut.items[3]), '50x20'); + expect(boundsRect(sut.items[3]), '800x24'); + expect(boundsRect(sut.items[4]), '50x20'); }); }); From 8f3c84989c0df047178cf5cb1c2a3f67d80c58f0 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 7 Oct 2024 17:04:21 +0200 Subject: [PATCH 11/21] fix tests on web --- flutter/test/replay/masking_config_test.dart | 36 +++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart index 1f96f51b38..67f2d3480f 100644 --- a/flutter/test/replay/masking_config_test.dart +++ b/flutter/test/replay/masking_config_test.dart @@ -102,18 +102,22 @@ void main() async { }); group('$SentryReplayOptions.buildMaskingConfig()', () { - List stringRules(SentryReplayOptions options) { + List rulesAsStrings(SentryReplayOptions options) { final config = options.buildMaskingConfig(); return config.rules .map((rule) => rule.toString()) - .map((str) => str.replaceAll(RegExp(r'@[0-9]+'), '@')) + // This normalizes the string on VM & web: + .map((str) => str.replaceAll( + RegExp( + r"MaskingDecision from:? [fF]unction '?_maskImagesExceptAssets[@(].*"), + 'MaskingDecision from Function _maskImagesExceptAssets')) .toList(); } test('defaults', () { final sut = SentryReplayOptions(); - expect(stringRules(sut), [ - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function \'_maskImagesExceptAssets@\': static.)', + expect(rulesAsStrings(sut), [ + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function _maskImagesExceptAssets', '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' ]); @@ -124,7 +128,7 @@ void main() async { ..maskAllText = false ..maskAllImages = true ..maskAssetImages = true; - expect(stringRules(sut), [ + expect(rulesAsStrings(sut), [ '$SentryMaskingConstantRule<$Image>(mask)', ]); }); @@ -134,8 +138,8 @@ void main() async { ..maskAllText = false ..maskAllImages = true ..maskAssetImages = false; - expect(stringRules(sut), [ - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function \'_maskImagesExceptAssets@\': static.)', + expect(rulesAsStrings(sut), [ + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function _maskImagesExceptAssets', ]); }); @@ -144,7 +148,7 @@ void main() async { ..maskAllText = true ..maskAllImages = false ..maskAssetImages = false; - expect(stringRules(sut), [ + expect(rulesAsStrings(sut), [ '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' ]); @@ -155,32 +159,32 @@ void main() async { ..maskAllText = false ..maskAllImages = false ..maskAssetImages = false; - expect(stringRules(sut), isEmpty); + expect(rulesAsStrings(sut), isEmpty); }); group('user rules', () { final defaultRules = [ - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function \'_maskImagesExceptAssets@\': static.)', + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function _maskImagesExceptAssets', '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' ]; test('mask() takes precedence', () { final sut = SentryReplayOptions(); sut.mask(); - expect(stringRules(sut), + expect(rulesAsStrings(sut), ['$SentryMaskingConstantRule<$Image>(mask)', ...defaultRules]); }); test('unmask() takes precedence', () { final sut = SentryReplayOptions(); sut.unmask(); - expect(stringRules(sut), + expect(rulesAsStrings(sut), ['$SentryMaskingConstantRule<$Image>(unmask)', ...defaultRules]); }); test('are ordered in the call order', () { var sut = SentryReplayOptions(); sut.mask(); sut.unmask(); - expect(stringRules(sut), [ + expect(rulesAsStrings(sut), [ '$SentryMaskingConstantRule<$Image>(mask)', '$SentryMaskingConstantRule<$Image>(unmask)', ...defaultRules @@ -188,7 +192,7 @@ void main() async { sut = SentryReplayOptions(); sut.unmask(); sut.mask(); - expect(stringRules(sut), [ + expect(rulesAsStrings(sut), [ '$SentryMaskingConstantRule<$Image>(unmask)', '$SentryMaskingConstantRule<$Image>(mask)', ...defaultRules @@ -197,7 +201,7 @@ void main() async { sut.unmask(); sut.maskCallback((_, Image widget) => MaskingDecision.mask); sut.mask(); - expect(stringRules(sut), [ + expect(rulesAsStrings(sut), [ '$SentryMaskingConstantRule<$Image>(unmask)', '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $MaskingDecision)', '$SentryMaskingConstantRule<$Image>(mask)', @@ -207,7 +211,7 @@ void main() async { test('maskCallback() takes precedence', () { final sut = SentryReplayOptions(); sut.maskCallback((_, Image widget) => MaskingDecision.mask); - expect(stringRules(sut), [ + expect(rulesAsStrings(sut), [ '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $MaskingDecision)', ...defaultRules ]); From cc5d71f1e5fae98277c00867074247f4b984ae0d Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 7 Oct 2024 17:46:27 +0200 Subject: [PATCH 12/21] fix tests on web --- flutter/test/replay/masking_config_test.dart | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart index 67f2d3480f..3973774298 100644 --- a/flutter/test/replay/masking_config_test.dart +++ b/flutter/test/replay/masking_config_test.dart @@ -106,11 +106,15 @@ void main() async { final config = options.buildMaskingConfig(); return config.rules .map((rule) => rule.toString()) - // This normalizes the string on VM & web: + // These normalize the string on VM & web: .map((str) => str.replaceAll( RegExp( - r"MaskingDecision from:? [fF]unction '?_maskImagesExceptAssets[@(].*"), + r"MaskingDecision from:? [fF]unction '?_maskImagesExceptAssets[@(].*", + dotAll: true), 'MaskingDecision from Function _maskImagesExceptAssets')) + .map((str) => str.replaceAll( + ' from: (element, widget) => masking_config.MaskingDecision.mask', + '')) .toList(); } @@ -199,7 +203,8 @@ void main() async { ]); sut = SentryReplayOptions(); sut.unmask(); - sut.maskCallback((_, Image widget) => MaskingDecision.mask); + sut.maskCallback( + (Element element, Image widget) => MaskingDecision.mask); sut.mask(); expect(rulesAsStrings(sut), [ '$SentryMaskingConstantRule<$Image>(unmask)', @@ -210,7 +215,8 @@ void main() async { }); test('maskCallback() takes precedence', () { final sut = SentryReplayOptions(); - sut.maskCallback((_, Image widget) => MaskingDecision.mask); + sut.maskCallback( + (Element element, Image widget) => MaskingDecision.mask); expect(rulesAsStrings(sut), [ '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $MaskingDecision)', ...defaultRules From e8777957edfade6463146cf9ef28627d0db34be4 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 7 Oct 2024 17:58:39 +0200 Subject: [PATCH 13/21] fix tests for wasm --- flutter/test/replay/masking_config_test.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart index 3973774298..98c2797e3c 100644 --- a/flutter/test/replay/masking_config_test.dart +++ b/flutter/test/replay/masking_config_test.dart @@ -106,12 +106,12 @@ void main() async { final config = options.buildMaskingConfig(); return config.rules .map((rule) => rule.toString()) - // These normalize the string on VM & web: + // These normalize the string on VM & js & wasm: .map((str) => str.replaceAll( RegExp( r"MaskingDecision from:? [fF]unction '?_maskImagesExceptAssets[@(].*", dotAll: true), - 'MaskingDecision from Function _maskImagesExceptAssets')) + 'MaskingDecision)')) .map((str) => str.replaceAll( ' from: (element, widget) => masking_config.MaskingDecision.mask', '')) @@ -121,7 +121,7 @@ void main() async { test('defaults', () { final sut = SentryReplayOptions(); expect(rulesAsStrings(sut), [ - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function _maskImagesExceptAssets', + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' ]); @@ -143,7 +143,7 @@ void main() async { ..maskAllImages = true ..maskAssetImages = false; expect(rulesAsStrings(sut), [ - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function _maskImagesExceptAssets', + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', ]); }); @@ -168,7 +168,7 @@ void main() async { group('user rules', () { final defaultRules = [ - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision from Function _maskImagesExceptAssets', + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' ]; From 85a7fee1af2c69d5f84633540cc302d3c484971c Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Oct 2024 14:12:14 +0200 Subject: [PATCH 14/21] add SentryMask and SentryUnmask widgets --- flutter/lib/sentry_flutter.dart | 2 + flutter/lib/src/replay/masking_config.dart | 21 ++++++-- flutter/lib/src/replay/widget_filter.dart | 27 ++++++---- .../src/screenshot/sentry_mask_widget.dart | 11 ++++ .../src/screenshot/sentry_unmask_widget.dart | 11 ++++ flutter/lib/src/sentry_replay_options.dart | 38 +++++++++++--- flutter/test/replay/masking_config_test.dart | 50 ++++++++++++++----- flutter/test/replay/widget_filter_test.dart | 21 ++++++++ 8 files changed, 147 insertions(+), 34 deletions(-) create mode 100644 flutter/lib/src/screenshot/sentry_mask_widget.dart create mode 100644 flutter/lib/src/screenshot/sentry_unmask_widget.dart diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index c74013e81e..ba4d939d86 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -12,6 +12,8 @@ export 'src/sentry_replay_options.dart'; export 'src/flutter_sentry_attachment.dart'; export 'src/sentry_asset_bundle.dart' show SentryAssetBundle; export 'src/integrations/on_error_integration.dart'; +export 'src/screenshot/sentry_mask_widget.dart'; +export 'src/screenshot/sentry_unmask_widget.dart'; export 'src/screenshot/sentry_screenshot_widget.dart'; export 'src/screenshot/sentry_screenshot_quality.dart'; export 'src/user_interaction/sentry_user_interaction_widget.dart'; diff --git a/flutter/lib/src/replay/masking_config.dart b/flutter/lib/src/replay/masking_config.dart index 5d04e0f46c..7ef0381a6e 100644 --- a/flutter/lib/src/replay/masking_config.dart +++ b/flutter/lib/src/replay/masking_config.dart @@ -13,24 +13,35 @@ class SentryMaskingConfig { : rules = List.of(rules, growable: false), length = rules.length; - bool shouldMask(Element element, T widget) { + MaskingDecision shouldMask(Element element, T widget) { for (int i = 0; i < length; i++) { if (rules[i].appliesTo(widget)) { + // We use a switch here to get lints if more values are added. switch (rules[i].shouldMask(element, widget)) { case MaskingDecision.mask: - return true; + return MaskingDecision.mask; case MaskingDecision.unmask: - return false; + return MaskingDecision.unmask; case MaskingDecision.continueProcessing: // Continue to the next matching rule. } } } - return false; + return MaskingDecision.continueProcessing; } } -enum MaskingDecision { mask, unmask, continueProcessing } +enum MaskingDecision { + /// Mask the widget and its children + mask, + + /// Leave the widget visible, including its children (no more rules will + /// be checked for children). + unmask, + + /// Don't make a decision - continue checking other rules and children. + continueProcessing +} @internal abstract class SentryMaskingRule { diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index a99668050f..d479f8c755 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -40,14 +40,21 @@ class WidgetFilter { return; } - if (config.shouldMask(element, widget)) { - final item = _obscureElementOrParent(element, widget); - if (item != null) { - items.add(item); - } - } else { - // If this element should not be obscured, visit and check its children. - element.visitChildElements(_process); + final decision = config.shouldMask(element, widget); + switch (decision) { + case MaskingDecision.mask: + final item = _obscureElementOrParent(element, widget); + if (item != null) { + items.add(item); + } + break; + case MaskingDecision.unmask: + logger(SentryLevel.debug, "WidgetFilter unmasked: $widget"); + break; + case MaskingDecision.continueProcessing: + // If this element should not be obscured, visit and check its children. + element.visitChildElements(_process); + break; } } @@ -65,7 +72,7 @@ class WidgetFilter { _warnedWidgets.add(widget.hashCode); logger( SentryLevel.warning, - 'WidgetFilter cannot obscure widget $widget: $e.' + 'WidgetFilter cannot mask widget $widget: $e.' 'Obscuring the parent instead: ${parent?.widget}.', stackTrace: stackTrace); } @@ -109,7 +116,7 @@ class WidgetFilter { } assert(() { - logger(SentryLevel.debug, "WidgetFilter obscuring: $widget"); + logger(SentryLevel.debug, "WidgetFilter masking: $widget"); return true; }()); diff --git a/flutter/lib/src/screenshot/sentry_mask_widget.dart b/flutter/lib/src/screenshot/sentry_mask_widget.dart new file mode 100644 index 0000000000..d8f56d6d12 --- /dev/null +++ b/flutter/lib/src/screenshot/sentry_mask_widget.dart @@ -0,0 +1,11 @@ +import 'package:flutter/material.dart'; + +/// Wrapping your widget in [SentryMask] will mask it when capturing replays. +class SentryMask extends StatelessWidget { + final Widget child; + + const SentryMask(this.child, {super.key}); + + @override + Widget build(BuildContext context) => child; +} diff --git a/flutter/lib/src/screenshot/sentry_unmask_widget.dart b/flutter/lib/src/screenshot/sentry_unmask_widget.dart new file mode 100644 index 0000000000..46e4bd72c6 --- /dev/null +++ b/flutter/lib/src/screenshot/sentry_unmask_widget.dart @@ -0,0 +1,11 @@ +import 'package:flutter/material.dart'; + +/// Wrapping your widget in [SentryUnmask] will unmask it when capturing replays. +class SentryUnmask extends StatelessWidget { + final Widget child; + + const SentryUnmask(this.child, {super.key}); + + @override + Widget build(BuildContext context) => child; +} diff --git a/flutter/lib/src/sentry_replay_options.dart b/flutter/lib/src/sentry_replay_options.dart index 8d54fef866..c7d2e7bcac 100644 --- a/flutter/lib/src/sentry_replay_options.dart +++ b/flutter/lib/src/sentry_replay_options.dart @@ -4,6 +4,8 @@ import 'package:meta/meta.dart'; import 'replay/masking_config.dart'; import 'replay/widget_filter.dart'; +import 'screenshot/sentry_mask_widget.dart'; +import 'screenshot/sentry_unmask_widget.dart'; /// Configuration of the experimental replay feature. class SentryReplayOptions { @@ -54,7 +56,16 @@ class SentryReplayOptions { @internal SentryMaskingConfig buildMaskingConfig() { + // First, we collect rules defined by the user (so they're applied first). final rules = _userMaskingRules.toList(); + + // Then, we apply rules for [SentryMask] and [SentryUnmask]. + rules + .add(const SentryMaskingConstantRule(MaskingDecision.mask)); + rules.add( + const SentryMaskingConstantRule(MaskingDecision.unmask)); + + // Then, we apply apply rules based on the configuration. if (maskAllImages) { if (maskAssetImages) { rules.add(const SentryMaskingConstantRule(MaskingDecision.mask)); @@ -74,23 +85,36 @@ class SentryReplayOptions { return SentryMaskingConfig(rules); } - /// Mask given widget type (or it's subclasses) in the replay. - /// Note: masking rules are called in the order they're added. + /// Mask given widget type [T] (or subclasses of [T]) in the replay. + /// Note: masking rules are called in the order they're added so if a previous + /// rule already makes a decision, this rule won't be called. void mask() { + assert(T != SentryMask); + assert(T != SentryUnmask); _userMaskingRules.add(SentryMaskingConstantRule(MaskingDecision.mask)); } - /// Unmask given widget type (or it's subclasses) in the replay, if it would - /// have otherwise been masked by other rules, for example the default rules - /// [maskAllText] or [maskAllImages]. + /// Unmask given widget type [T] (or subclasses of [T]) in the replay. This is + /// useful to explicitly show certain widgets that would otherwise be masked + /// by other rules, for example default [maskAllText] or [maskAllImages]. + /// The [MaskingDecision.unmask] will apply to the widget and its children, + /// so no other rules will be checked for the children. + /// Note: masking rules are called in the order they're added so if a previous + /// rule already makes a decision, this rule won't be called. void unmask() { + assert(T != SentryMask); + assert(T != SentryUnmask); _userMaskingRules.add(SentryMaskingConstantRule(MaskingDecision.unmask)); } - /// Provide a custom callback to decide whether to mask the widget. - /// Note: masking rules are called in the order they're added. + /// Provide a custom callback to decide whether to mask the widget of class + /// [T] (or subclasses of [T]). + /// Note: masking rules are called in the order they're added so if a previous + /// rule already makes a decision, this rule won't be called. void maskCallback( MaskingDecision Function(Element, T) shouldMask) { + assert(T != SentryMask); + assert(T != SentryUnmask); _userMaskingRules.add(SentryMaskingCustomRule(shouldMask)); } diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart index 98c2797e3c..2d3ad11fda 100644 --- a/flutter/test/replay/masking_config_test.dart +++ b/flutter/test/replay/masking_config_test.dart @@ -8,12 +8,18 @@ import 'test_widget.dart'; void main() async { TestWidgetsFlutterBinding.ensureInitialized(); + final alwaysEnabledRules = [ + '$SentryMaskingConstantRule<$SentryMask>(mask)', + '$SentryMaskingConstantRule<$SentryUnmask>(unmask)', + ]; + testWidgets('will not mask if there are no rules', (tester) async { final sut = SentryMaskingConfig([]); final element = await pumpTestElement(tester); expect(sut.rules, isEmpty); expect(sut.length, 0); - expect(sut.shouldMask(element, element.widget), isFalse); + expect(sut.shouldMask(element, element.widget), + MaskingDecision.continueProcessing); }); for (final value in [MaskingDecision.mask, MaskingDecision.unmask]) { @@ -23,8 +29,7 @@ void main() async { SentryMaskingConfig([SentryMaskingConstantRule(value)]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), - value == MaskingDecision.mask); + expect(sut.shouldMask(element, element.widget), value); }); testWidgets('will mask subtype widget by type', (tester) async { @@ -32,8 +37,7 @@ void main() async { SentryMaskingConfig([SentryMaskingConstantRule(value)]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), - value == MaskingDecision.mask); + expect(sut.shouldMask(element, element.widget), value); }); testWidgets('will not mask widget of a different type', (tester) async { @@ -41,7 +45,8 @@ void main() async { SentryMaskingConfig([SentryMaskingConstantRule(value)]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), false); + expect(sut.shouldMask(element, element.widget), + MaskingDecision.continueProcessing); }, skip: value == MaskingDecision.unmask); }); } @@ -57,7 +62,8 @@ void main() async { ]); final rootElement = await pumpTestElement(tester); for (final element in rootElement.findAllChildren()) { - expect(sut.shouldMask(element, element.widget), isFalse); + expect(sut.shouldMask(element, element.widget), + MaskingDecision.continueProcessing); } // Note: there are actually 5 Image widgets in the tree but when it's // inside an `Visibility(visible: false)`, it won't be visited. @@ -73,7 +79,7 @@ void main() async { ]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), isTrue); + expect(sut.shouldMask(element, element.widget), MaskingDecision.mask); }); testWidgets('stops iteration on first "unmask" rule', (tester) async { @@ -85,7 +91,7 @@ void main() async { ]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), isFalse); + expect(sut.shouldMask(element, element.widget), MaskingDecision.unmask); }); testWidgets('retuns false if no rule matches', (tester) async { @@ -97,7 +103,8 @@ void main() async { ]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), isFalse); + expect(sut.shouldMask(element, element.widget), + MaskingDecision.continueProcessing); }); }); @@ -121,6 +128,7 @@ void main() async { test('defaults', () { final sut = SentryReplayOptions(); expect(rulesAsStrings(sut), [ + ...alwaysEnabledRules, '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' @@ -133,6 +141,7 @@ void main() async { ..maskAllImages = true ..maskAssetImages = true; expect(rulesAsStrings(sut), [ + ...alwaysEnabledRules, '$SentryMaskingConstantRule<$Image>(mask)', ]); }); @@ -143,6 +152,7 @@ void main() async { ..maskAllImages = true ..maskAssetImages = false; expect(rulesAsStrings(sut), [ + ...alwaysEnabledRules, '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', ]); }); @@ -153,8 +163,9 @@ void main() async { ..maskAllImages = false ..maskAssetImages = false; expect(rulesAsStrings(sut), [ + ...alwaysEnabledRules, '$SentryMaskingConstantRule<$Text>(mask)', - '$SentryMaskingConstantRule<$EditableText>(mask)' + '$SentryMaskingConstantRule<$EditableText>(mask)', ]); }); @@ -163,11 +174,12 @@ void main() async { ..maskAllText = false ..maskAllImages = false ..maskAssetImages = false; - expect(rulesAsStrings(sut), isEmpty); + expect(rulesAsStrings(sut), alwaysEnabledRules); }); group('user rules', () { final defaultRules = [ + ...alwaysEnabledRules, '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' @@ -222,6 +234,20 @@ void main() async { ...defaultRules ]); }); + test('User cannot add $SentryMask and $SentryUnmask rules', () { + final sut = SentryReplayOptions(); + expect(sut.mask, throwsA(isA())); + expect(sut.mask, throwsA(isA())); + expect(sut.unmask, throwsA(isA())); + expect(sut.unmask, throwsA(isA())); + expect( + () => sut.maskCallback((_, __) => MaskingDecision.mask), + throwsA(isA())); + expect( + () => + sut.maskCallback((_, __) => MaskingDecision.mask), + throwsA(isA())); + }); }); }); } diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index 44fd4b22af..9baf1b7544 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -129,6 +129,27 @@ void main() async { expect(boundsRect(sut.items[2]), '50x20'); }); }); + + testWidgets('respects $SentryMask', (tester) async { + final sut = createSut(redactText: false, redactImages: false); + final element = await pumpTestElement(tester, children: [ + SentryMask(Padding(padding: EdgeInsets.all(100), child: Text('foo'))), + ]); + sut.obscure(element, 1.0, defaultBounds); + expect(sut.items.length, 1); + expect(boundsRect(sut.items[0]), '344x248'); + }); + + testWidgets('respects $SentryUnmask', (tester) async { + final sut = createSut(redactText: true, redactImages: true); + final element = await pumpTestElement(tester, children: [ + SentryUnmask(Text('foo')), + SentryUnmask(newImage()), + SentryUnmask(SentryMask(Text('foo'))), + ]); + sut.obscure(element, 1.0, defaultBounds); + expect(sut.items, isEmpty); + }); } class TestAssetBundle extends CachingAssetBundle { From edaf73296d3209cb71f48f2d239ac81444ebc15d Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Oct 2024 15:22:53 +0200 Subject: [PATCH 15/21] linter issue --- flutter/test/replay/widget_filter_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index 9baf1b7544..8214539221 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -1,4 +1,3 @@ -import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; From 118398e9805724dc53617e1cc7e2439929c99fab Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Oct 2024 15:32:07 +0200 Subject: [PATCH 16/21] chore: changelog --- CHANGELOG.md | 19 +++++++++++++++++++ flutter/lib/sentry_flutter.dart | 1 + 2 files changed, 20 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c15213b151..f82a5f8c1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,25 @@ - Emit `transaction.data` inside `contexts.trace.data` ([#2284](https://github.com/getsentry/sentry-dart/pull/2284)) - Blocking app starts if "appLaunchedInForeground" is false. (Android only) ([#2291](https://github.com/getsentry/sentry-dart/pull/2291)) +- Replay: user-configurable masking (redaction) for widget classes and specific widget instances. ([#2324](https://github.com/getsentry/sentry-dart/pull/2324)) + Some examples of the configuration: + + ```dart + await SentryFlutter.init( + (options) { + ... + options.experimental.replay.mask(); + options.experimental.replay.unmask(); + options.experimental.replay.maskCallback( + (Element element, Text widget) => + (widget.data?.contains('secret') ?? false) + ? MaskingDecision.mask + : MaskingDecision.continueProcessing); + }, + appRunner: () => runApp(MyApp()), + ); + ``` + ### Enhancements diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index ba4d939d86..70a59eec51 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -12,6 +12,7 @@ export 'src/sentry_replay_options.dart'; export 'src/flutter_sentry_attachment.dart'; export 'src/sentry_asset_bundle.dart' show SentryAssetBundle; export 'src/integrations/on_error_integration.dart'; +export 'src/replay/masking_config.dart' show MaskingDecision; export 'src/screenshot/sentry_mask_widget.dart'; export 'src/screenshot/sentry_unmask_widget.dart'; export 'src/screenshot/sentry_screenshot_widget.dart'; From 3101e0a05f970499451f77f3b8ca00a8135efb58 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Oct 2024 21:50:16 +0200 Subject: [PATCH 17/21] rename to SentryMaskingDecision --- CHANGELOG.md | 4 +- flutter/lib/sentry_flutter.dart | 2 +- flutter/lib/src/replay/masking_config.dart | 32 +++++----- flutter/lib/src/replay/widget_filter.dart | 6 +- flutter/lib/src/sentry_replay_options.dart | 32 +++++----- flutter/test/replay/masking_config_test.dart | 62 +++++++++++--------- 6 files changed, 75 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f82a5f8c1a..7041ffb80f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,8 @@ options.experimental.replay.maskCallback( (Element element, Text widget) => (widget.data?.contains('secret') ?? false) - ? MaskingDecision.mask - : MaskingDecision.continueProcessing); + ? SentryMaskingDecision.mask + : SentryMaskingDecision.continueProcessing); }, appRunner: () => runApp(MyApp()), ); diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index 70a59eec51..18b9fdbb43 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -12,7 +12,7 @@ export 'src/sentry_replay_options.dart'; export 'src/flutter_sentry_attachment.dart'; export 'src/sentry_asset_bundle.dart' show SentryAssetBundle; export 'src/integrations/on_error_integration.dart'; -export 'src/replay/masking_config.dart' show MaskingDecision; +export 'src/replay/masking_config.dart' show SentryMaskingDecision; export 'src/screenshot/sentry_mask_widget.dart'; export 'src/screenshot/sentry_unmask_widget.dart'; export 'src/screenshot/sentry_screenshot_widget.dart'; diff --git a/flutter/lib/src/replay/masking_config.dart b/flutter/lib/src/replay/masking_config.dart index 7ef0381a6e..17ee32982d 100644 --- a/flutter/lib/src/replay/masking_config.dart +++ b/flutter/lib/src/replay/masking_config.dart @@ -13,25 +13,26 @@ class SentryMaskingConfig { : rules = List.of(rules, growable: false), length = rules.length; - MaskingDecision shouldMask(Element element, T widget) { + SentryMaskingDecision shouldMask( + Element element, T widget) { for (int i = 0; i < length; i++) { if (rules[i].appliesTo(widget)) { // We use a switch here to get lints if more values are added. switch (rules[i].shouldMask(element, widget)) { - case MaskingDecision.mask: - return MaskingDecision.mask; - case MaskingDecision.unmask: - return MaskingDecision.unmask; - case MaskingDecision.continueProcessing: + case SentryMaskingDecision.mask: + return SentryMaskingDecision.mask; + case SentryMaskingDecision.unmask: + return SentryMaskingDecision.unmask; + case SentryMaskingDecision.continueProcessing: // Continue to the next matching rule. } } } - return MaskingDecision.continueProcessing; + return SentryMaskingDecision.continueProcessing; } } -enum MaskingDecision { +enum SentryMaskingDecision { /// Mask the widget and its children mask, @@ -46,19 +47,19 @@ enum MaskingDecision { @internal abstract class SentryMaskingRule { bool appliesTo(Widget widget) => widget is T; - MaskingDecision shouldMask(Element element, T widget); + SentryMaskingDecision shouldMask(Element element, T widget); const SentryMaskingRule(); } @internal class SentryMaskingCustomRule extends SentryMaskingRule { - final MaskingDecision Function(Element element, T widget) callback; + final SentryMaskingDecision Function(Element element, T widget) callback; const SentryMaskingCustomRule(this.callback); @override - MaskingDecision shouldMask(Element element, T widget) => + SentryMaskingDecision shouldMask(Element element, T widget) => callback(element, widget); @override @@ -67,17 +68,18 @@ class SentryMaskingCustomRule extends SentryMaskingRule { @internal class SentryMaskingConstantRule extends SentryMaskingRule { - final MaskingDecision _value; + final SentryMaskingDecision _value; const SentryMaskingConstantRule(this._value); @override - MaskingDecision shouldMask(Element element, T widget) { + SentryMaskingDecision shouldMask(Element element, T widget) { // This rule only makes sense with true/false. Continue won't do anything. - assert(_value == MaskingDecision.mask || _value == MaskingDecision.unmask); + assert(_value == SentryMaskingDecision.mask || + _value == SentryMaskingDecision.unmask); return _value; } @override String toString() => - '$SentryMaskingConstantRule<$T>(${_value == MaskingDecision.mask ? 'mask' : 'unmask'})'; + '$SentryMaskingConstantRule<$T>(${_value == SentryMaskingDecision.mask ? 'mask' : 'unmask'})'; } diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index d479f8c755..ccf35e393c 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -42,16 +42,16 @@ class WidgetFilter { final decision = config.shouldMask(element, widget); switch (decision) { - case MaskingDecision.mask: + case SentryMaskingDecision.mask: final item = _obscureElementOrParent(element, widget); if (item != null) { items.add(item); } break; - case MaskingDecision.unmask: + case SentryMaskingDecision.unmask: logger(SentryLevel.debug, "WidgetFilter unmasked: $widget"); break; - case MaskingDecision.continueProcessing: + case SentryMaskingDecision.continueProcessing: // If this element should not be obscured, visit and check its children. element.visitChildElements(_process); break; diff --git a/flutter/lib/src/sentry_replay_options.dart b/flutter/lib/src/sentry_replay_options.dart index c7d2e7bcac..a13feedb0c 100644 --- a/flutter/lib/src/sentry_replay_options.dart +++ b/flutter/lib/src/sentry_replay_options.dart @@ -60,15 +60,16 @@ class SentryReplayOptions { final rules = _userMaskingRules.toList(); // Then, we apply rules for [SentryMask] and [SentryUnmask]. - rules - .add(const SentryMaskingConstantRule(MaskingDecision.mask)); - rules.add( - const SentryMaskingConstantRule(MaskingDecision.unmask)); + rules.add(const SentryMaskingConstantRule( + SentryMaskingDecision.mask)); + rules.add(const SentryMaskingConstantRule( + SentryMaskingDecision.unmask)); // Then, we apply apply rules based on the configuration. if (maskAllImages) { if (maskAssetImages) { - rules.add(const SentryMaskingConstantRule(MaskingDecision.mask)); + rules.add( + const SentryMaskingConstantRule(SentryMaskingDecision.mask)); } else { rules .add(const SentryMaskingCustomRule(_maskImagesExceptAssets)); @@ -78,9 +79,10 @@ class SentryReplayOptions { "maskAssetImages can't be true if maskAllImages is false"); } if (maskAllText) { - rules.add(const SentryMaskingConstantRule(MaskingDecision.mask)); rules.add( - const SentryMaskingConstantRule(MaskingDecision.mask)); + const SentryMaskingConstantRule(SentryMaskingDecision.mask)); + rules.add(const SentryMaskingConstantRule( + SentryMaskingDecision.mask)); } return SentryMaskingConfig(rules); } @@ -91,20 +93,22 @@ class SentryReplayOptions { void mask() { assert(T != SentryMask); assert(T != SentryUnmask); - _userMaskingRules.add(SentryMaskingConstantRule(MaskingDecision.mask)); + _userMaskingRules + .add(SentryMaskingConstantRule(SentryMaskingDecision.mask)); } /// Unmask given widget type [T] (or subclasses of [T]) in the replay. This is /// useful to explicitly show certain widgets that would otherwise be masked /// by other rules, for example default [maskAllText] or [maskAllImages]. - /// The [MaskingDecision.unmask] will apply to the widget and its children, + /// The [SentryMaskingDecision.unmask] will apply to the widget and its children, /// so no other rules will be checked for the children. /// Note: masking rules are called in the order they're added so if a previous /// rule already makes a decision, this rule won't be called. void unmask() { assert(T != SentryMask); assert(T != SentryUnmask); - _userMaskingRules.add(SentryMaskingConstantRule(MaskingDecision.unmask)); + _userMaskingRules + .add(SentryMaskingConstantRule(SentryMaskingDecision.unmask)); } /// Provide a custom callback to decide whether to mask the widget of class @@ -112,7 +116,7 @@ class SentryReplayOptions { /// Note: masking rules are called in the order they're added so if a previous /// rule already makes a decision, this rule won't be called. void maskCallback( - MaskingDecision Function(Element, T) shouldMask) { + SentryMaskingDecision Function(Element, T) shouldMask) { assert(T != SentryMask); assert(T != SentryUnmask); _userMaskingRules.add(SentryMaskingCustomRule(shouldMask)); @@ -123,14 +127,14 @@ class SentryReplayOptions { ((sessionSampleRate ?? 0) > 0) || ((onErrorSampleRate ?? 0) > 0); } -MaskingDecision _maskImagesExceptAssets(Element element, Widget widget) { +SentryMaskingDecision _maskImagesExceptAssets(Element element, Widget widget) { if (widget is Image) { final image = widget.image; if (image is AssetBundleImageProvider) { if (WidgetFilter.isBuiltInAssetImage(image, rootBundle)) { - return MaskingDecision.continueProcessing; + return SentryMaskingDecision.continueProcessing; } } } - return MaskingDecision.mask; + return SentryMaskingDecision.mask; } diff --git a/flutter/test/replay/masking_config_test.dart b/flutter/test/replay/masking_config_test.dart index 2d3ad11fda..46e6a99261 100644 --- a/flutter/test/replay/masking_config_test.dart +++ b/flutter/test/replay/masking_config_test.dart @@ -19,10 +19,13 @@ void main() async { expect(sut.rules, isEmpty); expect(sut.length, 0); expect(sut.shouldMask(element, element.widget), - MaskingDecision.continueProcessing); + SentryMaskingDecision.continueProcessing); }); - for (final value in [MaskingDecision.mask, MaskingDecision.unmask]) { + for (final value in [ + SentryMaskingDecision.mask, + SentryMaskingDecision.unmask + ]) { group('$SentryMaskingConstantRule($value)', () { testWidgets('will mask widget by type', (tester) async { final sut = @@ -46,8 +49,8 @@ void main() async { final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); expect(sut.shouldMask(element, element.widget), - MaskingDecision.continueProcessing); - }, skip: value == MaskingDecision.unmask); + SentryMaskingDecision.continueProcessing); + }, skip: value == SentryMaskingDecision.unmask); }); } @@ -57,13 +60,13 @@ void main() async { final sut = SentryMaskingConfig([ SentryMaskingCustomRule((e, w) { called[w.runtimeType] = (called[w.runtimeType] ?? 0) + 1; - return MaskingDecision.continueProcessing; + return SentryMaskingDecision.continueProcessing; }) ]); final rootElement = await pumpTestElement(tester); for (final element in rootElement.findAllChildren()) { expect(sut.shouldMask(element, element.widget), - MaskingDecision.continueProcessing); + SentryMaskingDecision.continueProcessing); } // Note: there are actually 5 Image widgets in the tree but when it's // inside an `Visibility(visible: false)`, it won't be visited. @@ -73,38 +76,40 @@ void main() async { testWidgets('stops iteration on the first "mask" rule', (tester) async { final sut = SentryMaskingConfig([ SentryMaskingCustomRule( - (e, w) => MaskingDecision.continueProcessing), - SentryMaskingCustomRule((e, w) => MaskingDecision.mask), + (e, w) => SentryMaskingDecision.continueProcessing), + SentryMaskingCustomRule((e, w) => SentryMaskingDecision.mask), SentryMaskingCustomRule((e, w) => fail('should not be called')) ]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), MaskingDecision.mask); + expect( + sut.shouldMask(element, element.widget), SentryMaskingDecision.mask); }); testWidgets('stops iteration on first "unmask" rule', (tester) async { final sut = SentryMaskingConfig([ SentryMaskingCustomRule( - (e, w) => MaskingDecision.continueProcessing), - SentryMaskingCustomRule((e, w) => MaskingDecision.unmask), + (e, w) => SentryMaskingDecision.continueProcessing), + SentryMaskingCustomRule((e, w) => SentryMaskingDecision.unmask), SentryMaskingCustomRule((e, w) => fail('should not be called')) ]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); - expect(sut.shouldMask(element, element.widget), MaskingDecision.unmask); + expect(sut.shouldMask(element, element.widget), + SentryMaskingDecision.unmask); }); testWidgets('retuns false if no rule matches', (tester) async { final sut = SentryMaskingConfig([ SentryMaskingCustomRule( - (e, w) => MaskingDecision.continueProcessing), + (e, w) => SentryMaskingDecision.continueProcessing), SentryMaskingCustomRule( - (e, w) => MaskingDecision.continueProcessing), + (e, w) => SentryMaskingDecision.continueProcessing), ]); final rootElement = await pumpTestElement(tester); final element = rootElement.findFirstOfType(); expect(sut.shouldMask(element, element.widget), - MaskingDecision.continueProcessing); + SentryMaskingDecision.continueProcessing); }); }); @@ -116,11 +121,11 @@ void main() async { // These normalize the string on VM & js & wasm: .map((str) => str.replaceAll( RegExp( - r"MaskingDecision from:? [fF]unction '?_maskImagesExceptAssets[@(].*", + r"SentryMaskingDecision from:? [fF]unction '?_maskImagesExceptAssets[@(].*", dotAll: true), - 'MaskingDecision)')) + 'SentryMaskingDecision)')) .map((str) => str.replaceAll( - ' from: (element, widget) => masking_config.MaskingDecision.mask', + ' from: (element, widget) => masking_config.SentryMaskingDecision.mask', '')) .toList(); } @@ -129,7 +134,7 @@ void main() async { final sut = SentryReplayOptions(); expect(rulesAsStrings(sut), [ ...alwaysEnabledRules, - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => SentryMaskingDecision)', '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' ]); @@ -153,7 +158,7 @@ void main() async { ..maskAssetImages = false; expect(rulesAsStrings(sut), [ ...alwaysEnabledRules, - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => SentryMaskingDecision)', ]); }); @@ -180,7 +185,7 @@ void main() async { group('user rules', () { final defaultRules = [ ...alwaysEnabledRules, - '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => MaskingDecision)', + '$SentryMaskingCustomRule<$Image>(Closure: (Element, Widget) => SentryMaskingDecision)', '$SentryMaskingConstantRule<$Text>(mask)', '$SentryMaskingConstantRule<$EditableText>(mask)' ]; @@ -216,11 +221,11 @@ void main() async { sut = SentryReplayOptions(); sut.unmask(); sut.maskCallback( - (Element element, Image widget) => MaskingDecision.mask); + (Element element, Image widget) => SentryMaskingDecision.mask); sut.mask(); expect(rulesAsStrings(sut), [ '$SentryMaskingConstantRule<$Image>(unmask)', - '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $MaskingDecision)', + '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $SentryMaskingDecision)', '$SentryMaskingConstantRule<$Image>(mask)', ...defaultRules ]); @@ -228,9 +233,9 @@ void main() async { test('maskCallback() takes precedence', () { final sut = SentryReplayOptions(); sut.maskCallback( - (Element element, Image widget) => MaskingDecision.mask); + (Element element, Image widget) => SentryMaskingDecision.mask); expect(rulesAsStrings(sut), [ - '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $MaskingDecision)', + '$SentryMaskingCustomRule<$Image>(Closure: ($Element, $Image) => $SentryMaskingDecision)', ...defaultRules ]); }); @@ -241,11 +246,12 @@ void main() async { expect(sut.unmask, throwsA(isA())); expect(sut.unmask, throwsA(isA())); expect( - () => sut.maskCallback((_, __) => MaskingDecision.mask), + () => sut.maskCallback( + (_, __) => SentryMaskingDecision.mask), throwsA(isA())); expect( - () => - sut.maskCallback((_, __) => MaskingDecision.mask), + () => sut.maskCallback( + (_, __) => SentryMaskingDecision.mask), throwsA(isA())); }); }); From 1fe6f3f21bdde1976652a8da18a7f7d6a7c2c6a5 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Oct 2024 21:53:13 +0200 Subject: [PATCH 18/21] mark new replay APIs as experimental --- flutter/lib/src/replay/masking_config.dart | 1 + flutter/lib/src/screenshot/sentry_mask_widget.dart | 2 ++ flutter/lib/src/screenshot/sentry_unmask_widget.dart | 2 ++ flutter/lib/src/sentry_replay_options.dart | 1 + 4 files changed, 6 insertions(+) diff --git a/flutter/lib/src/replay/masking_config.dart b/flutter/lib/src/replay/masking_config.dart index 17ee32982d..3c8d6a118a 100644 --- a/flutter/lib/src/replay/masking_config.dart +++ b/flutter/lib/src/replay/masking_config.dart @@ -32,6 +32,7 @@ class SentryMaskingConfig { } } +@experimental enum SentryMaskingDecision { /// Mask the widget and its children mask, diff --git a/flutter/lib/src/screenshot/sentry_mask_widget.dart b/flutter/lib/src/screenshot/sentry_mask_widget.dart index d8f56d6d12..d750fe7f1b 100644 --- a/flutter/lib/src/screenshot/sentry_mask_widget.dart +++ b/flutter/lib/src/screenshot/sentry_mask_widget.dart @@ -1,6 +1,8 @@ import 'package:flutter/material.dart'; +import 'package:meta/meta.dart'; /// Wrapping your widget in [SentryMask] will mask it when capturing replays. +@experimental class SentryMask extends StatelessWidget { final Widget child; diff --git a/flutter/lib/src/screenshot/sentry_unmask_widget.dart b/flutter/lib/src/screenshot/sentry_unmask_widget.dart index 46e4bd72c6..cf1fa52b5d 100644 --- a/flutter/lib/src/screenshot/sentry_unmask_widget.dart +++ b/flutter/lib/src/screenshot/sentry_unmask_widget.dart @@ -1,6 +1,8 @@ import 'package:flutter/material.dart'; +import 'package:meta/meta.dart'; /// Wrapping your widget in [SentryUnmask] will unmask it when capturing replays. +@experimental class SentryUnmask extends StatelessWidget { final Widget child; diff --git a/flutter/lib/src/sentry_replay_options.dart b/flutter/lib/src/sentry_replay_options.dart index a13feedb0c..a6e83fec4f 100644 --- a/flutter/lib/src/sentry_replay_options.dart +++ b/flutter/lib/src/sentry_replay_options.dart @@ -8,6 +8,7 @@ import 'screenshot/sentry_mask_widget.dart'; import 'screenshot/sentry_unmask_widget.dart'; /// Configuration of the experimental replay feature. +@experimental class SentryReplayOptions { double? _sessionSampleRate; From 35495b2f7faede3598dc180f1bd0b75d9f189376 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:39:42 +0200 Subject: [PATCH 19/21] Update flutter/lib/src/replay/masking_config.dart Co-authored-by: Giancarlo Buenaflor --- flutter/lib/src/replay/masking_config.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/flutter/lib/src/replay/masking_config.dart b/flutter/lib/src/replay/masking_config.dart index 3c8d6a118a..a9f6abbbdc 100644 --- a/flutter/lib/src/replay/masking_config.dart +++ b/flutter/lib/src/replay/masking_config.dart @@ -47,6 +47,7 @@ enum SentryMaskingDecision { @internal abstract class SentryMaskingRule { + @pragma('vm:prefer-inline') bool appliesTo(Widget widget) => widget is T; SentryMaskingDecision shouldMask(Element element, T widget); From 4fa9a3d406d27b9b4cd6e5393292af28283659be Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 10 Oct 2024 17:43:49 +0200 Subject: [PATCH 20/21] chore: update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f498069ed..82f658bb01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,12 @@ ); ``` + Also, you can wrap any of your widgets with `SentryMask()` or `SentryUnmask()` widgets to mask/unmask them, respectively. For example: + + ```dart +  SentryUnmask(Text('Not secret at all')); + ``` + ### Enhancements - Use native spotlight integrations on Flutter Android, iOS, macOS ([#2285](https://github.com/getsentry/sentry-dart/pull/2285)) From be99d931475db82c7741e6e03f9a01d7380d207d Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 10 Oct 2024 20:05:04 +0200 Subject: [PATCH 21/21] test: mask parent if masking the child fails --- flutter/lib/src/replay/widget_filter.dart | 13 +++++++++++++ flutter/test/replay/widget_filter_test.dart | 14 ++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index ccf35e393c..32231a0d12 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -16,6 +16,10 @@ class WidgetFilter { late Rect _bounds; final _warnedWidgets = {}; + /// Used to test _obscureElementOrParent + @visibleForTesting + bool throwInObscure = false; + WidgetFilter(this.config, this.logger); void obscure(BuildContext context, double pixelRatio, Rect bounds) { @@ -129,6 +133,15 @@ class WidgetFilter { color = (widget).color; } + // test-only code + assert(() { + if (throwInObscure) { + throwInObscure = false; + return false; + } + return true; + }()); + return WidgetFilterItem(color ?? _defaultColor, rect); } diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index 8214539221..ad76e9bfaa 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -149,6 +149,20 @@ void main() async { sut.obscure(element, 1.0, defaultBounds); expect(sut.items, isEmpty); }); + + testWidgets('obscureElementOrParent', (tester) async { + final sut = createSut(redactText: true); + final element = await pumpTestElement(tester, children: [ + Padding(padding: EdgeInsets.all(100), child: Text('foo')), + ]); + sut.obscure(element, 1.0, defaultBounds); + expect(sut.items.length, 1); + expect(boundsRect(sut.items[0]), '144x48'); + sut.throwInObscure = true; + sut.obscure(element, 1.0, defaultBounds); + expect(sut.items.length, 1); + expect(boundsRect(sut.items[0]), '344x248'); + }); } class TestAssetBundle extends CachingAssetBundle {