From c853fdaa2040e94c3786bb830d2ac8800e72cf4f Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 20 Aug 2024 09:54:02 +0200 Subject: [PATCH 1/2] feat: asset images don't need to be obscured --- flutter/example/lib/main.dart | 10 +++ flutter/lib/sentry_flutter.dart | 2 +- flutter/lib/src/replay/widget_filter.dart | 32 +++++++++- flutter/lib/src/sentry_asset_bundle.dart | 7 ++ flutter/test/replay/test_widget.dart | 71 ++++++++++----------- flutter/test/replay/widget_filter_test.dart | 41 +++++++++++- 6 files changed, 123 insertions(+), 40 deletions(-) diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index ed8961741c..0f1cd9279d 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -1043,6 +1043,8 @@ Future showDialogWithTextAndImage(BuildContext context) async { await DefaultAssetBundle.of(context).loadString('assets/lorem-ipsum.txt'); if (!context.mounted) return; + final imageBytes = + await DefaultAssetBundle.of(context).load('assets/sentry-wordmark.png'); await showDialog( context: context, // gets tracked if using SentryNavigatorObserver @@ -1056,7 +1058,15 @@ Future showDialogWithTextAndImage(BuildContext context) async { child: Column( mainAxisSize: MainAxisSize.min, children: [ + // Use various ways an image is included in the app. + // Local asset images are not obscured in replay recording. Image.asset('assets/sentry-wordmark.png'), + Image.asset('assets/sentry-wordmark.png', bundle: rootBundle), + Image.asset('assets/sentry-wordmark.png', + bundle: DefaultAssetBundle.of(context)), + Image.network( + 'https://www.gstatic.com/recaptcha/api2/logo_48.png'), + Image.memory(imageBytes.buffer.asUint8List()), Text(text), ], ), diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index bea9016630..c74013e81e 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -10,7 +10,7 @@ export 'src/sentry_flutter.dart'; export 'src/sentry_flutter_options.dart'; export 'src/sentry_replay_options.dart'; export 'src/flutter_sentry_attachment.dart'; -export 'src/sentry_asset_bundle.dart'; +export 'src/sentry_asset_bundle.dart' show SentryAssetBundle; export 'src/integrations/on_error_integration.dart'; export 'src/screenshot/sentry_screenshot_widget.dart'; export 'src/screenshot/sentry_screenshot_quality.dart'; diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index 83e069cb97..f269dd041f 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -1,8 +1,9 @@ +import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; -import 'package:sentry/sentry.dart'; import '../../sentry_flutter.dart'; +import '../sentry_asset_bundle.dart'; @internal class WidgetFilter { @@ -14,11 +15,14 @@ class WidgetFilter { late double _pixelRatio; late Rect _bounds; final _warnedWidgets = {}; + final AssetBundle _rootAssetBundle; WidgetFilter( {required this.redactText, required this.redactImages, - required this.logger}); + required this.logger, + @visibleForTesting AssetBundle? rootAssetBundle}) + : _rootAssetBundle = rootAssetBundle ?? rootBundle; void obscure(BuildContext context, double pixelRatio, Rect bounds) { _pixelRatio = pixelRatio; @@ -57,6 +61,14 @@ class WidgetFilter { } else if (redactText && 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; + } + } color = widget.color; } else { // No other type is currently obscured. @@ -115,6 +127,22 @@ class WidgetFilter { return true; } + @visibleForTesting + @pragma('vm:prefer-inline') + bool isBuiltInAssetImage(AssetBundleImageProvider image) { + late final AssetBundle? bundle; + if (image is AssetImage) { + bundle = image.bundle; + } else if (image is ExactAssetImage) { + bundle = image.bundle; + } else { + return false; + } + return (bundle == null || + bundle == _rootAssetBundle || + (bundle is SentryAssetBundle && bundle.bundle == _rootAssetBundle)); + } + @pragma('vm:prefer-inline') void _cantObscure(Widget widget, String message) { if (!_warnedWidgets.contains(widget.hashCode)) { diff --git a/flutter/lib/src/sentry_asset_bundle.dart b/flutter/lib/src/sentry_asset_bundle.dart index 52d1a2da0c..a41288209f 100644 --- a/flutter/lib/src/sentry_asset_bundle.dart +++ b/flutter/lib/src/sentry_asset_bundle.dart @@ -7,6 +7,7 @@ import 'dart:ui'; import 'package:flutter/services.dart'; import 'package:flutter/material.dart'; +import 'package:meta/meta.dart'; import 'package:sentry/sentry.dart'; typedef _StringParser = Future Function(String value); @@ -375,3 +376,9 @@ class SentryAssetBundle implements AssetBundle { as Future; } } + +@internal +extension SentryAssetBundleInternal on SentryAssetBundle { + /// Returns the wrapped [AssetBundle]. + AssetBundle get bundle => _bundle; +} diff --git a/flutter/test/replay/test_widget.dart b/flutter/test/replay/test_widget.dart index e85dfacaf8..c5321ce750 100644 --- a/flutter/test/replay/test_widget.dart +++ b/flutter/test/replay/test_widget.dart @@ -1,10 +1,10 @@ -import 'dart:typed_data'; - import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -Future pumpTestElement(WidgetTester tester) async { +Future pumpTestElement(WidgetTester tester, + {List? children}) async { await tester.pumpWidget( MaterialApp( home: SentryWidget( @@ -14,25 +14,26 @@ Future pumpTestElement(WidgetTester tester) async { child: Opacity( opacity: 0.5, child: Column( - children: [ - newImage(), - const Padding( - padding: EdgeInsets.all(15), - child: Center(child: Text('Centered text')), - ), - ElevatedButton( - onPressed: () {}, - child: Text('Button title'), - ), - newImage(), - // Invisible widgets won't be obscured. - Visibility(visible: false, child: Text('Invisible text')), - Visibility(visible: false, child: newImage()), - Opacity(opacity: 0, child: Text('Invisible text')), - Opacity(opacity: 0, child: newImage()), - Offstage(offstage: true, child: Text('Offstage text')), - Offstage(offstage: true, child: newImage()), - ], + children: children ?? + [ + newImage(), + const Padding( + padding: EdgeInsets.all(15), + child: Center(child: Text('Centered text')), + ), + ElevatedButton( + onPressed: () {}, + child: Text('Button title'), + ), + newImage(), + // Invisible widgets won't be obscured. + Visibility(visible: false, child: Text('Invisible text')), + Visibility(visible: false, child: newImage()), + Opacity(opacity: 0, child: Text('Invisible text')), + Opacity(opacity: 0, child: newImage()), + Offstage(offstage: true, child: Text('Offstage text')), + Offstage(offstage: true, child: newImage()), + ], ), ), ), @@ -43,17 +44,15 @@ Future pumpTestElement(WidgetTester tester) async { return TestWidgetsFlutterBinding.instance.rootElement!; } -Image newImage() => Image.memory( - Uint8List.fromList([ - 66, 77, 142, 0, 0, 0, 0, 0, 0, 0, 138, 0, 0, 0, 124, 0, 0, 0, 1, 0, - 0, 0, 255, 255, 255, 255, 1, 0, 32, 0, 3, 0, 0, 0, 4, 0, 0, 0, 19, - 11, 0, 0, 19, 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 0, 0, - 255, 0, 0, 255, 0, 0, 0, 0, 0, 0, 255, 66, 71, 82, 115, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 135, 135, 135, 255, - // This comment prevents dartfmt reformatting this to single-item lines. - ]), - width: 1, - height: 1, - ); +final testImageData = Uint8List.fromList([ + 66, 77, 142, 0, 0, 0, 0, 0, 0, 0, 138, 0, 0, 0, 124, 0, 0, 0, 1, 0, + 0, 0, 255, 255, 255, 255, 1, 0, 32, 0, 3, 0, 0, 0, 4, 0, 0, 0, 19, + 11, 0, 0, 19, 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 0, 0, + 255, 0, 0, 255, 0, 0, 0, 0, 0, 0, 255, 66, 71, 82, 115, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 135, 135, 135, 255, + // This comment prevents dartfmt reformatting this to single-item lines. +]); + +Image newImage() => Image.memory(testImageData, width: 1, height: 1); diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index 3e17f2b5b6..3f4136b90a 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -1,5 +1,8 @@ -import 'package:flutter/material.dart'; +import 'package:flutter/rendering.dart'; +import 'package:flutter/services.dart'; +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/widget_filter.dart'; import 'test_widget.dart'; @@ -7,12 +10,15 @@ import 'test_widget.dart'; void main() async { TestWidgetsFlutterBinding.ensureInitialized(); const defaultBounds = Rect.fromLTRB(0, 0, 1000, 1000); + 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, ); group('redact text', () { @@ -47,6 +53,32 @@ void main() async { expect(sut.items.length, 2); }); + // Note: we cannot currently test actual asset images without either: + // - introducing assets to the package because those wouldn't get tree-shaken in final user apps (https://github.com/flutter/flutter/issues/64106) + // - using a mock asset bundle implementation, because the image widget loads AssetManifest.bin first and we don't have a way to mock that (https://github.com/flutter/flutter/issues/126860) + // Therefore we only check the function that actually decides whether the image is a built-in asset image. + for (var newAssetImage in [AssetImage.new, ExactAssetImage.new]) { + 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)), + isTrue); + expect(sut.isBuiltInAssetImage(newAssetImage('', bundle: otherBundle)), + isFalse); + expect( + sut.isBuiltInAssetImage(newAssetImage('', + bundle: SentryAssetBundle(bundle: rootBundle))), + isTrue); + expect( + sut.isBuiltInAssetImage(newAssetImage('', + bundle: SentryAssetBundle(bundle: otherBundle))), + isFalse); + }); + } + testWidgets('does not redact text when disabled', (tester) async { final sut = createSut(redactImages: false); final element = await pumpTestElement(tester); @@ -63,3 +95,10 @@ void main() async { }); }); } + +class TestAssetBundle extends CachingAssetBundle { + @override + Future load(String key) async { + return ByteData(0); + } +} From 4c2b3955ee67fdfc9c76ed9beb212567c2428c0d Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 4 Sep 2024 19:56:00 +0200 Subject: [PATCH 2/2] chore: update changelog --- CHANGELOG.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b915e3dcf6..b12c189997 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,29 +4,29 @@ ### Features -- Support allowUrls and denyUrls for Flutter Web ([#2227](https://github.com/getsentry/sentry-dart/pull/2227)) +- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208), [#2269](https://github.com/getsentry/sentry-dart/pull/2269)). + + To try out replay, you can set following options (access is limited to early access orgs on Sentry. If you're interested, [sign up for the waitlist](https://sentry.io/lp/mobile-replay-beta/)): ```dart await SentryFlutter.init( (options) { ... - options.allowUrls = ["^https://sentry.com.*\$", "my-custom-domain"]; - options.denyUrls = ["^.*ends-with-this\$", "denied-url"]; + options.experimental.replay.sessionSampleRate = 1.0; + options.experimental.replay.errorSampleRate = 1.0; }, appRunner: () => runApp(MyApp()), ); ``` -- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208)). - - To try out replay, you can set following options (access is limited to early access orgs on Sentry. If you're interested, [sign up for the waitlist](https://sentry.io/lp/mobile-replay-beta/)): +- Support allowUrls and denyUrls for Flutter Web ([#2227](https://github.com/getsentry/sentry-dart/pull/2227)) ```dart await SentryFlutter.init( (options) { ... - options.experimental.replay.sessionSampleRate = 1.0; - options.experimental.replay.errorSampleRate = 1.0; + options.allowUrls = ["^https://sentry.com.*\$", "my-custom-domain"]; + options.denyUrls = ["^.*ends-with-this\$", "denied-url"]; }, appRunner: () => runApp(MyApp()), );