Unverified Commit 48a5cf92 authored by Yegor's avatar Yegor Committed by GitHub

[tool] verify download URLs; docs for base URLs (#94178)

parent 403c1de5
......@@ -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: <String>[
storageBaseUrl,
cipdBaseUrl,
],
);
}
static const List<String> _hostsBlockedInChina = <String> [
'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<String> 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<String> _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<void> _download(Uri url, File file) async {
Future<void> _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) {
......
......@@ -32,6 +32,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
await artifactUpdater.downloadZipArchive(
......@@ -55,6 +56,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['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: <String>['http://test.zip'],
);
await artifactUpdater.downloadZipArchive(
......@@ -127,6 +130,7 @@ void main() {
]),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
await artifactUpdater.downloadZipArchive(
......@@ -170,6 +174,7 @@ void main() {
]),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
await expectLater(() async => artifactUpdater.downloadZipArchive(
......@@ -198,6 +203,7 @@ void main() {
]),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
await artifactUpdater.downloadZipArchive(
......@@ -225,6 +231,7 @@ void main() {
]),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
await expectLater(() async => artifactUpdater.downloadZipArchive(
......@@ -251,15 +258,16 @@ void main() {
},
),
httpClient: FakeHttpClient.list(<FakeRequest>[
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: <String>['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: <String>['http://test.zip'],
);
await expectLater(() async => artifactUpdater.downloadZipArchive(
......@@ -305,6 +314,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
operatingSystemUtils.failures = 1;
......@@ -329,6 +339,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
operatingSystemUtils.failures = 1;
......@@ -353,6 +364,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
operatingSystemUtils.failures = 2;
......@@ -377,6 +389,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
operatingSystemUtils.failures = 2;
......@@ -401,6 +414,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
await artifactUpdater.downloadZippedTarball(
......@@ -423,6 +437,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
artifactUpdater.downloadedFiles.addAll(<File>[
......@@ -450,6 +465,7 @@ void main() {
httpClient: FakeHttpClient.any(),
tempStorage: fileSystem.currentDirectory.childDirectory('temp')
..createSync(),
allowedBaseUrls: <String>['http://test.zip'],
);
final Directory errorDirectory = fileSystem.currentDirectory
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment