[flutter_svg] Throw on non-2xx responses in SvgNetworkLoader by francisrath · Pull Request #13403 · flutter/packages

francisrath · 2026-10-23T15:08:08Z

SvgNetworkLoader.prepareMessage returned response.bodyBytes without checking the status code, so a 404 or 503 HTML error page was handed to the SVG parser and failed with a misleading StateError: Bad state: Invalid SVG data. This PR makes it throw an http.ClientException naming the status code and URL for any status outside 200–299, so the real failure is visible.

It also moves closing an internally created client into a finally, so that client is closed on the error path too (and if get itself throws). Tests cover a 404 (throws, client still closed) and a 200 (returns the body).

Fixes flutter/flutter#195162

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

google-cla · 2026-10-23T15:08:29Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

SvgNetworkLoader.prepareMessage returned the response body regardless of status, so an HTML 404/503 page reached the SVG parser and failed with a misleading "Invalid SVG data". It now throws an http.ClientException naming the status code and URL, and closes an internally created client in a finally so the error path closes it too.

gemini-code-assist

Code Review

This pull request updates the SvgNetworkLoader to throw an http.ClientException when receiving non-2xx HTTP responses, ensuring that only valid SVG content is processed. It also includes comprehensive test cases to verify this behavior and ensures that the HTTP client is properly closed. A review comment identified a potential resource leak if Uri.parse fails before the try block, suggesting that URI parsing should be moved inside the try block to ensure the client is always closed.

gemini-code-assist · 2026-10-23T15:09:56Z

    final http.Client client = _httpClient ?? http.Client();
-    final http.Response response = await client.get(Uri.parse(url), headers: headers);
-    if (_httpClient == null) {
-      client.close();
+    final Uri uri = Uri.parse(url);
+    try {
+      final http.Response response = await client.get(uri, headers: headers);


If Uri.parse(url) throws a FormatException (for example, if the URL is malformed), the try block is never entered. Consequently, the finally block is not executed, and the internally created http.Client is leaked. Moving the URI parsing inside the try block ensures that the client is always closed properly.

Suggested change

final http.Client client = _httpClient ?? http.Client();

final http.Response response = await client.get(Uri.parse(url), headers: headers);

if (_httpClient == null) {

client.close();

final Uri uri = Uri.parse(url);

try {

final http.Response response = await client.get(uri, headers: headers);

final http.Client client = _httpClient ?? http.Client();

try {

final Uri uri = Uri.parse(url);

final http.Response response = await client.get(uri, headers: headers);

Thanks — fixed by parsing the URL before creating the client, so a malformed URL throws before there's a client to close.

…oader A malformed URL now throws before an internally created client exists, so it cannot leak.

andywolff

LGTM, thanks for the fix

auto-submit · 2026-10-27T20:28:11Z

autosubmit label was removed for flutter/packages/13403, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

jtmcdole · 2026-10-27T20:33:42Z

    expect(client.closeCalled, isFalse);
  });
+
+  test('SvgNetworkLoader throws on a non-2xx response and still closes its client', () async {


No test for an invalid URI?

francisrath · 2026-10-27T20:48:52Z

Thanks @jtmcdole, good catch. Added a test that a malformed URL throws a FormatException and that no client is created (pushed).

jtmcdole

💵 The CodeFu 1024-Note: Collector's Edition

The LGTM exchange rates are currently trading at:

1 LGTM = Golden images match closely enough if I squint.
2 LGTMs = I checked that this won't break the devicelab benchmarks.
3 LGTMs = I trust that this C++ pointer won't leak memory every frame.
4 LGTMs = CI is green on the first try without a single retry or shard failure.

💵 Value officially backed by the full faith, credit, and compute budget of the Flutter EngProd Infrastructure.

jtmcdole · 2026-10-27T21:15:57Z

You'll need to edit your commit history to remove the bot:

❌ 8b86412 Co-Author: <no****y@anthropic.com>

…ated

francisrath · 2026-10-27T21:42:49Z

Thanks, removed the co-author trailer: the two follow-up commits are squashed into one with no trailer. It's ready for the autosubmit label when you are.

flutter/packages@13d38e0...0b72e92 2026-10-29 [email protected] [material_ui] Mirror the GappedSliderTrackShape secondary track correctly in RTL (flutter/packages#13226) 2026-10-29 [email protected] [pigeon] update jni dependency to ^1.1.1 and remove workaround proguard rules (flutter/packages#13464) 2026-10-28 [email protected] [cupertino_ui] Document translucent default background of CupertinoSlidingSegmentedControl (flutter/packages#13178) 2026-10-28 [email protected] [flutter_svg] Throw on non-2xx responses in SvgNetworkLoader (flutter/packages#13403) 2026-10-28 [email protected] [tool] Fix package matching logic for `--package-tags` filter (flutter/packages#13472) 2026-10-28 [email protected] Roll Flutter from d4c8577 to 0c2473c (21 revisions) (flutter/packages#13470) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

github-actions Bot added p: flutter_svg triage-engine Should be looked at in engine triage labels Oct 23, 2026

francisrath added 2 commits October 23, 2026 17:09

[flutter_svg] Document the non-2xx ClientException on SvgNetworkLoader

f9b08c6

gemini-code-assist Bot reviewed Oct 23, 2026

View reviewed changes

francisrath force-pushed the flutter_svg-network-status branch from 4778026 to f9b08c6 Compare October 23, 2026 15:10

[flutter_svg] Parse the URL before creating the client in SvgNetworkL…

566a8a2

…oader A malformed URL now throws before an internally created client exists, so it cannot leak.

gaaclarke requested a review from andywolff October 26, 2026 18:23

andywolff added the CICD Run CI/CD label Oct 27, 2026

andywolff approved these changes Oct 27, 2026

View reviewed changes

andywolff added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 27, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 27, 2026

jtmcdole reviewed Oct 27, 2026

View reviewed changes

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 27, 2026

jtmcdole approved these changes Oct 27, 2026

View reviewed changes

[flutter_svg] Test that a malformed URL throws before a client is cre…

06ed82b

…ated

francisrath force-pushed the flutter_svg-network-status branch from 8b86412 to 06ed82b Compare October 27, 2026 21:42

andywolff added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Oct 28, 2026

auto-submit Bot merged commit 9d6cf9a into flutter:main Oct 28, 2026
14 checks passed

engine-flutter-autoroll mentioned this pull request Oct 29, 2026

Roll Packages from 13d38e0d97a8 to 0b72e92bf5dd (6 revisions) flutter/flutter#195588

Merged

Navigation Menu

Repository navigation

[flutter_svg] Throw on non-2xx responses in SvgNetworkLoader - #13403

[flutter_svg] Throw on non-2xx responses in SvgNetworkLoader#13403
auto-submit[bot] merged 4 commits into
flutter:mainfrom
francisrath:flutter_svg-network-status

francisrath commented Oct 23, 2026

Uh oh!

google-cla Bot commented Oct 23, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Oct 23, 2026

Uh oh!

francisrath Oct 23, 2026

Uh oh!

andywolff left a comment

Uh oh!

auto-submit Bot commented Oct 27, 2026

Uh oh!

jtmcdole Oct 27, 2026

Uh oh!

francisrath commented Oct 27, 2026

Uh oh!

jtmcdole left a comment

Uh oh!

jtmcdole commented Oct 27, 2026

Uh oh!

francisrath commented Oct 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

francisrath commented Oct 23, 2026

Pre-Review Checklist

Footnotes

Uh oh!

google-cla Bot commented Oct 23, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

francisrath Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

andywolff left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit Bot commented Oct 27, 2026

Uh oh!

jtmcdole Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

francisrath commented Oct 27, 2026

Uh oh!

jtmcdole left a comment

Choose a reason for hiding this comment

💵 The CodeFu 1024-Note: Collector's Edition

Uh oh!

jtmcdole commented Oct 27, 2026

Uh oh!

francisrath commented Oct 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants