From 48a5cf92251c735aa1ea3971729df3735a00ed87 Mon Sep 17 00:00:00 2001 From: Yegor Date: Thu, 2 Dec 2021 10:11:36 -0800 Subject: [PATCH] [tool] verify download URLs; docs for base URLs (#94178) --- packages/flutter_tools/lib/src/cache.dart | 83 ++++++++++++++++++- .../general.shard/artifact_updater_test.dart | 20 ++++- 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart index 44ff070e169..defc86cc20a 100644 --- a/packages/flutter_tools/lib/src/cache.dart +++ b/packages/flutter_tools/lib/src/cache.dart @@ -103,6 +103,20 @@ class DevelopmentArtifact { /// /// This does not provide any artifacts by default. See [FlutterCache] for the default /// artifact set. +/// +/// ## Artifact mirrors +/// +/// Some environments cannot reach the Google Cloud Storage buckets and CIPD due +/// to regional or corporate policies. +/// +/// To enable Flutter users in these environments, the Flutter tool supports +/// custom artifact mirrors that the administrators of such environments may +/// provide. To use an artifact mirror, the user defines the +/// `FLUTTER_STORAGE_BASE_URL` environment variable that points to the mirror. +/// Flutter tool reads this variable and uses it instead of the default URLs. +/// +/// For more details on specific URLs used to download artifacts, see +/// [storageBaseUrl] and [cipdBaseUrl]. class Cache { /// [rootOverride] is configurable for testing. /// [artifacts] is configurable for testing. @@ -179,11 +193,16 @@ class Cache { tempStorage: getDownloadDir(), platform: _platform, httpClient: HttpClient(), + allowedBaseUrls: [ + storageBaseUrl, + cipdBaseUrl, + ], ); } static const List _hostsBlockedInChina = [ 'storage.googleapis.com', + 'chrome-infra-packages.appspot.com', ]; // Initialized by FlutterCommandRunner on startup. @@ -431,6 +450,17 @@ class Cache { } String? _engineRevision; + /// The base for URLs that store Flutter engine artifacts that are fetched + /// during the installation of the Flutter SDK. + /// + /// By default the base URL is https://storage.googleapis.com. However, if + /// `FLUTTER_STORAGE_BASE_URL` environment variable is provided, the + /// environment variable value is returned instead. + /// + /// See also: + /// + /// * [cipdBaseUrl], which determines how CIPD artifacts are fetched. + /// * [Cache] class-level dartdocs that explain how artifact mirrors work. String get storageBaseUrl { final String? overrideUrl = _platform.environment['FLUTTER_STORAGE_BASE_URL']; if (overrideUrl == null) { @@ -446,6 +476,25 @@ class Cache { return overrideUrl; } + /// The base for URLs that store Flutter engine artifacts in CIPD. + /// + /// For some platforms, such as Web and Fuchsia, CIPD artifacts are fetched + /// during the installation of the Flutter SDK, in addition to those fetched + /// from [storageBaseUrl]. + /// + /// By default the base URL is https://chrome-infra-packages.appspot.com/dl. + /// However, if `FLUTTER_STORAGE_BASE_URL` environment variable is provided, + /// then the following value is used: + /// + /// FLUTTER_STORAGE_BASE_URL/flutter_infra_release/cipd + /// + /// See also: + /// + /// * [storageBaseUrl], which determines how engine artifacts stored in the + /// Google Cloud Storage buckets are fetched. + /// * https://chromium.googlesource.com/infra/luci/luci-go/+/refs/heads/main/cipd, + /// which contains information about CIPD. + /// * [Cache] class-level dartdocs that explain how artifact mirrors work. String get cipdBaseUrl { final String? overrideUrl = _platform.environment['FLUTTER_STORAGE_BASE_URL']; if (overrideUrl == null) { @@ -925,12 +974,14 @@ class ArtifactUpdater { required Directory tempStorage, required HttpClient httpClient, required Platform platform, + required List allowedBaseUrls, }) : _operatingSystemUtils = operatingSystemUtils, _httpClient = httpClient, _logger = logger, _fileSystem = fileSystem, _tempStorage = tempStorage, - _platform = platform; + _platform = platform, + _allowedBaseUrls = allowedBaseUrls; /// The number of times the artifact updater will repeat the artifact download loop. static const int _kRetryCount = 2; @@ -942,6 +993,13 @@ class ArtifactUpdater { final HttpClient _httpClient; final Platform _platform; + /// Artifacts should only be downloaded from URLs that use one of these + /// prefixes. + /// + /// [ArtifactUpdater] will issue a warning if an attempt to download from a + /// non-compliant URL is made. + final List _allowedBaseUrls; + /// Keep track of the files we've downloaded for this execution so we /// can delete them after completion. We don't delete them right after /// extraction in case [update] is interrupted, so we can restart without @@ -994,7 +1052,7 @@ class ArtifactUpdater { if (tempFile.existsSync()) { tempFile.deleteSync(); } - await _download(url, tempFile); + await _download(url, tempFile, status); if (!tempFile.existsSync()) { throw Exception('Did not find downloaded file ${tempFile.path}'); @@ -1077,7 +1135,26 @@ class ArtifactUpdater { /// /// See also: /// * https://cloud.google.com/storage/docs/xml-api/reference-headers#xgooghash - Future _download(Uri url, File file) async { + Future _download(Uri url, File file, Status status) async { + final bool isAllowedUrl = _allowedBaseUrls.any((String baseUrl) => url.toString().startsWith(baseUrl)); + + // In tests make this a hard failure. + assert( + isAllowedUrl, + 'URL not allowed: $url\n' + 'Allowed URLs must be based on one of: ${_allowedBaseUrls.join(', ')}', + ); + + // In production, issue a warning but allow the download to proceed. + if (!isAllowedUrl) { + status.pause(); + _logger.printWarning( + 'Downloading an artifact that may not be reachable in some environments (e.g. firewalled environments): $url\n' + 'This should not have happened. This is likely a Flutter SDK bug. Please file an issue at https://github.com/flutter/flutter/issues/new?template=1_activation.md' + ); + status.resume(); + } + final HttpClientRequest request = await _httpClient.getUrl(url); final HttpClientResponse response = await request.close(); if (response.statusCode != HttpStatus.ok) { diff --git a/packages/flutter_tools/test/general.shard/artifact_updater_test.dart b/packages/flutter_tools/test/general.shard/artifact_updater_test.dart index 9946e803279..12bfef989b3 100644 --- a/packages/flutter_tools/test/general.shard/artifact_updater_test.dart +++ b/packages/flutter_tools/test/general.shard/artifact_updater_test.dart @@ -32,6 +32,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); await artifactUpdater.downloadZipArchive( @@ -55,6 +56,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); // Unrelated file from another cache. fileSystem.file('out/bar').createSync(recursive: true); @@ -92,6 +94,7 @@ void main() { ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); await artifactUpdater.downloadZipArchive( @@ -127,6 +130,7 @@ void main() { ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); await artifactUpdater.downloadZipArchive( @@ -170,6 +174,7 @@ void main() { ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); await expectLater(() async => artifactUpdater.downloadZipArchive( @@ -198,6 +203,7 @@ void main() { ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); await artifactUpdater.downloadZipArchive( @@ -225,6 +231,7 @@ void main() { ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); await expectLater(() async => artifactUpdater.downloadZipArchive( @@ -251,15 +258,16 @@ void main() { }, ), httpClient: FakeHttpClient.list([ - FakeRequest(Uri.parse('http:///foo-bar/test.zip'), responseError: ArgumentError()) + FakeRequest(Uri.parse('http://foo-bar/test.zip'), responseError: ArgumentError()) ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://foo-bar/test.zip'], ); await expectLater(() async => artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///foo-bar/test.zip'), + Uri.parse('http://foo-bar/test.zip'), fileSystem.currentDirectory.childDirectory('out'), ), throwsToolExit()); @@ -281,6 +289,7 @@ void main() { ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); await expectLater(() async => artifactUpdater.downloadZipArchive( @@ -305,6 +314,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); operatingSystemUtils.failures = 1; @@ -329,6 +339,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); operatingSystemUtils.failures = 1; @@ -353,6 +364,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); operatingSystemUtils.failures = 2; @@ -377,6 +389,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); operatingSystemUtils.failures = 2; @@ -401,6 +414,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); await artifactUpdater.downloadZippedTarball( @@ -423,6 +437,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); artifactUpdater.downloadedFiles.addAll([ @@ -450,6 +465,7 @@ void main() { httpClient: FakeHttpClient.any(), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), + allowedBaseUrls: ['http://test.zip'], ); final Directory errorDirectory = fileSystem.currentDirectory