safer svg parsing

This commit is contained in:
Thibault Deckers 2022-11-07 22:02:27 +01:00
parent eb3acaa307
commit 7d05fb6aef
11 changed files with 77 additions and 40 deletions

View file

@ -68,6 +68,7 @@ class MediaFetchBytesHandler(private val context: Context) : MethodCallHandler {
val uri = call.argument<String>("uri")?.let { Uri.parse(it) } val uri = call.argument<String>("uri")?.let { Uri.parse(it) }
val mimeType = call.argument<String>("mimeType") val mimeType = call.argument<String>("mimeType")
val pageId = call.argument<Int>("pageId") val pageId = call.argument<Int>("pageId")
val sizeBytes = call.argument<Number>("sizeBytes")?.toLong()
val sampleSize = call.argument<Int>("sampleSize") val sampleSize = call.argument<Int>("sampleSize")
val x = call.argument<Int>("regionX") val x = call.argument<Int>("regionX")
val y = call.argument<Int>("regionY") val y = call.argument<Int>("regionY")
@ -85,6 +86,7 @@ class MediaFetchBytesHandler(private val context: Context) : MethodCallHandler {
when (mimeType) { when (mimeType) {
MimeTypes.SVG -> SvgRegionFetcher(context).fetch( MimeTypes.SVG -> SvgRegionFetcher(context).fetch(
uri = uri, uri = uri,
sizeBytes = sizeBytes,
regionRect = regionRect, regionRect = regionRect,
imageWidth = imageWidth, imageWidth = imageWidth,
imageHeight = imageHeight, imageHeight = imageHeight,

View file

@ -23,11 +23,21 @@ class SvgRegionFetcher internal constructor(
suspend fun fetch( suspend fun fetch(
uri: Uri, uri: Uri,
sizeBytes: Long?,
regionRect: Rect, regionRect: Rect,
imageWidth: Int, imageWidth: Int,
imageHeight: Int, imageHeight: Int,
result: MethodChannel.Result, result: MethodChannel.Result,
) { ) {
if (sizeBytes != null && sizeBytes > FILE_SIZE_DANGER_THRESHOLD) {
val availableHeapSize = Runtime.getRuntime().let { it.maxMemory() - (it.totalMemory() - it.freeMemory()) }
if (sizeBytes > availableHeapSize) {
// opening an SVG that large would yield an OOM during parsing from `com.caverock.androidsvg.SVGParser`
result.error("fetch-read-large", "SVG too large at $sizeBytes bytes, with only $availableHeapSize free bytes, for uri=$uri regionRect=$regionRect", null)
return
}
}
var currentSvgRef = lastSvgRef var currentSvgRef = lastSvgRef
if (currentSvgRef != null && currentSvgRef.uri != uri) { if (currentSvgRef != null && currentSvgRef.uri != uri) {
currentSvgRef = null currentSvgRef = null
@ -103,4 +113,9 @@ class SvgRegionFetcher internal constructor(
val uri: Uri, val uri: Uri,
val svg: SVG, val svg: SVG,
) )
companion object {
// arbitrary size to detect files that may yield an OOM
private const val FILE_SIZE_DANGER_THRESHOLD = 10 * (1 shl 20) // MB
}
} }

View file

@ -117,16 +117,16 @@ object Metadata {
return date.time + parseSubSecond(subSecond) return date.time + parseSubSecond(subSecond)
} }
// opening large PSD/TIFF files yields an OOM (both with `metadata-extractor` v2.15.0 and `ExifInterface` v1.3.1), // Opening large PSD/TIFF files yields an OOM (both with `metadata-extractor` v2.15.0 and `ExifInterface` v1.3.1),
// so we define an arbitrary threshold to avoid a crash on launch. // so we define an arbitrary threshold to avoid a crash on launch.
// It is not clear whether it is because of the file itself or its metadata. // It is not clear whether it is because of the file itself or its metadata.
private const val fileSizeBytesMax = 100 * (1 shl 20) // MB private const val FILE_SIZE_MAX = 100 * (1 shl 20) // MB
fun isDangerouslyLarge(sizeBytes: Long?) = sizeBytes == null || sizeBytes > fileSizeBytesMax fun isDangerouslyLarge(sizeBytes: Long?) = sizeBytes == null || sizeBytes > FILE_SIZE_MAX
// we try and read metadata from large files by copying an arbitrary amount from its beginning // we try and read metadata from large files by copying an arbitrary amount from its beginning
// to a temporary file, and reusing that preview file for all metadata reading purposes // to a temporary file, and reusing that preview file for all metadata reading purposes
private const val previewSize: Long = 5 * (1 shl 20) // MB private const val PREVIEW_SIZE: Long = 5 * (1 shl 20) // MB
private val previewFiles = HashMap<Uri, File>() private val previewFiles = HashMap<Uri, File>()
@ -159,7 +159,7 @@ object Metadata {
fun createPreviewFile(context: Context, uri: Uri): File { fun createPreviewFile(context: Context, uri: Uri): File {
return File.createTempFile("aves", null, context.cacheDir).apply { return File.createTempFile("aves", null, context.cacheDir).apply {
deleteOnExit() deleteOnExit()
transferFrom(StorageUtils.openInputStream(context, uri), previewSize) transferFrom(StorageUtils.openInputStream(context, uri), PREVIEW_SIZE)
} }
} }

View file

@ -45,9 +45,9 @@ object BitmapUtils {
val bufferSize = stream.size() val bufferSize = stream.size()
if (bufferSize > BUFFER_SIZE_DANGER_THRESHOLD) { if (bufferSize > BUFFER_SIZE_DANGER_THRESHOLD) {
val availHeapSize = Runtime.getRuntime().let { it.maxMemory() - (it.totalMemory() - it.freeMemory()) } val availableHeapSize = Runtime.getRuntime().let { it.maxMemory() - (it.totalMemory() - it.freeMemory()) }
if (bufferSize > availHeapSize) { if (bufferSize > availableHeapSize) {
throw Exception("compressed bitmap to $bufferSize bytes, which cannot be allocated to a new byte array, with only $availHeapSize free bytes") throw Exception("compressed bitmap to $bufferSize bytes, which cannot be allocated to a new byte array, with only $availableHeapSize free bytes")
} }
} }

View file

@ -42,6 +42,7 @@ class RegionProvider extends ImageProvider<RegionProviderKey> {
key.region, key.region,
key.imageSize, key.imageSize,
pageId: pageId, pageId: pageId,
sizeBytes: key.sizeBytes,
taskKey: key, taskKey: key,
); );
if (bytes.isEmpty) { if (bytes.isEmpty) {
@ -70,7 +71,7 @@ class RegionProviderKey extends Equatable {
// do not store the entry as it is, because the key should be constant // do not store the entry as it is, because the key should be constant
// but the entry attributes may change over time // but the entry attributes may change over time
final String uri, mimeType; final String uri, mimeType;
final int? pageId; final int? pageId, sizeBytes;
final int rotationDegrees, sampleSize; final int rotationDegrees, sampleSize;
final bool isFlipped; final bool isFlipped;
final Rectangle<int> region; final Rectangle<int> region;
@ -83,13 +84,11 @@ class RegionProviderKey extends Equatable {
required this.uri, required this.uri,
required this.mimeType, required this.mimeType,
required this.pageId, required this.pageId,
required this.sizeBytes,
required this.rotationDegrees, required this.rotationDegrees,
required this.isFlipped, required this.isFlipped,
required this.sampleSize, required this.sampleSize,
required this.region, required this.region,
required this.imageSize, required this.imageSize,
}); });
@override
String toString() => '$runtimeType#${shortHash(this)}{uri=$uri, mimeType=$mimeType, pageId=$pageId, rotationDegrees=$rotationDegrees, isFlipped=$isFlipped, sampleSize=$sampleSize, region=$region, imageSize=$imageSize}';
} }

View file

@ -52,8 +52,8 @@ class UriImage extends ImageProvider<UriImage> with EquatableMixin {
final bytes = await mediaFetchService.getImage( final bytes = await mediaFetchService.getImage(
uri, uri,
mimeType, mimeType,
rotationDegrees, rotationDegrees: rotationDegrees,
isFlipped, isFlipped: isFlipped,
pageId: pageId, pageId: pageId,
sizeBytes: sizeBytes, sizeBytes: sizeBytes,
onBytesReceived: (cumulative, total) { onBytesReceived: (cumulative, total) {
@ -76,7 +76,4 @@ class UriImage extends ImageProvider<UriImage> with EquatableMixin {
unawaited(chunkEvents.close()); unawaited(chunkEvents.close());
} }
} }
@override
String toString() => '$runtimeType#${shortHash(this)}{uri=$uri, mimeType=$mimeType, rotationDegrees=$rotationDegrees, isFlipped=$isFlipped, pageId=$pageId, scale=$scale}';
} }

View file

@ -34,6 +34,7 @@ extension ExtraAvesEntryImages on AvesEntry {
uri: uri, uri: uri,
mimeType: mimeType, mimeType: mimeType,
pageId: pageId, pageId: pageId,
sizeBytes: sizeBytes,
rotationDegrees: rotationDegrees, rotationDegrees: rotationDegrees,
isFlipped: isFlipped, isFlipped: isFlipped,
sampleSize: sampleSize, sampleSize: sampleSize,

View file

@ -17,17 +17,17 @@ abstract class MediaFetchService {
Future<Uint8List> getSvg( Future<Uint8List> getSvg(
String uri, String uri,
String mimeType, { String mimeType, {
int? expectedContentLength, required int? sizeBytes,
BytesReceivedCallback? onBytesReceived, BytesReceivedCallback? onBytesReceived,
}); });
Future<Uint8List> getImage( Future<Uint8List> getImage(
String uri, String uri,
String mimeType, String mimeType, {
int? rotationDegrees, required int? rotationDegrees,
bool isFlipped, { required bool isFlipped,
int? pageId, required int? pageId,
int? sizeBytes, required int? sizeBytes,
BytesReceivedCallback? onBytesReceived, BytesReceivedCallback? onBytesReceived,
}); });
@ -40,7 +40,8 @@ abstract class MediaFetchService {
int sampleSize, int sampleSize,
Rectangle<int> regionRect, Rectangle<int> regionRect,
Size imageSize, { Size imageSize, {
int? pageId, required int? pageId,
required int? sizeBytes,
Object? taskKey, Object? taskKey,
int? priority, int? priority,
}); });
@ -93,26 +94,27 @@ class PlatformMediaFetchService implements MediaFetchService {
Future<Uint8List> getSvg( Future<Uint8List> getSvg(
String uri, String uri,
String mimeType, { String mimeType, {
int? expectedContentLength, required int? sizeBytes,
BytesReceivedCallback? onBytesReceived, BytesReceivedCallback? onBytesReceived,
}) => }) =>
getImage( getImage(
uri, uri,
mimeType, mimeType,
0, rotationDegrees: 0,
false, isFlipped: false,
sizeBytes: expectedContentLength, pageId: null,
sizeBytes: sizeBytes,
onBytesReceived: onBytesReceived, onBytesReceived: onBytesReceived,
); );
@override @override
Future<Uint8List> getImage( Future<Uint8List> getImage(
String uri, String uri,
String mimeType, String mimeType, {
int? rotationDegrees, required int? rotationDegrees,
bool isFlipped, { required bool isFlipped,
int? pageId, required int? pageId,
int? sizeBytes, required int? sizeBytes,
BytesReceivedCallback? onBytesReceived, BytesReceivedCallback? onBytesReceived,
}) async { }) async {
try { try {
@ -166,7 +168,8 @@ class PlatformMediaFetchService implements MediaFetchService {
int sampleSize, int sampleSize,
Rectangle<int> regionRect, Rectangle<int> regionRect,
Size imageSize, { Size imageSize, {
int? pageId, required int? pageId,
required int? sizeBytes,
Object? taskKey, Object? taskKey,
int? priority, int? priority,
}) { }) {
@ -176,6 +179,7 @@ class PlatformMediaFetchService implements MediaFetchService {
final result = await _platformBytes.invokeMethod('getRegion', <String, dynamic>{ final result = await _platformBytes.invokeMethod('getRegion', <String, dynamic>{
'uri': uri, 'uri': uri,
'mimeType': mimeType, 'mimeType': mimeType,
'sizeBytes': sizeBytes,
'pageId': pageId, 'pageId': pageId,
'sampleSize': sampleSize, 'sampleSize': sampleSize,
'regionX': regionRect.left, 'regionX': regionRect.left,

View file

@ -17,7 +17,11 @@ class SvgMetadataService {
static Future<Size?> getSize(AvesEntry entry) async { static Future<Size?> getSize(AvesEntry entry) async {
try { try {
final data = await mediaFetchService.getSvg(entry.uri, entry.mimeType); final data = await mediaFetchService.getSvg(
entry.uri,
entry.mimeType,
sizeBytes: entry.sizeBytes,
);
final document = XmlDocument.parse(utf8.decode(data)); final document = XmlDocument.parse(utf8.decode(data));
final root = document.rootElement; final root = document.rootElement;
@ -63,7 +67,11 @@ class SvgMetadataService {
} }
try { try {
final data = await mediaFetchService.getSvg(entry.uri, entry.mimeType); final data = await mediaFetchService.getSvg(
entry.uri,
entry.mimeType,
sizeBytes: entry.sizeBytes,
);
final document = XmlDocument.parse(utf8.decode(data)); final document = XmlDocument.parse(utf8.decode(data));
final root = document.rootElement; final root = document.rootElement;

View file

@ -335,7 +335,14 @@ class EntryActionDelegate with FeedbackMixin, PermissionAwareMixin, SizeAwareMix
MaterialPageRoute( MaterialPageRoute(
settings: const RouteSettings(name: SourceViewerPage.routeName), settings: const RouteSettings(name: SourceViewerPage.routeName),
builder: (context) => SourceViewerPage( builder: (context) => SourceViewerPage(
loader: () => mediaFetchService.getSvg(entry.uri, entry.mimeType).then(utf8.decode), loader: () async {
final data = await mediaFetchService.getSvg(
entry.uri,
entry.mimeType,
sizeBytes: entry.sizeBytes,
);
return utf8.decode(data);
},
), ),
), ),
); );

View file

@ -74,9 +74,13 @@ class EntryPrinter with FeedbackMixin {
Future<pdf.Widget?> _buildPageImage(AvesEntry entry) async { Future<pdf.Widget?> _buildPageImage(AvesEntry entry) async {
if (entry.isSvg) { if (entry.isSvg) {
final bytes = await mediaFetchService.getSvg(entry.uri, entry.mimeType); final data = await mediaFetchService.getSvg(
if (bytes.isNotEmpty) { entry.uri,
return pdf.SvgImage(svg: utf8.decode(bytes)); entry.mimeType,
sizeBytes: entry.sizeBytes,
);
if (data.isNotEmpty) {
return pdf.SvgImage(svg: utf8.decode(data));
} }
} else { } else {
return pdf.Image(await flutterImageProvider(entry.uriImage)); return pdf.Image(await flutterImageProvider(entry.uriImage));