From b142c9bbdb30f7763a52b4b0a618d46b063ecadf Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 28 Oct 2019 10:54:03 -0700 Subject: [PATCH] catch failure to parse FLUTTER_STORAGE_BASE_URL (#43599) --- packages/flutter_tools/lib/src/cache.dart | 19 +++++++++++++------ .../test/general.shard/cache_test.dart | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart index 6910d5f34b6..1e25e0126a4 100644 --- a/packages/flutter_tools/lib/src/cache.dart +++ b/packages/flutter_tools/lib/src/cache.dart @@ -482,16 +482,23 @@ abstract class CachedArtifact extends ArtifactSet { /// Template method to perform artifact update. Future updateInner(); - String get _storageBaseUrl { + @visibleForTesting + String get storageBaseUrl { final String overrideUrl = platform.environment['FLUTTER_STORAGE_BASE_URL']; if (overrideUrl == null) { return 'https://storage.googleapis.com'; } + // verify that this is a valid URI. + try { + Uri.parse(overrideUrl); + } on FormatException catch (err) { + throwToolExit('"FLUTTER_STORAGE_BASE_URL" contains an invalid URI:\n$err'); + } _maybeWarnAboutStorageOverride(overrideUrl); return overrideUrl; } - Uri _toStorageUri(String path) => Uri.parse('$_storageBaseUrl/$path'); + Uri _toStorageUri(String path) => Uri.parse('$storageBaseUrl/$path'); /// Download an archive from the given [url] and unzip it to [location]. Future _downloadArchive(String message, Uri url, Directory location, bool verifier(File f), void extractor(File f, Directory d)) { @@ -587,7 +594,7 @@ class FlutterWebSdk extends CachedArtifact { } else if (platform.isWindows) { platformName += 'windows-x64'; } - final Uri url = Uri.parse('$_storageBaseUrl/flutter_infra/flutter/$version/$platformName.zip'); + final Uri url = Uri.parse('$storageBaseUrl/flutter_infra/flutter/$version/$platformName.zip'); await _downloadZipArchive('Downloading Web SDK...', url, location); // This is a temporary work-around for not being able to safely download into a shared directory. for (FileSystemEntity entity in location.listSync(recursive: true)) { @@ -652,7 +659,7 @@ abstract class EngineCachedArtifact extends CachedArtifact { @override Future updateInner() async { - final String url = '$_storageBaseUrl/flutter_infra/flutter/$version/'; + final String url = '$storageBaseUrl/flutter_infra/flutter/$version/'; final Directory pkgDir = cache.getCacheDir('pkg'); for (String pkgName in getPackageDirs()) { @@ -687,7 +694,7 @@ abstract class EngineCachedArtifact extends CachedArtifact { Future checkForArtifacts(String engineVersion) async { engineVersion ??= version; - final String url = '$_storageBaseUrl/flutter_infra/flutter/$engineVersion/'; + final String url = '$storageBaseUrl/flutter_infra/flutter/$engineVersion/'; bool exists = false; for (String pkgName in getPackageDirs()) { @@ -1111,7 +1118,7 @@ class IosUsbArtifacts extends CachedArtifact { } @visibleForTesting - Uri get archiveUri => Uri.parse('$_storageBaseUrl/flutter_infra/ios-usb-dependencies${cache.useUnsignedMacBinaries ? '/unsigned' : ''}/$name/$version/$name.zip'); + Uri get archiveUri => Uri.parse('$storageBaseUrl/flutter_infra/ios-usb-dependencies${cache.useUnsignedMacBinaries ? '/unsigned' : ''}/$name/$version/$name.zip'); } // Many characters are problematic in filenames, especially on Windows. diff --git a/packages/flutter_tools/test/general.shard/cache_test.dart b/packages/flutter_tools/test/general.shard/cache_test.dart index ae3dcb658be..f1cf2ded7d7 100644 --- a/packages/flutter_tools/test/general.shard/cache_test.dart +++ b/packages/flutter_tools/test/general.shard/cache_test.dart @@ -5,6 +5,7 @@ import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:file_testing/file_testing.dart'; +import 'package:flutter_tools/src/base/platform.dart'; import 'package:meta/meta.dart'; import 'package:mockito/mockito.dart'; import 'package:platform/platform.dart'; @@ -202,6 +203,18 @@ void main() { ); } }); + + testUsingContext('Invalid URI for FLUTTER_STORAGE_BASE_URL throws ToolExit', () async { + when(platform.environment).thenReturn(const { + 'FLUTTER_STORAGE_BASE_URL': ' http://foo', + }); + final Cache cache = Cache(); + final CachedArtifact artifact = MaterialFonts(cache); + + expect(() => artifact.storageBaseUrl, throwsA(isInstanceOf())); + }, overrides: { + Platform: () => MockPlatform(), + }); }); testUsingContext('flattenNameSubdirs', () { @@ -379,3 +392,4 @@ class MockIosUsbArtifacts extends Mock implements IosUsbArtifacts {} class MockInternetAddress extends Mock implements InternetAddress {} class MockCache extends Mock implements Cache {} class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {} +class MockPlatform extends Mock implements Platform {}