Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

redact screenshots via view hierarchy #2361

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

- Switching from traditional screenshot to view hierarchy for screenshots which allows redacting ([#2361](https://github.com/getsentry/sentry-dart/pull/2361))
Copy link
Contributor

Choose a reason for hiding this comment

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

the wording is a bit confusing from a user perspective we can improve it by sayingAllow Screenshots to be masked for privacy reasons or something like that

Also show an exampl how a user can enable it and please add the screenshot & replay masking behaviour (the one that I commented) as sub bullet points for more context so it's clear how it behaves


## 8.10.0-beta.2

### Fixes
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 @@ -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 SentryMaskingDecision;
export 'src/screenshot/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';
Expand Down
97 changes: 24 additions & 73 deletions flutter/lib/src/event_processor/screenshot_event_processor.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import 'dart:async';
import 'dart:math';
import 'dart:typed_data';
import 'dart:ui';

import 'package:sentry/sentry.dart';
import '../screenshot/recorder.dart';
import '../screenshot/recorder_config.dart';
import '../screenshot/sentry_screenshot_widget.dart';
import '../sentry_flutter_options.dart';
import 'package:flutter/rendering.dart';
import '../renderer/renderer.dart';
import 'package:flutter/widgets.dart' as widget;

Expand Down Expand Up @@ -75,83 +75,34 @@
return event;
}

final bytes = await _createScreenshot();
if (bytes != null) {
hint.screenshot = SentryAttachment.fromScreenshotData(bytes);
}
return event;
}
// ignore: deprecated_member_use
var recorder = ScreenshotRecorder(
ScreenshotRecorderConfig(quality: _options.screenshotQuality),
_options);

Future<Uint8List?> _createScreenshot() async {
try {
final renderObject =
sentryScreenshotWidgetGlobalKey.currentContext?.findRenderObject();
if (renderObject is RenderRepaintBoundary) {
// ignore: deprecated_member_use
final pixelRatio = window.devicePixelRatio;
var imageResult = _getImage(renderObject, pixelRatio);
Image image;
if (imageResult is Future<Image>) {
image = await imageResult;
} else {
image = imageResult;
}
// At the time of writing there's no other image format available which
// Sentry understands.
Uint8List? _screenshotCache;
martinhaintz marked this conversation as resolved.
Show resolved Hide resolved

if (image.width == 0 || image.height == 0) {
_options.logger(SentryLevel.debug,
'View\'s width and height is zeroed, not taking screenshot.');
return null;
}

final targetResolution = _options.screenshotQuality.targetResolution();
if (targetResolution != null) {
var ratioWidth = targetResolution / image.width;
var ratioHeight = targetResolution / image.height;
var ratio = min(ratioWidth, ratioHeight);
if (ratio > 0.0 && ratio < 1.0) {
imageResult = _getImage(renderObject, ratio * pixelRatio);
if (imageResult is Future<Image>) {
image = await imageResult;
} else {
image = imageResult;
}
}
}
final byteData = await image.toByteData(format: ImageByteFormat.png);
await recorder.capture((Image image) async {
_screenshotCache = await _convertImageToUint8List(image);
});

final bytes = byteData?.buffer.asUint8List();
if (bytes?.isNotEmpty == true) {
return bytes;
} else {
_options.logger(SentryLevel.debug,
'Screenshot is 0 bytes, not attaching the image.');
return null;
}
}
} catch (exception, stackTrace) {
_options.logger(
SentryLevel.error,
'Taking screenshot failed.',
exception: exception,
stackTrace: stackTrace,
);
if (_options.automatedTestMode) {
rethrow;
}
if (_screenshotCache != null) {
hint.screenshot = SentryAttachment.fromScreenshotData(_screenshotCache!);
}
return null;

return event;
}

FutureOr<Image> _getImage(
RenderRepaintBoundary repaintBoundary, double pixelRatio) {
// This one is a hack to use https://api.flutter.dev/flutter/rendering/RenderRepaintBoundary/toImage.html on versions older than 3.7 and https://api.flutter.dev/flutter/rendering/RenderRepaintBoundary/toImageSync.html on versions equal or newer than 3.7
try {
return (repaintBoundary as dynamic).toImageSync(pixelRatio: pixelRatio)
as Image;
} on NoSuchMethodError catch (_) {
return repaintBoundary.toImage(pixelRatio: pixelRatio);
Future<Uint8List?> _convertImageToUint8List(Image image) async {
final byteData = await image.toByteData(format: ImageByteFormat.png);

final bytes = byteData?.buffer.asUint8List();
if (bytes?.isNotEmpty == true) {
return bytes;
} else {
_options.logger(

Check warning on line 103 in flutter/lib/src/event_processor/screenshot_event_processor.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/event_processor/screenshot_event_processor.dart#L103

Added line #L103 was not covered by tests
SentryLevel.debug, 'Screenshot is 0 bytes, not attaching the image.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ever happened to you while testing? Looks like a bug.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, not that I am aware of. This code was already here before.
Maybe @denrase knows more?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@vaind I looked up the toByteData method, and it will return null if an error occurs and also throws an exception. I would rewrite this, with a try/catch block, but we can also remove this code, if you think, that we don`t need it. WDYT?

return null;
}
}
}
4 changes: 2 additions & 2 deletions flutter/lib/src/native/cocoa/sentry_native_cocoa.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import 'package:meta/meta.dart';

import '../../../sentry_flutter.dart';
import '../../event_processor/replay_event_processor.dart';
import '../../screenshot/recorder.dart';
import '../../screenshot/recorder_config.dart';
import '../../replay/integration.dart';
import '../../replay/recorder.dart';
import '../../replay/recorder_config.dart';
import '../sentry_native_channel.dart';
import 'binding.dart' as cocoa;

Expand Down
2 changes: 1 addition & 1 deletion flutter/lib/src/native/java/sentry_native_java.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import '../../../sentry_flutter.dart';
import '../../event_processor/replay_event_processor.dart';
import '../../replay/integration.dart';
import '../../replay/scheduled_recorder.dart';
import '../../replay/recorder_config.dart';
import '../../replay/scheduled_recorder_config.dart';
import '../sentry_native_channel.dart';

// Note: currently this doesn't do anything. Later, it shall be used with
Expand Down
4 changes: 2 additions & 2 deletions flutter/lib/src/replay/scheduled_recorder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import 'dart:async';
import 'dart:ui';

import 'package:meta/meta.dart';
import 'scheduled_recorder_config.dart';

import '../../sentry_flutter.dart';
import 'recorder.dart';
import 'recorder_config.dart';
import '../screenshot/recorder.dart';
import 'scheduler.dart';

@internal
Expand Down
11 changes: 11 additions & 0 deletions flutter/lib/src/replay/scheduled_recorder_config.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import '../screenshot/recorder_config.dart';

class ScheduledScreenshotRecorderConfig extends ScreenshotRecorderConfig {
final int frameRate;

const ScheduledScreenshotRecorderConfig({
super.width,
super.height,
required this.frameRate,
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import 'package:flutter/rendering.dart';
import 'package:meta/meta.dart';

import '../../sentry_flutter.dart';
import '../replay/scheduled_recorder_config.dart';
import 'masking_config.dart';
import 'recorder_config.dart';
import 'widget_filter.dart';

Expand All @@ -21,7 +23,13 @@ class ScreenshotRecorder {
bool warningLogged = false;

ScreenshotRecorder(this.config, this.options) {
final maskingConfig = options.experimental.replay.buildMaskingConfig();
SentryMaskingConfig maskingConfig;
if (config is ScheduledScreenshotRecorderConfig) {
maskingConfig = options.experimental.replay.buildMaskingConfig();
} else {
maskingConfig = options.experimental.screenshot.buildMaskingConfig();
}
martinhaintz marked this conversation as resolved.
Show resolved Hide resolved

if (maskingConfig.length > 0) {
_widgetFilter = WidgetFilter(maskingConfig, options.logger);
}
Expand Down Expand Up @@ -82,12 +90,21 @@ class ScreenshotRecorder {
final picture = recorder.endRecording();

try {
final finalImage = await picture.toImage(
(srcWidth * pixelRatio).round(), (srcHeight * pixelRatio).round());
Image finalImage;
if (config is ScheduledScreenshotRecorderConfig) {
finalImage = await picture.toImage((srcWidth * pixelRatio).round(),
(srcHeight * pixelRatio).round());
} else {
final targetHeight = config.quality
.calculateHeight(srcWidth.toInt(), srcHeight.toInt());
final targetWidth = config.quality
.calculateWidth(srcWidth.toInt(), srcHeight.toInt());
finalImage = await picture.toImage(targetWidth, targetHeight);
martinhaintz marked this conversation as resolved.
Show resolved Hide resolved
}
try {
await callback(finalImage);
} finally {
finalImage.dispose();
finalImage.dispose(); // image needs to be disposed manually
}
} finally {
picture.dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,19 @@ import 'dart:math';

import 'package:meta/meta.dart';

import '../../sentry_flutter.dart';

@internal
class ScreenshotRecorderConfig {
final int? width;
final int? height;
final SentryScreenshotQuality quality;

const ScreenshotRecorderConfig({this.width, this.height});
const ScreenshotRecorderConfig({
this.width,
this.height,
this.quality = SentryScreenshotQuality.full,
});

double getPixelRatio(double srcWidth, double srcHeight) {
assert((width == null) == (height == null));
Expand All @@ -17,13 +24,3 @@ class ScreenshotRecorderConfig {
return min(width! / srcWidth, height! / srcHeight);
}
}

class ScheduledScreenshotRecorderConfig extends ScreenshotRecorderConfig {
final int frameRate;

const ScheduledScreenshotRecorderConfig({
super.width,
super.height,
required this.frameRate,
});
}
26 changes: 26 additions & 0 deletions flutter/lib/src/screenshot/sentry_screenshot_quality.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,30 @@ enum SentryScreenshotQuality {
return 854;
}
}

int calculateHeight(int width, int height) {
martinhaintz marked this conversation as resolved.
Show resolved Hide resolved
if (this == SentryScreenshotQuality.full) {
return height;
} else {
if (height > width) {
return targetResolution()!;
} else {
var ratio = targetResolution()! / width;
return (height * ratio).round();
}
}
}

int calculateWidth(int width, int height) {
if (this == SentryScreenshotQuality.full) {
return width;
} else {
if (width > height) {
return targetResolution()!;
} else {
var ratio = targetResolution()! / height;
return (width * ratio).round();
}
}
}
}
2 changes: 2 additions & 0 deletions flutter/lib/src/sentry_flutter_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import 'event_processor/screenshot_event_processor.dart';
import 'screenshot/sentry_screenshot_widget.dart';
import 'sentry_flutter.dart';
import 'sentry_replay_options.dart';
import 'sentry_screenshot_options.dart';
import 'user_interaction/sentry_user_interaction_widget.dart';

/// This class adds options which are only available in a Flutter environment.
Expand Down Expand Up @@ -380,6 +381,7 @@ class SentryFlutterOptions extends SentryOptions {
class _SentryFlutterExperimentalOptions {
/// Replay recording configuration.
final replay = SentryReplayOptions();
final screenshot = SentryScreenshotOptions();
}

/// Callback being executed in [ScreenshotEventProcessor], deciding if a
Expand Down
Loading
Loading