From 47635029d36c91c022b7f0279305b74b385e1f40 Mon Sep 17 00:00:00 2001 From: Hung Luong Do Minh Date: Mon, 4 Nov 2024 22:19:22 +0700 Subject: [PATCH 1/4] feat: Error handling enhancement from flutter_svg - copy from this PR: https://github.com/dnfield/flutter_svg/pull/1104 --- .../flutter_svg/example/assets/invalid.svg | 1 + .../flutter_svg/example/lib/grid.dart | 158 ++++++++++++------ .../packages/flutter_svg/lib/src/loaders.dart | 66 +++++--- third_party/packages/flutter_svg/lib/svg.dart | 18 ++ .../test/svg_picture_error_test.dart | 152 +++++++++++++++++ 5 files changed, 316 insertions(+), 79 deletions(-) create mode 100644 third_party/packages/flutter_svg/example/assets/invalid.svg create mode 100644 third_party/packages/flutter_svg/test/svg_picture_error_test.dart diff --git a/third_party/packages/flutter_svg/example/assets/invalid.svg b/third_party/packages/flutter_svg/example/assets/invalid.svg new file mode 100644 index 000000000000..20bd54528ca9 --- /dev/null +++ b/third_party/packages/flutter_svg/example/assets/invalid.svg @@ -0,0 +1 @@ +< width="69" height="69" viewBox="0 0 69 69"> diff --git a/third_party/packages/flutter_svg/example/lib/grid.dart b/third_party/packages/flutter_svg/example/lib/grid.dart index 163893f50560..77ce2d17b0e9 100644 --- a/third_party/packages/flutter_svg/example/lib/grid.dart +++ b/third_party/packages/flutter_svg/example/lib/grid.dart @@ -1,8 +1,11 @@ +import 'dart:math'; + import 'package:flutter/material.dart'; import 'package:flutter_svg/flutter_svg.dart'; const List _assetNames = [ - // 'assets/notfound.svg', // uncomment to test an asset that doesn't exist. + 'assets/invalid.svg', + 'assets/notfound.svg', // uncomment to test an asset that doesn't exist. 'assets/flutter_logo.svg', 'assets/dart.svg', 'assets/simple/clip_path_3.svg', @@ -35,7 +38,7 @@ const List _assetNames = [ ]; /// Assets treated as "icons" - using a color filter to render differently. -const List iconNames = [ +const List _iconNames = [ 'assets/deborah_ufw/new-action-expander.svg', 'assets/deborah_ufw/new-camera.svg', 'assets/deborah_ufw/new-gif-button.svg', @@ -49,12 +52,27 @@ const List iconNames = [ ]; /// Assets to test network access. -const List uriNames = [ +const List _uriNames = [ 'http://upload.wikimedia.org/wikipedia/commons/0/02/SVG_logo.svg', 'https://dev.w3.org/SVG/tools/svgweb/samples/svg-files/410.svg', 'https://upload.wikimedia.org/wikipedia/commons/b/b4/Chess_ndd45.svg', ]; +const List _uriFailedNames = [ + 'an error image url.svg', // invalid url. + 'https: /sadf.svg', // invalid url. + 'http://www.google.com/404', // 404 url. + 'https://picsum.photos/200', // wrong format image url. +]; + +const List _stringNames = [ + ''' ''', // Shows an example of an SVG image that will fetch a raster image from a URL. + ''' ''', // valid svg + '''''', // empty svg. + 'sdf sdf ', // invalid svg. + '', // empty string. +]; + void main() { runApp(_MyApp()); } @@ -81,59 +99,10 @@ class _MyHomePage extends StatefulWidget { } class _MyHomePageState extends State<_MyHomePage> { - final List _painters = []; - late double _dimension; - - @override - void initState() { - super.initState(); - _dimension = 203.0; - for (final String assetName in _assetNames) { - _painters.add( - SvgPicture.asset(assetName), - ); - } - - for (int i = 0; i < iconNames.length; i++) { - _painters.add( - Directionality( - textDirection: TextDirection.ltr, - child: SvgPicture.asset( - iconNames[i], - colorFilter: ColorFilter.mode( - Colors.blueGrey[(i + 1) * 100] ?? Colors.blueGrey, - BlendMode.srcIn, - ), - matchTextDirection: true, - ), - ), - ); - } - - for (final String uriName in uriNames) { - _painters.add( - SvgPicture.network( - uriName, - placeholderBuilder: (BuildContext context) => Container( - padding: const EdgeInsets.all(30.0), - child: const CircularProgressIndicator(), - ), - ), - ); - } - // Shows an example of an SVG image that will fetch a raster image from a URL. - _painters.add(SvgPicture.string(''' - - -''')); - } + double _dimension = 60; @override Widget build(BuildContext context) { - if (_dimension > MediaQuery.of(context).size.width - 10.0) { - _dimension = MediaQuery.of(context).size.width - 10.0; - } return Scaffold( appBar: AppBar( title: Text(widget.title), @@ -144,7 +113,7 @@ class _MyHomePageState extends State<_MyHomePage> { max: MediaQuery.of(context).size.width - 10.0, value: _dimension, onChanged: (double val) { - setState(() => _dimension = val); + setState(() => _dimension = min(MediaQuery.of(context).size.width - 10.0, val)); }, ), Expanded( @@ -154,7 +123,86 @@ class _MyHomePageState extends State<_MyHomePage> { padding: const EdgeInsets.all(4.0), mainAxisSpacing: 4.0, crossAxisSpacing: 4.0, - children: _painters.toList(), + children: [ + ..._assetNames.map( + (String e) => SvgPicture.asset( + e, + placeholderBuilder: (BuildContext context) => Container( + padding: const EdgeInsets.all(30.0), + child: const CircularProgressIndicator(), + ), + errorBuilder: (BuildContext context, Object error, StackTrace stackTrace) => Container( + color: Colors.brown, + width: 10, + height: 10, + ), + ), + ), + ..._iconNames.map( + (String e) => Directionality( + textDirection: TextDirection.ltr, + child: SvgPicture.asset( + e, + colorFilter: ColorFilter.mode( + Colors.blueGrey[(_iconNames.indexOf(e) + 1) * 100] ?? Colors.blueGrey, + BlendMode.srcIn, + ), + matchTextDirection: true, + placeholderBuilder: (BuildContext context) => Container( + padding: const EdgeInsets.all(30.0), + child: const CircularProgressIndicator(), + ), + errorBuilder: (BuildContext context, Object error, StackTrace stackTrace) => Container( + color: Colors.yellow, + width: 10, + height: 10, + ), + ), + ), + ), + ..._uriNames.map( + (String e) => SvgPicture.network( + e, + placeholderBuilder: (BuildContext context) => Container( + padding: const EdgeInsets.all(30.0), + child: const CircularProgressIndicator(), + ), + errorBuilder: (BuildContext context, Object error, StackTrace stackTrace) => Container( + color: Colors.red, + width: 10, + height: 10, + ), + ), + ), + ..._uriFailedNames.map( + (String e) => SvgPicture.network( + e, + placeholderBuilder: (BuildContext context) => Container( + padding: const EdgeInsets.all(30.0), + child: const CircularProgressIndicator(), + ), + errorBuilder: (BuildContext context, Object error, StackTrace stackTrace) => Container( + color: Colors.deepPurple, + width: 10, + height: 10, + ), + ), + ), + ..._stringNames.map( + (String e) => SvgPicture.string( + e, + placeholderBuilder: (BuildContext context) => Container( + padding: const EdgeInsets.all(30.0), + child: const CircularProgressIndicator(), + ), + errorBuilder: (BuildContext context, Object error, StackTrace stackTrace) => Container( + color: Colors.pinkAccent, + width: 10, + height: 10, + ), + ), + ), + ], ), ), ]), diff --git a/third_party/packages/flutter_svg/lib/src/loaders.dart b/third_party/packages/flutter_svg/lib/src/loaders.dart index 2996cc11f64f..06edbce75a50 100644 --- a/third_party/packages/flutter_svg/lib/src/loaders.dart +++ b/third_party/packages/flutter_svg/lib/src/loaders.dart @@ -152,20 +152,29 @@ abstract class SvgLoader extends BytesLoader { final SvgTheme theme = getTheme(context); return prepareMessage(context).then((T? message) { return compute((T? message) { - return vg - .encodeSvg( - xml: provideSvg(message), - theme: theme.toVgTheme(), - colorMapper: colorMapper == null - ? null - : _DelegateVgColorMapper(colorMapper!), - debugName: 'Svg loader', - enableClippingOptimizer: false, - enableMaskingOptimizer: false, - enableOverdrawOptimizer: false, - ) - .buffer - .asByteData(); + try { + debugPrint('SvgLoader._load.provideSvg: empty'); + final String xml = provideSvg(message); + if (xml.isEmpty) { + return Future.value(ByteData(0)); + } else { + return vg + .encodeSvg( + xml: xml, + theme: theme.toVgTheme(), + colorMapper: colorMapper == null ? null : _DelegateVgColorMapper(colorMapper!), + debugName: 'Svg loader', + enableClippingOptimizer: false, + enableMaskingOptimizer: false, + enableOverdrawOptimizer: false, + ) + .buffer + .asByteData(); + } + } catch (e) { + debugPrint('SvgLoader._load.error: $e'); + return Future.value(ByteData(0)); + } }, message, debugLabel: 'Load Bytes'); }); } @@ -373,15 +382,19 @@ class SvgAssetLoader extends SvgLoader { } @override - Future prepareMessage(BuildContext? context) { - return _resolveBundle(context).load( - packageName == null ? assetName : 'packages/$packageName/$assetName', - ); + Future prepareMessage(BuildContext? context) async { + try { + return await _resolveBundle(context).load( + packageName == null ? assetName : 'packages/$packageName/$assetName', + ); + } catch (e) { + debugPrint('SvgAssetLoader.prepareMessage.error: $e'); + return Future.value(); + } } @override - String provideSvg(ByteData? message) => - utf8.decode(message!.buffer.asUint8List(), allowMalformed: true); + String provideSvg(ByteData? message) => utf8.decode(message!.buffer.asUint8List(), allowMalformed: true); @override SvgCacheKey cacheKey(BuildContext? context) { @@ -437,13 +450,18 @@ class SvgNetworkLoader extends SvgLoader { @override Future prepareMessage(BuildContext? context) async { - final http.Client client = _httpClient ?? http.Client(); - return (await client.get(Uri.parse(url), headers: headers)).bodyBytes; + try { + final http.Client client = _httpClient ?? http.Client(); + final http.Response res = await client.get(Uri.parse(url), headers: headers); + return res.bodyBytes; + } catch (e) { + debugPrint('SvgNetworkLoader.prepareMessage.error: $e'); + return null; + } } @override - String provideSvg(Uint8List? message) => - utf8.decode(message!, allowMalformed: true); + String provideSvg(Uint8List? message) => message == null ? '' : utf8.decode(message, allowMalformed: true); @override int get hashCode => Object.hash(url, headers, theme, colorMapper); diff --git a/third_party/packages/flutter_svg/lib/svg.dart b/third_party/packages/flutter_svg/lib/svg.dart index 0efcbdaa1eef..1b05a3625ec0 100644 --- a/third_party/packages/flutter_svg/lib/svg.dart +++ b/third_party/packages/flutter_svg/lib/svg.dart @@ -86,6 +86,7 @@ class SvgPicture extends StatelessWidget { this.semanticsLabel, this.excludeFromSemantics = false, this.clipBehavior = Clip.hardEdge, + this.errorBuilder, @Deprecated( 'No code should use this parameter. It never was implemented properly. ' 'The SVG theme must be set on the bytesLoader.') @@ -93,6 +94,7 @@ class SvgPicture extends StatelessWidget { @Deprecated('This no longer does anything.') bool cacheColorFilter = false, }); + /// Instantiates a widget that renders an SVG picture from an [AssetBundle]. /// /// The key will be derived from the `assetName`, `package`, and `bundle` @@ -190,6 +192,7 @@ class SvgPicture extends StatelessWidget { @Deprecated('Use colorFilter instead.') ui.BlendMode colorBlendMode = ui.BlendMode.srcIn, @Deprecated('This no longer does anything.') bool cacheColorFilter = false, + this.errorBuilder, }) : bytesLoader = SvgAssetLoader( assetName, packageName: package, @@ -251,6 +254,7 @@ class SvgPicture extends StatelessWidget { @Deprecated('This no longer does anything.') bool cacheColorFilter = false, SvgTheme? theme, http.Client? httpClient, + this.errorBuilder, }) : bytesLoader = SvgNetworkLoader( url, headers: headers, @@ -308,6 +312,7 @@ class SvgPicture extends StatelessWidget { this.clipBehavior = Clip.hardEdge, SvgTheme? theme, @Deprecated('This no longer does anything.') bool cacheColorFilter = false, + this.errorBuilder, }) : bytesLoader = SvgFileLoader(file, theme: theme), colorFilter = colorFilter ?? _getColorFilter(color, colorBlendMode); @@ -357,6 +362,7 @@ class SvgPicture extends StatelessWidget { this.clipBehavior = Clip.hardEdge, SvgTheme? theme, @Deprecated('This no longer does anything.') bool cacheColorFilter = false, + this.errorBuilder, }) : bytesLoader = SvgBytesLoader(bytes, theme: theme), colorFilter = colorFilter ?? _getColorFilter(color, colorBlendMode); @@ -406,6 +412,7 @@ class SvgPicture extends StatelessWidget { this.clipBehavior = Clip.hardEdge, SvgTheme? theme, @Deprecated('This no longer does anything.') bool cacheColorFilter = false, + this.errorBuilder, }) : bytesLoader = SvgStringLoader(string, theme: theme), colorFilter = colorFilter ?? _getColorFilter(color, colorBlendMode); @@ -490,6 +497,9 @@ class SvgPicture extends StatelessWidget { /// The color filter, if any, to apply to this widget. final ColorFilter? colorFilter; + /// The widget to show when failed to fetch, decode, and parse the SVG data. + final SvgPictureErrorWidgetBuilder? errorBuilder; + @override Widget build(BuildContext context) { return createCompatVectorGraphic( @@ -505,6 +515,7 @@ class SvgPicture extends StatelessWidget { placeholderBuilder: placeholderBuilder, clipViewbox: !allowDrawingOutsideViewBox, matchTextDirection: matchTextDirection, + errorBuilder: errorBuilder, ); } @@ -567,3 +578,10 @@ class SvgPicture extends StatelessWidget { )); } } + +/// The signature that [VectorGraphic.errorBuilder] uses to report exceptions. +typedef SvgPictureErrorWidgetBuilder = Widget Function( + BuildContext context, + Object error, + StackTrace stackTrace, +); diff --git a/third_party/packages/flutter_svg/test/svg_picture_error_test.dart b/third_party/packages/flutter_svg/test/svg_picture_error_test.dart new file mode 100644 index 000000000000..2034b9ff56f4 --- /dev/null +++ b/third_party/packages/flutter_svg/test/svg_picture_error_test.dart @@ -0,0 +1,152 @@ +import 'dart:io'; + +import 'package:flutter/foundation.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_svg/flutter_svg.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; + +void main() { + group('SvgPicture.string - use placeHolder or errorWidget if an error causes', () { + setUp(() {}); + tearDown(() {}); + testWidgets('load an empty string', (WidgetTester tester) async { + await tester.pumpWidget(SvgPicture.string('')); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + }); + testWidgets('show placeholder', (WidgetTester tester) async { + await tester.pumpWidget(SvgPicture.string( + 'an invalid svg format string', + placeholderBuilder: buildPlaceHolderWidget, + )); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + await tester.pumpAndSettle(); + expect(find.text('placeholder'), findsOneWidget); + }); + testWidgets('show errorWidget', (WidgetTester tester) async { + await tester.pumpWidget(SvgPicture.string( + 'an invalid svg format string', + placeholderBuilder: buildPlaceHolderWidget, + errorBuilder: buildErrorWidget, + )); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + await tester.pumpAndSettle(); + expectOneErrorWidget(tester); + }); + }); + + group('SvgPicture.asset - use placeHolder or errorWidget if an error causes', () { + late FakeAssetBundle assetBundle; + setUp(() { + assetBundle = FakeAssetBundle(); + }); + tearDown(() {}); + testWidgets('load an empty asset', (WidgetTester tester) async { + await tester.pumpWidget(DefaultAssetBundle( + bundle: assetBundle, + child: SvgPicture.asset( + 'empty', + ))); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + }); + testWidgets('show placeholder', (WidgetTester tester) async { + await tester.pumpWidget(DefaultAssetBundle( + bundle: assetBundle, + child: SvgPicture.asset( + 'an invalid asset', + placeholderBuilder: buildPlaceHolderWidget, + ))); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + await tester.pumpAndSettle(); + expect(find.text('placeholder'), findsOneWidget); + }); + testWidgets('show errorWidget', (WidgetTester tester) async { + await tester.pumpWidget(DefaultAssetBundle( + bundle: assetBundle, + child: SvgPicture.asset( + 'an invalid asset', + placeholderBuilder: buildPlaceHolderWidget, + errorBuilder: buildErrorWidget, + ))); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + await tester.pumpAndSettle(); + expectOneErrorWidget(tester); + }); + }); + + group('SvgPicture.network - use placeHolder or errorWidget if an error causes', () { + late FakeHttpClient httpClient = FakeHttpClient(); + setUp(() { + httpClient = FakeHttpClient(); + }); + tearDown(() {}); + testWidgets('http exception', (WidgetTester tester) async { + await tester.pumpWidget(SvgPicture.network('/404', httpClient: httpClient)); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + }); + testWidgets('wrong svg format - show placeholder', (WidgetTester tester) async { + await tester.pumpWidget(SvgPicture.network( + '/200', + placeholderBuilder: buildPlaceHolderWidget, + httpClient: httpClient, + )); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + await tester.pumpAndSettle(); + expect(find.text('placeholder'), findsOneWidget); + }); + testWidgets('show placeholder - show errorWidget', (WidgetTester tester) async { + await tester.pumpWidget(SvgPicture.network( + '/200', + placeholderBuilder: buildPlaceHolderWidget, + errorBuilder: buildErrorWidget, + httpClient: httpClient, + )); + await tester.pumpAndSettle(); + expectOneSvgPicture(tester); + await tester.pumpAndSettle(); + expectOneErrorWidget(tester); + }); + }); +} + +void expectOneSvgPicture(WidgetTester tester) => expect(find.byType(SvgPicture), findsOneWidget); +void expectOneErrorWidget(WidgetTester tester) => expect(find.text('error'), findsOneWidget); + +Widget buildPlaceHolderWidget(BuildContext context) => const Text('placeholder', textDirection: TextDirection.ltr); +Widget buildErrorWidget(BuildContext context, Object error, StackTrace stackTrace) => + const Text('error', textDirection: TextDirection.ltr); + +class FakeAssetBundle extends Fake implements AssetBundle { + @override + Future load(String key) async { + if (key == 'empty') { + return Future.value(ByteData(0)); + } + throw Exception('error'); + } +} + +class FakeHttpClient extends Fake implements http.Client { + FakeHttpClient(); + + @override + Future get(Uri url, {Map? headers}) async { + debugPrint('FakeHttpClient.get: ${url.path}'); + if (url.path == '/404') { + return Future.value(http.Response('', HttpStatus.notFound)); + } else if (url.path == '/200') { + return Future.value(http.Response('''invalid svg format''', HttpStatus.ok)); + } else { + throw Exception('$url is invalid'); + } + } +} From e49a91f717c89e775b976e0fffb776e91f9fa379 Mon Sep 17 00:00:00 2001 From: Hung Luong Do Minh Date: Tue, 5 Nov 2024 01:03:47 +0700 Subject: [PATCH 2/4] fix: changelog # Conflicts: # third_party/packages/flutter_svg/CHANGELOG.md --- third_party/packages/flutter_svg/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/third_party/packages/flutter_svg/CHANGELOG.md b/third_party/packages/flutter_svg/CHANGELOG.md index 1adca859374c..ab0a58905cf6 100644 --- a/third_party/packages/flutter_svg/CHANGELOG.md +++ b/third_party/packages/flutter_svg/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.0.15 +* Adds error handling when parsing an invalid svg string. +* Adds error handling when downloading svg string from network. +* Adds error handling when reading svg file from asset. +* Expose ErrorWidgetBuilder. + ## 2.0.14 * Makes the package WASM compatible. From ec6ca0487ea4dcdec23751a52e72e8985e7affb3 Mon Sep 17 00:00:00 2001 From: Hung Luong Do Minh Date: Tue, 12 Nov 2024 22:16:13 +0700 Subject: [PATCH 3/4] chore: increase version --- third_party/packages/flutter_svg/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/packages/flutter_svg/pubspec.yaml b/third_party/packages/flutter_svg/pubspec.yaml index e18e014e9596..f08db3a5ad64 100644 --- a/third_party/packages/flutter_svg/pubspec.yaml +++ b/third_party/packages/flutter_svg/pubspec.yaml @@ -2,7 +2,7 @@ name: flutter_svg description: An SVG rendering and widget library for Flutter, which allows painting and displaying Scalable Vector Graphics 1.1 files. repository: https://github.com/flutter/packages/tree/main/third_party/packages/flutter_svg issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+flutter_svg%22 -version: 2.0.14 +version: 2.0.15 environment: sdk: ^3.4.0 From 5f89e4b4aa6ed8e10669559023e2dff75dd22c60 Mon Sep 17 00:00:00 2001 From: Hung Luong Do Minh Date: Tue, 12 Nov 2024 22:16:51 +0700 Subject: [PATCH 4/4] feat: use vector_graphics: ^1.1.15 --- third_party/packages/flutter_svg/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/packages/flutter_svg/pubspec.yaml b/third_party/packages/flutter_svg/pubspec.yaml index f08db3a5ad64..5ce87f7c621a 100644 --- a/third_party/packages/flutter_svg/pubspec.yaml +++ b/third_party/packages/flutter_svg/pubspec.yaml @@ -12,7 +12,7 @@ dependencies: flutter: sdk: flutter http: ^1.0.0 - vector_graphics: ^1.1.13 + vector_graphics: ^1.1.15 vector_graphics_codec: ^1.1.11+1 vector_graphics_compiler: ^1.1.14