[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 flutter/flutter#158428
flutter · Nov 26, 2024 · 2033119 · 2033119
1 parent e9a6e24
commit 2033119
Show file tree

Hide file tree

Showing 6 changed files with 148 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {