Skip to content

Commit

Permalink
Update setAssetDirectory service extension to fail if provided path i…
Browse files Browse the repository at this point in the history
…s invalid (flutter#35178)
  • Loading branch information
jonahwilliams authored Aug 5, 2022
1 parent cdccc60 commit 2c7b4f6
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 10 deletions.
10 changes: 6 additions & 4 deletions assets/asset_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,22 @@ AssetManager::AssetManager() = default;

AssetManager::~AssetManager() = default;

void AssetManager::PushFront(std::unique_ptr<AssetResolver> resolver) {
bool AssetManager::PushFront(std::unique_ptr<AssetResolver> resolver) {
if (resolver == nullptr || !resolver->IsValid()) {
return;
return false;
}

resolvers_.push_front(std::move(resolver));
return true;
}

void AssetManager::PushBack(std::unique_ptr<AssetResolver> resolver) {
bool AssetManager::PushBack(std::unique_ptr<AssetResolver> resolver) {
if (resolver == nullptr || !resolver->IsValid()) {
return;
return false;
}

resolvers_.push_back(std::move(resolver));
return true;
}

void AssetManager::UpdateResolverByType(
Expand Down
18 changes: 16 additions & 2 deletions assets/asset_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,23 @@ class AssetManager final : public AssetResolver {

~AssetManager() override;

void PushFront(std::unique_ptr<AssetResolver> resolver);
//--------------------------------------------------------------------------
/// @brief Adds an asset resolver to the front of the resolver queue.
/// Assets would be loaded from this resolver before any follwing
/// resolvers.
///
/// @return Returns whether this resolver is valid and has been added to
/// the resolver queue.
bool PushFront(std::unique_ptr<AssetResolver> resolver);

void PushBack(std::unique_ptr<AssetResolver> resolver);
//--------------------------------------------------------------------------
/// @brief Adds an asset resolver to the end of the resolver queue.
/// Assets would be loaded from this resolver after any previous
/// resolvers.
///
/// @return Returns whether this resolver is valid and has been added to
/// the resolver queue.
bool PushBack(std::unique_ptr<AssetResolver> resolver);

//--------------------------------------------------------------------------
/// @brief Replaces an asset resolver of the specified `type` with
Expand Down
13 changes: 9 additions & 4 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1773,10 +1773,15 @@ bool Shell::OnServiceProtocolSetAssetBundlePath(

auto asset_manager = std::make_shared<AssetManager>();

asset_manager->PushFront(std::make_unique<DirectoryAssetBundle>(
fml::OpenDirectory(params.at("assetDirectory").data(), false,
fml::FilePermission::kRead),
false));
if (!asset_manager->PushFront(std::make_unique<DirectoryAssetBundle>(
fml::OpenDirectory(params.at("assetDirectory").data(), false,
fml::FilePermission::kRead),
false))) {
// The new asset directory path was invalid.
FML_DLOG(ERROR) << "Could not update asset directory.";
ServiceProtocolFailureError(response, "Could not update asset directory.");
return false;
}

// Preserve any original asset resolvers to avoid syncing unchanged assets
// over the DevFS connection.
Expand Down
1 change: 1 addition & 0 deletions testing/dart/observatory/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ tests = [
"skp_test.dart",
"tracing_test.dart",
"shader_reload_test.dart",
"vmservice_methods_test.dart",
]

foreach(test, tests) {
Expand Down
44 changes: 44 additions & 0 deletions testing/dart/observatory/vmservice_methods_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:developer' as developer;

import 'package:litetest/litetest.dart';
import 'package:vm_service/vm_service.dart' as vms;
import 'package:vm_service/vm_service_io.dart';

void main() {
test('Setting invalid directory returns an error', () async {
vms.VmService? vmService;
try {
final developer.ServiceProtocolInfo info = await developer.Service.getInfo();
if (info.serverUri == null) {
fail('This test must not be run with --disable-observatory.');
}

vmService = await vmServiceConnectUri(
'ws://localhost:${info.serverUri!.port}${info.serverUri!.path}ws',
);
final vms.Response response = await vmService.callMethod('_flutter.listViews');
final List<Object?>? rawViews = response.json!['views'] as List<Object?>?;
final String viewId = (rawViews![0]! as Map<String, Object?>?)!['id']! as String;

dynamic error;
try {
final vms.Response setAssetDirectoryPath = await vmService.callMethod(
'_flutter.setAssetBundlePath',
args: <String, Object>{
'viewId': viewId,
'assetDirectory': ''
},
);
} catch (err) {
error = err;
}
expect(error != null, true);
} finally {
await vmService?.dispose();
}
});
}

0 comments on commit 2c7b4f6

Please sign in to comment.