Merge "more normalized path fix"

This commit is contained in:
TreeHugger Robot
2020-02-04 04:40:43 +00:00
committed by Android (Google) Code Review
3 changed files with 73 additions and 57 deletions

View File

@@ -617,15 +617,7 @@ int IncrementalService::bind(StorageId storage, std::string_view source, std::st
if (storageInfo == ifs->storages.end()) {
return -EINVAL;
}
std::string normSource;
if (path::isAbsolute(source)) {
normSource = path::normalize(source);
} else {
normSource = path::normalize(path::join(storageInfo->second.name, source));
}
if (!path::startsWith(normSource, storageInfo->second.name)) {
return -EINVAL;
}
std::string normSource = normalizePathToStorage(ifs, storage, source);
l.unlock();
std::unique_lock l2(mLock, std::defer_lock);
return addBindMount(*ifs, storage, storageInfo->second.name, std::move(normSource),
@@ -674,20 +666,29 @@ int IncrementalService::unbind(StorageId storage, std::string_view target) {
return 0;
}
std::string IncrementalService::normalizePathToStorage(const IncrementalService::IfsMountPtr ifs,
StorageId storage, std::string_view path) {
const auto storageInfo = ifs->storages.find(storage);
if (storageInfo == ifs->storages.end()) {
return {};
}
std::string normPath;
if (path::isAbsolute(path)) {
normPath = path::normalize(path);
} else {
normPath = path::normalize(path::join(storageInfo->second.name, path));
}
if (!path::startsWith(normPath, storageInfo->second.name)) {
return {};
}
return normPath;
}
int IncrementalService::makeFile(StorageId storage, std::string_view path, int mode, FileId id,
incfs::NewFileParams params) {
if (auto ifs = getIfs(storage)) {
const auto storageInfo = ifs->storages.find(storage);
if (storageInfo == ifs->storages.end()) {
return -EINVAL;
}
std::string normPath;
if (path::isAbsolute(path)) {
normPath = path::normalize(path);
} else {
normPath = path::normalize(path::join(storageInfo->second.name, path));
}
if (!path::startsWith(normPath, storageInfo->second.name)) {
std::string normPath = normalizePathToStorage(ifs, storage, path);
if (normPath.empty()) {
return -EINVAL;
}
auto err = mIncFs->makeFile(ifs->control, normPath, mode, id, params);
@@ -706,7 +707,11 @@ int IncrementalService::makeFile(StorageId storage, std::string_view path, int m
int IncrementalService::makeDir(StorageId storageId, std::string_view path, int mode) {
if (auto ifs = getIfs(storageId)) {
return mIncFs->makeDir(ifs->control, path, mode);
std::string normPath = normalizePathToStorage(ifs, storageId, path);
if (normPath.empty()) {
return -EINVAL;
}
return mIncFs->makeDir(ifs->control, normPath, mode);
}
return -EINVAL;
}
@@ -716,31 +721,40 @@ int IncrementalService::makeDirs(StorageId storageId, std::string_view path, int
if (!ifs) {
return -EINVAL;
}
auto err = mIncFs->makeDir(ifs->control, path, mode);
std::string normPath = normalizePathToStorage(ifs, storageId, path);
if (normPath.empty()) {
return -EINVAL;
}
auto err = mIncFs->makeDir(ifs->control, normPath, mode);
if (err == -EEXIST) {
return 0;
} else if (err != -ENOENT) {
return err;
}
if (auto err = makeDirs(storageId, path::dirname(path), mode)) {
if (auto err = makeDirs(storageId, path::dirname(normPath), mode)) {
return err;
}
return mIncFs->makeDir(ifs->control, path, mode);
return mIncFs->makeDir(ifs->control, normPath, mode);
}
int IncrementalService::link(StorageId sourceStorageId, std::string_view oldPath,
StorageId destStorageId, std::string_view newPath) {
if (auto ifsSrc = getIfs(sourceStorageId), ifsDest = getIfs(destStorageId);
ifsSrc && ifsSrc == ifsDest) {
return mIncFs->link(ifsSrc->control, oldPath, newPath);
std::string normOldPath = normalizePathToStorage(ifsSrc, sourceStorageId, oldPath);
std::string normNewPath = normalizePathToStorage(ifsDest, destStorageId, newPath);
if (normOldPath.empty() || normNewPath.empty()) {
return -EINVAL;
}
return mIncFs->link(ifsSrc->control, normOldPath, normNewPath);
}
return -EINVAL;
}
int IncrementalService::unlink(StorageId storage, std::string_view path) {
if (auto ifs = getIfs(storage)) {
return mIncFs->unlink(ifs->control, path);
std::string normOldPath = normalizePathToStorage(ifs, storage, path);
return mIncFs->unlink(ifs->control, normOldPath);
}
return -EINVAL;
}

View File

@@ -18,8 +18,8 @@
#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <android/os/incremental/IIncrementalManager.h>
#include <android/content/pm/DataLoaderParamsParcel.h>
#include <android/os/incremental/IIncrementalManager.h>
#include <binder/IServiceManager.h>
#include <utils/String16.h>
#include <utils/StrongPointer.h>
@@ -92,8 +92,7 @@ public:
std::optional<std::future<void>> onSystemReady();
StorageId createStorage(std::string_view mountPoint,
DataLoaderParamsParcel&& dataLoaderParams,
StorageId createStorage(std::string_view mountPoint, DataLoaderParamsParcel&& dataLoaderParams,
CreateOptions options = CreateOptions::Default);
StorageId createLinkedStorage(std::string_view mountPoint, StorageId linkedStorage,
CreateOptions options = CreateOptions::Default);
@@ -207,6 +206,8 @@ private:
void deleteStorage(IncFsMount& ifs);
void deleteStorageLocked(IncFsMount& ifs, std::unique_lock<std::mutex>&& ifsLock);
MountMap::iterator getStorageSlotLocked();
std::string normalizePathToStorage(const IfsMountPtr incfs, StorageId storage,
std::string_view path);
// Member variables
std::unique_ptr<VoldServiceWrapper> mVold;

View File

@@ -406,29 +406,16 @@ TEST_F(IncrementalServiceTest, testMakeDirectory) {
int storageId =
mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel),
IncrementalService::CreateOptions::CreateNew);
std::string_view dir_path("test");
EXPECT_CALL(*mIncFs, makeDir(_, dir_path, _));
auto res = mIncrementalService->makeDir(storageId, dir_path, 0555);
ASSERT_EQ(res, 0);
}
std::string dir_path("test");
TEST_F(IncrementalServiceTest, testMakeDirectoryNested) {
mVold->mountIncFsSuccess();
mIncFs->makeFileSuccess();
mVold->bindMountSuccess();
mIncrementalManager->prepareDataLoaderSuccess();
mIncrementalManager->startDataLoaderSuccess();
TemporaryDir tempDir;
int storageId =
mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel),
IncrementalService::CreateOptions::CreateNew);
auto first = "first"sv;
auto second = "second"sv;
std::string dir_path = std::string(first) + "/" + std::string(second);
EXPECT_CALL(*mIncFs, makeDir(_, first, _)).Times(0);
EXPECT_CALL(*mIncFs, makeDir(_, second, _)).Times(0);
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(dir_path), _)).Times(1);
std::string tempPath(tempDir.path);
std::replace(tempPath.begin(), tempPath.end(), '/', '_');
std::string mount_dir = std::string(mRootDir.path) + "/" + tempPath.substr(1);
std::string normalized_dir_path = mount_dir + "/mount/st_1_0/" + dir_path;
// Expecting incfs to call makeDir on a path like:
// /data/local/tmp/TemporaryDir-06yixG/data_local_tmp_TemporaryDir-xwdFhT/mount/st_1_0/test
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _));
auto res = mIncrementalService->makeDir(storageId, dir_path, 0555);
ASSERT_EQ(res, 0);
}
@@ -446,15 +433,29 @@ TEST_F(IncrementalServiceTest, testMakeDirectories) {
auto first = "first"sv;
auto second = "second"sv;
auto third = "third"sv;
std::string tempPath(tempDir.path);
std::replace(tempPath.begin(), tempPath.end(), '/', '_');
std::string mount_dir = std::string(mRootDir.path) + "/" + tempPath.substr(1);
InSequence seq;
auto parent_path = std::string(first) + "/" + std::string(second);
auto dir_path = parent_path + "/" + std::string(third);
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(dir_path), _)).WillOnce(Return(-ENOENT));
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(parent_path), _)).WillOnce(Return(-ENOENT));
EXPECT_CALL(*mIncFs, makeDir(_, first, _)).WillOnce(Return(0));
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(parent_path), _)).WillOnce(Return(0));
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(dir_path), _)).WillOnce(Return(0));
auto res = mIncrementalService->makeDirs(storageId, dir_path, 0555);
std::string normalized_first_path = mount_dir + "/mount/st_1_0/" + std::string(first);
std::string normalized_parent_path = mount_dir + "/mount/st_1_0/" + parent_path;
std::string normalized_dir_path = mount_dir + "/mount/st_1_0/" + dir_path;
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _))
.WillOnce(Return(-ENOENT));
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_parent_path), _))
.WillOnce(Return(-ENOENT));
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_first_path), _))
.WillOnce(Return(0));
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_parent_path), _))
.WillOnce(Return(0));
EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _)).WillOnce(Return(0));
auto res = mIncrementalService->makeDirs(storageId, normalized_dir_path, 0555);
ASSERT_EQ(res, 0);
}
} // namespace android::os::incremental