feat: asset images don't need to be obscured in replay (#2269)

* feat: asset images don't need to be obscured * chore: update changelog
getsentry · Sep 4, 2024 · a40bb7c · a40bb7c
1 parent 3a16179
commit a40bb7c
Show file tree

Hide file tree

Showing 7 changed files with 131 additions and 48 deletions.
diff --git 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()),
   );

diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart
@@ -1043,6 +1043,8 @@ Future<void> 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<void>(
     context: context,
     // gets tracked if using SentryNavigatorObserver
@@ -1056,7 +1058,15 @@ Future<void> 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
@@ -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
@@ -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 = <int>{};
+  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
@@ -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<T> = Future<T> Function(String value);
@@ -375,3 +376,9 @@ class SentryAssetBundle implements AssetBundle {
         as Future<T>;
   }
 }
+
+@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
@@ -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<Element> pumpTestElement(WidgetTester tester) async {
+Future<Element> pumpTestElement(WidgetTester tester,
+    {List<Widget>? children}) async {
   await tester.pumpWidget(
     MaterialApp(
       home: SentryWidget(
@@ -14,25 +14,26 @@ Future<Element> pumpTestElement(WidgetTester tester) async {
             child: Opacity(
               opacity: 0.5,
               child: Column(
-                children: <Widget>[
-                  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 ??
+                    <Widget>[
+                      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<Element> 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
@@ -1,18 +1,24 @@
-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';
 
 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<ByteData> load(String key) async {
+    return ByteData(0);
+  }
+}