From c4f355690bdc048a4f53e85f3521a5ad4355e3b3 Mon Sep 17 00:00:00 2001 From: NobodyForNothing Date: Tue, 19 Nov 2024 20:05:28 +0100 Subject: [PATCH 1/8] add test for bug --- .../flutter_svg/test/loaders_test.dart | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/third_party/packages/flutter_svg/test/loaders_test.dart b/third_party/packages/flutter_svg/test/loaders_test.dart index 591b3b01e708..57d74b08e748 100644 --- a/third_party/packages/flutter_svg/test/loaders_test.dart +++ b/third_party/packages/flutter_svg/test/loaders_test.dart @@ -2,6 +2,7 @@ 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() { setUp(() { @@ -57,6 +58,24 @@ void main() { expect((await loader.prepareMessage(null))!.lengthInBytes, 0); expect((await packageLoader.prepareMessage(null))!.lengthInBytes, 1); }); + + test('SvgNetworkLoader closes internal client', () async { + final List createdClients = []; + + await http.runWithClient(() async { + const SvgNetworkLoader loader = SvgNetworkLoader(''); + + expect(createdClients, isEmpty); + await loader.prepareMessage(null); + + expect(createdClients, hasLength(1)); + expect(createdClients[0].closeCalled, true); + }, () { + final VerifyCloseClient client = VerifyCloseClient(); + createdClients.add(client); + return client; + }); + }); } class TestBundle extends Fake implements AssetBundle { @@ -96,3 +115,19 @@ class _TestColorMapper extends ColorMapper { return color; } } + +class VerifyCloseClient implements http.Client { + bool closeCalled = false; + + Future get(Uri url, {Map? headers}) async { + return http.Response('', 200); + } + + void close() { + assert(!closeCalled); + closeCalled = true; + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} \ No newline at end of file From 0299f2489c4b6e0f9d4af430fe44e7e1a0979156 Mon Sep 17 00:00:00 2001 From: NobodyForNothing Date: Tue, 19 Nov 2024 20:06:35 +0100 Subject: [PATCH 2/8] fix client not closed bug --- third_party/packages/flutter_svg/lib/src/loaders.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/third_party/packages/flutter_svg/lib/src/loaders.dart b/third_party/packages/flutter_svg/lib/src/loaders.dart index 2996cc11f64f..037867cc4e7e 100644 --- a/third_party/packages/flutter_svg/lib/src/loaders.dart +++ b/third_party/packages/flutter_svg/lib/src/loaders.dart @@ -438,7 +438,11 @@ 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; + final http.Response response = await client.get(Uri.parse(url), headers: headers); + if (_httpClient == null) { + client.close(); + } + return response.bodyBytes; } @override From 23d09a653bef06dd34bbac2e40571867e602905d Mon Sep 17 00:00:00 2001 From: NobodyForNothing Date: Tue, 19 Nov 2024 20:09:22 +0100 Subject: [PATCH 3/8] test passed client is not closed --- .../packages/flutter_svg/test/loaders_test.dart | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/third_party/packages/flutter_svg/test/loaders_test.dart b/third_party/packages/flutter_svg/test/loaders_test.dart index 57d74b08e748..90460729b223 100644 --- a/third_party/packages/flutter_svg/test/loaders_test.dart +++ b/third_party/packages/flutter_svg/test/loaders_test.dart @@ -69,13 +69,24 @@ void main() { await loader.prepareMessage(null); expect(createdClients, hasLength(1)); - expect(createdClients[0].closeCalled, true); + expect(createdClients[0].closeCalled, isTrue); }, () { final VerifyCloseClient client = VerifyCloseClient(); createdClients.add(client); return client; }); }); + + test("SvgNetworkLoader doesn't close passed client", () async { + final VerifyCloseClient client = VerifyCloseClient(); + final SvgNetworkLoader loader = SvgNetworkLoader('', + httpClient: client as http.Client, + ); + + expect(client.closeCalled, isFalse); + await loader.prepareMessage(null); + expect(client.closeCalled, isFalse); + }); } class TestBundle extends Fake implements AssetBundle { From 07aceae5ae2fc9043c41ed54d8aff63d1014cf61 Mon Sep 17 00:00:00 2001 From: NobodyForNothing Date: Tue, 19 Nov 2024 20:13:44 +0100 Subject: [PATCH 4/8] run dart format --- third_party/packages/flutter_svg/lib/src/loaders.dart | 3 ++- third_party/packages/flutter_svg/test/loaders_test.dart | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/third_party/packages/flutter_svg/lib/src/loaders.dart b/third_party/packages/flutter_svg/lib/src/loaders.dart index 037867cc4e7e..62e395982bd7 100644 --- a/third_party/packages/flutter_svg/lib/src/loaders.dart +++ b/third_party/packages/flutter_svg/lib/src/loaders.dart @@ -438,7 +438,8 @@ class SvgNetworkLoader extends SvgLoader { @override Future prepareMessage(BuildContext? context) async { final http.Client client = _httpClient ?? http.Client(); - final http.Response response = await client.get(Uri.parse(url), headers: headers); + final http.Response response = + await client.get(Uri.parse(url), headers: headers); if (_httpClient == null) { client.close(); } diff --git a/third_party/packages/flutter_svg/test/loaders_test.dart b/third_party/packages/flutter_svg/test/loaders_test.dart index 90460729b223..ebdc19035c9c 100644 --- a/third_party/packages/flutter_svg/test/loaders_test.dart +++ b/third_party/packages/flutter_svg/test/loaders_test.dart @@ -79,8 +79,9 @@ void main() { test("SvgNetworkLoader doesn't close passed client", () async { final VerifyCloseClient client = VerifyCloseClient(); - final SvgNetworkLoader loader = SvgNetworkLoader('', - httpClient: client as http.Client, + final SvgNetworkLoader loader = SvgNetworkLoader( + '', + httpClient: client as http.Client, ); expect(client.closeCalled, isFalse); @@ -141,4 +142,4 @@ class VerifyCloseClient implements http.Client { @override dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); -} \ No newline at end of file +} From 9eeaae4f8b42f0daa26094dc29b5060029812659 Mon Sep 17 00:00:00 2001 From: NobodyForNothing Date: Tue, 19 Nov 2024 20:17:49 +0100 Subject: [PATCH 5/8] update version --- third_party/packages/flutter_svg/CHANGELOG.md | 4 ++++ third_party/packages/flutter_svg/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/third_party/packages/flutter_svg/CHANGELOG.md b/third_party/packages/flutter_svg/CHANGELOG.md index 1adca859374c..5a2f4a6484a0 100644 --- a/third_party/packages/flutter_svg/CHANGELOG.md +++ b/third_party/packages/flutter_svg/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.0.15 + +* Fix `SvgNetworkLoader` not closing internally created http clients. + ## 2.0.14 * Makes the package WASM compatible. 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 84cb34ebe6f73cd741115a4124ad479d26ceaf70 Mon Sep 17 00:00:00 2001 From: NobodyForNothing Date: Tue, 19 Nov 2024 20:24:31 +0100 Subject: [PATCH 6/8] empty From 97e1d46ebdb2fd6314d7d0576957fd8011e0fd07 Mon Sep 17 00:00:00 2001 From: NobodyForNothing Date: Tue, 19 Nov 2024 22:49:43 +0100 Subject: [PATCH 7/8] apply changelog suggestion --- third_party/packages/flutter_svg/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/packages/flutter_svg/CHANGELOG.md b/third_party/packages/flutter_svg/CHANGELOG.md index 5a2f4a6484a0..7f8898396500 100644 --- a/third_party/packages/flutter_svg/CHANGELOG.md +++ b/third_party/packages/flutter_svg/CHANGELOG.md @@ -1,6 +1,6 @@ ## 2.0.15 -* Fix `SvgNetworkLoader` not closing internally created http clients. +* Fixes `SvgNetworkLoader` not closing internally created http clients. ## 2.0.14 From 15dc2ee9026fee9a512edf6702054088c3bb5ab5 Mon Sep 17 00:00:00 2001 From: NobodyForNothing Date: Tue, 19 Nov 2024 22:51:37 +0100 Subject: [PATCH 8/8] use Fake and fix lints --- third_party/packages/flutter_svg/test/loaders_test.dart | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/third_party/packages/flutter_svg/test/loaders_test.dart b/third_party/packages/flutter_svg/test/loaders_test.dart index ebdc19035c9c..28f532512633 100644 --- a/third_party/packages/flutter_svg/test/loaders_test.dart +++ b/third_party/packages/flutter_svg/test/loaders_test.dart @@ -128,18 +128,17 @@ class _TestColorMapper extends ColorMapper { } } -class VerifyCloseClient implements http.Client { +class VerifyCloseClient extends Fake implements http.Client { bool closeCalled = false; + @override Future get(Uri url, {Map? headers}) async { return http.Response('', 200); } + @override void close() { assert(!closeCalled); closeCalled = true; } - - @override - dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); }