Skip to content

Commit

Permalink
feat: asset images don't need to be obscured
Browse files Browse the repository at this point in the history
  • Loading branch information
vaind committed Aug 20, 2024
1 parent 1526023 commit d585ea8
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 40 deletions.
10 changes: 10 additions & 0 deletions flutter/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,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
Expand All @@ -1044,7 +1046,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),
],
),
Expand Down
2 changes: 1 addition & 1 deletion flutter/lib/sentry_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
32 changes: 30 additions & 2 deletions flutter/lib/src/replay/widget_filter.dart
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)) {
Expand Down
7 changes: 7 additions & 0 deletions flutter/lib/src/sentry_asset_bundle.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -375,3 +376,9 @@ class SentryAssetBundle implements AssetBundle {
as Future<T>;
}
}

@internal
extension SentryAssetBundleInternal on SentryAssetBundle {
/// Returns the wrapped [AssetBundle].
AssetBundle get bundle => _bundle;
}
71 changes: 35 additions & 36 deletions flutter/test/replay/test_widget.dart
Original file line number Diff line number Diff line change
@@ -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(
Expand All @@ -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()),
],
),
),
),
Expand All @@ -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);
41 changes: 40 additions & 1 deletion flutter/test/replay/widget_filter_test.dart
Original file line number Diff line number Diff line change
@@ -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', () {
Expand Down Expand Up @@ -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);
Expand All @@ -63,3 +95,10 @@ void main() async {
});
});
}

class TestAssetBundle extends CachingAssetBundle {
@override
Future<ByteData> load(String key) async {
return ByteData(0);
}
}

0 comments on commit d585ea8

Please sign in to comment.