From 20331195daf4673832420726c7085d800a1641aa Mon Sep 17 00:00:00 2001 From: Navaron Bracke Date: Tue, 26 Nov 2024 22:02:06 +0100 Subject: [PATCH] [flutter_markdown] fix invalid URI's causing unhandled image errors (#8058) This PR adds an error builder for images, so that any errors from those are caught. *List which issues are fixed by this PR. You must list at least one issue.* Fixes https://github.com/flutter/flutter/issues/158428 --- packages/flutter_markdown/CHANGELOG.md | 6 +- .../lib/src/_functions_io.dart | 48 +++++++++++++-- .../lib/src/_functions_web.dart | 59 ++++++++++++++++--- .../flutter_markdown/lib/src/builder.dart | 7 ++- packages/flutter_markdown/pubspec.yaml | 2 +- .../flutter_markdown/test/image_test.dart | 40 +++++++++++++ 6 files changed, 148 insertions(+), 14 deletions(-) diff --git a/packages/flutter_markdown/CHANGELOG.md b/packages/flutter_markdown/CHANGELOG.md index 40c40d6c871f..0c43f4fa88c9 100644 --- a/packages/flutter_markdown/CHANGELOG.md +++ b/packages/flutter_markdown/CHANGELOG.md @@ -1,6 +1,10 @@ +## 0.7.4+3 + +* Passes a default error builder to image widgets. + ## 0.7.4+2 -* Fixes pub.dev detection of WebAssembly support. +* Fixes pub.dev detection of WebAssembly support. ## 0.7.4+1 diff --git a/packages/flutter_markdown/lib/src/_functions_io.dart b/packages/flutter_markdown/lib/src/_functions_io.dart index b3020573e165..965d6175d24d 100644 --- a/packages/flutter_markdown/lib/src/_functions_io.dart +++ b/packages/flutter_markdown/lib/src/_functions_io.dart @@ -24,23 +24,62 @@ final ImageBuilder kDefaultImageBuilder = ( double? height, ) { if (uri.scheme == 'http' || uri.scheme == 'https') { - return Image.network(uri.toString(), width: width, height: height); + return Image.network( + uri.toString(), + width: width, + height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, + ); } else if (uri.scheme == 'data') { return _handleDataSchemeUri(uri, width, height); } else if (uri.scheme == 'resource') { - return Image.asset(uri.path, width: width, height: height); + return Image.asset( + uri.path, + width: width, + height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, + ); } else { final Uri fileUri = imageDirectory != null ? Uri.parse(imageDirectory + uri.toString()) : uri; if (fileUri.scheme == 'http' || fileUri.scheme == 'https') { - return Image.network(fileUri.toString(), width: width, height: height); + return Image.network( + fileUri.toString(), + width: width, + height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, + ); } else { - return Image.file(File.fromUri(fileUri), width: width, height: height); + try { + return Image.file( + File.fromUri(fileUri), + width: width, + height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, + ); + } catch (error, stackTrace) { + // Handle any invalid file URI's. + return Builder( + builder: (BuildContext context) { + return kDefaultImageErrorWidgetBuilder(context, error, stackTrace); + }, + ); + } } } }; +/// A default error widget builder for handling image errors. +// ignore: prefer_function_declarations_over_variables +final ImageErrorWidgetBuilder kDefaultImageErrorWidgetBuilder = ( + BuildContext context, + Object error, + StackTrace? stackTrace, +) { + return const SizedBox(); +}; + /// A default style sheet generator. final MarkdownStyleSheet Function(BuildContext, MarkdownStyleSheetBaseTheme?) // ignore: prefer_function_declarations_over_variables @@ -76,6 +115,7 @@ Widget _handleDataSchemeUri( uri.data!.contentAsBytes(), width: width, height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, ); } else if (mimeType.startsWith('text/')) { return Text(uri.data!.contentAsString()); diff --git a/packages/flutter_markdown/lib/src/_functions_web.dart b/packages/flutter_markdown/lib/src/_functions_web.dart index abf23453225c..d1aade078e89 100644 --- a/packages/flutter_markdown/lib/src/_functions_web.dart +++ b/packages/flutter_markdown/lib/src/_functions_web.dart @@ -25,24 +25,68 @@ final ImageBuilder kDefaultImageBuilder = ( double? height, ) { if (uri.scheme == 'http' || uri.scheme == 'https') { - return Image.network(uri.toString(), width: width, height: height); + return Image.network( + uri.toString(), + width: width, + height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, + ); } else if (uri.scheme == 'data') { return _handleDataSchemeUri(uri, width, height); } else if (uri.scheme == 'resource') { - return Image.asset(uri.path, width: width, height: height); + return Image.asset( + uri.path, + width: width, + height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, + ); } else { - final Uri fileUri = imageDirectory != null - ? Uri.parse(p.join(imageDirectory, uri.toString())) - : uri; + final Uri fileUri; + + if (imageDirectory != null) { + try { + fileUri = Uri.parse(p.join(imageDirectory, uri.toString())); + } catch (error, stackTrace) { + // Handle any invalid file URI's. + return Builder( + builder: (BuildContext context) { + return kDefaultImageErrorWidgetBuilder(context, error, stackTrace); + }, + ); + } + } else { + fileUri = uri; + } + if (fileUri.scheme == 'http' || fileUri.scheme == 'https') { - return Image.network(fileUri.toString(), width: width, height: height); + return Image.network( + fileUri.toString(), + width: width, + height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, + ); } else { final String src = p.join(p.current, fileUri.toString()); - return Image.network(src, width: width, height: height); + return Image.network( + src, + width: width, + height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, + ); } } }; +/// A default error widget builder for handling image errors. +// ignore: prefer_function_declarations_over_variables +final ImageErrorWidgetBuilder kDefaultImageErrorWidgetBuilder = ( + BuildContext context, + Object error, + StackTrace? stackTrace, +) { + return const SizedBox(); +}; + /// A default style sheet generator. final MarkdownStyleSheet Function(BuildContext, MarkdownStyleSheetBaseTheme?) // ignore: prefer_function_declarations_over_variables @@ -72,6 +116,7 @@ Widget _handleDataSchemeUri( uri.data!.contentAsBytes(), width: width, height: height, + errorBuilder: kDefaultImageErrorWidgetBuilder, ); } else if (mimeType.startsWith('text/')) { return Text(uri.data!.contentAsString()); diff --git a/packages/flutter_markdown/lib/src/builder.dart b/packages/flutter_markdown/lib/src/builder.dart index ea9af16acfbe..50bdd11591f8 100644 --- a/packages/flutter_markdown/lib/src/builder.dart +++ b/packages/flutter_markdown/lib/src/builder.dart @@ -601,7 +601,12 @@ class MarkdownBuilder implements md.NodeVisitor { } } - final Uri uri = Uri.parse(path); + final Uri? uri = Uri.tryParse(path); + + if (uri == null) { + return const SizedBox(); + } + Widget child; if (imageBuilder != null) { child = imageBuilder!(uri, title, alt); diff --git a/packages/flutter_markdown/pubspec.yaml b/packages/flutter_markdown/pubspec.yaml index b15a738f4ce3..b78c0dce3ddf 100644 --- a/packages/flutter_markdown/pubspec.yaml +++ b/packages/flutter_markdown/pubspec.yaml @@ -4,7 +4,7 @@ description: A Markdown renderer for Flutter. Create rich text output, formatted with simple Markdown tags. repository: https://github.com/flutter/packages/tree/main/packages/flutter_markdown issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+flutter_markdown%22 -version: 0.7.4+2 +version: 0.7.4+3 environment: sdk: ^3.3.0 diff --git a/packages/flutter_markdown/test/image_test.dart b/packages/flutter_markdown/test/image_test.dart index 00677f1b7284..07a4e87c9058 100644 --- a/packages/flutter_markdown/test/image_test.dart +++ b/packages/flutter_markdown/test/image_test.dart @@ -333,6 +333,46 @@ void defineTests() { }, ); + testWidgets( + 'should gracefully handle image URLs with empty scheme', + (WidgetTester tester) async { + const String data = '![alt](://img#x50)'; + await tester.pumpWidget( + boilerplate( + const Markdown(data: data), + ), + ); + + expect(find.byType(Image), findsNothing); + expect(tester.takeException(), isNull); + }, + ); + + testWidgets( + 'should gracefully handle image URLs with invalid scheme', + (WidgetTester tester) async { + const String data = '![alt](ttps://img#x50)'; + await tester.pumpWidget( + boilerplate( + const Markdown(data: data), + ), + ); + + // On the web, any URI with an unrecognized scheme is treated as a network image. + // Thus the error builder of the Image widget is called. + // On non-web, any URI with an unrecognized scheme is treated as a file image. + // However, constructing a file from an invalid URI will throw an exception. + // Thus the Image widget is never created, nor is its error builder called. + if (kIsWeb) { + expect(find.byType(Image), findsOneWidget); + } else { + expect(find.byType(Image), findsNothing); + } + + expect(tester.takeException(), isNull); + }, + ); + testWidgets( 'should gracefully handle width parsing failures', (WidgetTester tester) async {