From 34924ac842558fcdf0ac09880ac6e2ab2129ef3d Mon Sep 17 00:00:00 2001 From: Trevor Shoe Date: Fri, 15 Nov 2024 14:56:19 -0500 Subject: [PATCH] [platform] Remove chmod-esque functions --- include/multipass/file_ops.h | 2 - include/multipass/path.h | 2 + include/multipass/platform.h | 2 +- src/platform/platform_linux.cpp | 2 +- src/platform/platform_unix.cpp | 7 ++- src/ssh/sftp_client.cpp | 14 +++--- src/sshfs_mount/sftp_server.cpp | 12 ++--- src/utils/file_ops.cpp | 10 ---- tests/linux/test_platform_linux.cpp | 16 +++++-- tests/mock_file_ops.h | 2 - tests/test_file_ops.cpp | 8 ---- tests/test_sftp_client.cpp | 71 ++++++++++++++++++----------- tests/test_sftpserver.cpp | 17 ++++--- tests/unix/test_platform_unix.cpp | 2 +- 14 files changed, 84 insertions(+), 83 deletions(-) diff --git a/include/multipass/file_ops.h b/include/multipass/file_ops.h index fef2aeb5bb..7bb9ddf878 100644 --- a/include/multipass/file_ops.h +++ b/include/multipass/file_ops.h @@ -81,7 +81,6 @@ class FileOps : public Singleton virtual bool rename(QFile& file, const QString& newName) const; virtual bool resize(QFile& file, qint64 sz) const; virtual bool seek(QFile& file, qint64 pos) const; - virtual bool setPermissions(QFile& file, QFileDevice::Permissions permissions) const; virtual qint64 size(QFile& file) const; virtual qint64 write(QFile& file, const char* data, qint64 maxSize) const; virtual qint64 write(QFileDevice& file, const QByteArray& data) const; @@ -112,7 +111,6 @@ class FileOps : public Singleton virtual bool remove(const fs::path& path, std::error_code& err) const; virtual void create_symlink(const fs::path& to, const fs::path& path, std::error_code& err) const; virtual fs::path read_symlink(const fs::path& path, std::error_code& err) const; - virtual void permissions(const fs::path& path, fs::perms perms, std::error_code& err) const; virtual fs::file_status status(const fs::path& path, std::error_code& err) const; virtual fs::file_status symlink_status(const fs::path& path, std::error_code& err) const; virtual std::unique_ptr recursive_dir_iterator(const fs::path& path, diff --git a/include/multipass/path.h b/include/multipass/path.h index 14154b7fd7..f0658cc8fd 100644 --- a/include/multipass/path.h +++ b/include/multipass/path.h @@ -20,10 +20,12 @@ #ifndef MULTIPASS_PATH_H #define MULTIPASS_PATH_H +#include #include namespace multipass { using Path = QString; +using Perms = QFileDevice::Permissions; } #endif // MULTIPASS_PATH_H diff --git a/include/multipass/platform.h b/include/multipass/platform.h index 02ab2a58c1..c5e42e4191 100644 --- a/include/multipass/platform.h +++ b/include/multipass/platform.h @@ -58,7 +58,7 @@ class Platform : public Singleton virtual bool is_remote_supported(const std::string& remote) const; virtual bool is_backend_supported(const QString& backend) const; // temporary (?) virtual int chown(const char* path, unsigned int uid, unsigned int gid) const; - virtual bool set_permissions(const multipass::Path& path, const QFileDevice::Permissions permissions) const; + virtual bool set_permissions(const Path& path, const Perms permissions) const; virtual bool link(const char* target, const char* link) const; virtual bool symlink(const char* target, const char* link, bool is_dir) const; virtual int utime(const char* path, int atime, int mtime) const; diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index 9eb98766e2..57aec7d9c6 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -317,7 +317,7 @@ void mp::platform::Platform::create_alias_script(const std::string& alias, const auto permissions = MP_FILEOPS.permissions(file) | QFileDevice::ExeOwner | QFileDevice::ExeGroup | QFileDevice::ExeOther; - if (!MP_FILEOPS.setPermissions(file, permissions)) + if (!MP_PLATFORM.set_permissions(file.fileName(), permissions)) throw std::runtime_error(fmt::format("cannot set permissions to alias script '{}'", file_path)); } diff --git a/src/platform/platform_unix.cpp b/src/platform/platform_unix.cpp index 2a5c659642..c0c87e74ba 100644 --- a/src/platform/platform_unix.cpp +++ b/src/platform/platform_unix.cpp @@ -58,7 +58,7 @@ int mp::platform::Platform::chown(const char* path, unsigned int uid, unsigned i return ::lchown(path, uid, gid); } -bool mp::platform::Platform::set_permissions(const mp::Path& path, const QFileDevice::Permissions permissions) const +bool mp::platform::Platform::set_permissions(const Path& path, const Perms permissions) const { return QFile::setPermissions(path, permissions); } @@ -126,9 +126,8 @@ void mp::platform::Platform::set_server_socket_restrictions(const std::string& s if (chown(socket_path.c_str(), 0, gid) == -1) throw std::runtime_error(fmt::format("Could not set ownership of the multipass socket: {}", strerror(errno))); - if (!set_permissions(socket_path.c_str(), mode)) - throw std::runtime_error( - fmt::format("Could not set permissions for the multipass socket: {}", strerror(errno))); + if (!set_permissions(QString::fromStdString(socket_path), mode)) + throw std::runtime_error(fmt::format("Could not set permissions for the multipass socket")); } QString mp::platform::Platform::multipass_storage_location() const diff --git a/src/ssh/sftp_client.cpp b/src/ssh/sftp_client.cpp index 9806d61708..19a3ced274 100644 --- a/src/ssh/sftp_client.cpp +++ b/src/ssh/sftp_client.cpp @@ -20,6 +20,7 @@ #include "ssh_client_key_provider.h" #include #include +#include #include #include #include @@ -146,9 +147,8 @@ void SFTPClient::pull_file(const fs::path& source_path, const fs::path& target_p do_pull_file(source_path, *local_file); auto source_perms = mp_sftp_stat(sftp.get(), source_path.u8string().c_str())->permissions; - std::error_code err; - if (MP_FILEOPS.permissions(target_path, static_cast(source_perms), err); err) - throw SFTPError{"cannot set permissions for local file {}: {}", target_path, err.message()}; + if (!MP_PLATFORM.set_permissions(QString::fromStdWString(target_path.wstring()), static_cast(source_perms))) + throw SFTPError{"cannot set permissions for local file {}", target_path}; if (local_file->fail()) throw SFTPError{"cannot write to local file {}: {}", target_path, strerror(errno)}; @@ -301,11 +301,11 @@ bool SFTPClient::pull_dir(const fs::path& source_path, const fs::path& target_pa for (auto it = subdirectory_perms.crbegin(); it != subdirectory_perms.crend(); ++it) { const auto& [path, perms] = *it; - MP_FILEOPS.permissions(path, static_cast(perms), err); - if (err) + if (!MP_PLATFORM.set_permissions(QString::fromStdWString(path.wstring()), static_cast(perms))) { - mpl::log(mpl::Level::error, log_category, - fmt::format("cannot set permissions for local directory {}: {}", path, err.message())); + mpl::log(mpl::Level::error, + log_category, + fmt::format("cannot set permissions for local directory {}", path)); success = false; } } diff --git a/src/sshfs_mount/sftp_server.cpp b/src/sshfs_mount/sftp_server.cpp index ede226e7d7..6f8b1e8884 100644 --- a/src/sshfs_mount/sftp_server.cpp +++ b/src/sshfs_mount/sftp_server.cpp @@ -529,13 +529,11 @@ int mp::SftpServer::handle_mkdir(sftp_client_message msg) return reply_failure(msg); } - std::error_code err; - MP_FILEOPS.permissions(filename, static_cast(msg->attr->permissions), err); - if (err) + if (!MP_PLATFORM.set_permissions(filename, static_cast(msg->attr->permissions))) { mpl::log(mpl::Level::trace, category, - fmt::format("{}: set permissions failed for '{}': {}", __FUNCTION__, filename, err.message())); + fmt::format("{}: set permissions failed for '{}'", __FUNCTION__, filename)); return reply_failure(msg); } @@ -1020,13 +1018,11 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg) if (msg->attr->flags & SSH_FILEXFER_ATTR_PERMISSIONS) { - std::error_code err; - MP_FILEOPS.permissions(file.fileName().toStdString(), static_cast(msg->attr->permissions), err); - if (err) + if (!MP_PLATFORM.set_permissions(file.fileName(), static_cast(msg->attr->permissions))) { mpl::log(mpl::Level::trace, category, - fmt::format("{}: set permissions failed for '{}': {}", __FUNCTION__, filename, err.message())); + fmt::format("{}: set permissions failed for '{}'", __FUNCTION__, filename)); return reply_failure(msg); } } diff --git a/src/utils/file_ops.cpp b/src/utils/file_ops.cpp index 1240ab75ff..e266170859 100644 --- a/src/utils/file_ops.cpp +++ b/src/utils/file_ops.cpp @@ -145,11 +145,6 @@ bool mp::FileOps::seek(QFile& file, qint64 pos) const return file.seek(pos); } -bool mp::FileOps::setPermissions(QFile& file, QFileDevice::Permissions permissions) const -{ - return file.setPermissions(permissions); -} - qint64 mp::FileOps::size(QFile& file) const { return file.size(); @@ -262,11 +257,6 @@ fs::path mp::FileOps::read_symlink(const fs::path& path, std::error_code& err) c return fs::read_symlink(path, err); } -void mp::FileOps::permissions(const fs::path& path, fs::perms perms, std::error_code& err) const -{ - fs::permissions(path, perms, err); -} - fs::file_status mp::FileOps::status(const fs::path& path, std::error_code& err) const { return fs::status(path, err); diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index 3e85b89ab6..b277ac5c48 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -56,6 +56,7 @@ #include #include +#include namespace mp = multipass; namespace mpt = multipass::test; @@ -627,13 +628,16 @@ TEST_F(PlatformLinux, create_alias_script_overwrites) { auto [mock_utils, guard1] = mpt::MockUtils::inject(); auto [mock_file_ops, guard2] = mpt::MockFileOps::inject(); + auto [mock_platform, guard3] = mpt::MockPlatform::inject(); EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1); EXPECT_CALL(*mock_file_ops, permissions(_)).WillOnce(Return(QFileDevice::ReadOwner | QFileDevice::WriteOwner)); - EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillOnce(Return(true)); + // Calls the platform function directly since MP_PLATFORM is mocked. EXPECT_NO_THROW( - MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "other_command", "map"})); + mock_platform->Platform::create_alias_script("alias_name", + mp::AliasDefinition{"instance", "other_command", "map"})); } TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_create_path) @@ -664,14 +668,16 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_set_permissions) { auto [mock_utils, guard1] = mpt::MockUtils::inject(); auto [mock_file_ops, guard2] = mpt::MockFileOps::inject(); + auto [mock_platform, guard3] = mpt::MockPlatform::inject(); EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1); EXPECT_CALL(*mock_file_ops, permissions(_)).WillOnce(Return(QFileDevice::ReadOwner | QFileDevice::WriteOwner)); - EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(false)); + EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillOnce(Return(false)); MP_EXPECT_THROW_THAT( - MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}), - std::runtime_error, mpt::match_what(HasSubstr("cannot set permissions to alias script '"))); + mock_platform->Platform::create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}), + std::runtime_error, + mpt::match_what(HasSubstr("cannot set permissions to alias script '"))); } TEST_F(PlatformLinux, remove_alias_script_works) diff --git a/tests/mock_file_ops.h b/tests/mock_file_ops.h index cf257f23ea..de7bc4d85d 100644 --- a/tests/mock_file_ops.h +++ b/tests/mock_file_ops.h @@ -56,7 +56,6 @@ class MockFileOps : public FileOps MOCK_METHOD(bool, rename, (QFile&, const QString& newName), (const, override)); MOCK_METHOD(bool, resize, (QFile&, qint64 sz), (const, override)); MOCK_METHOD(bool, seek, (QFile&, qint64 pos), (const, override)); - MOCK_METHOD(bool, setPermissions, (QFile&, QFileDevice::Permissions), (const, override)); MOCK_METHOD(qint64, size, (QFile&), (const, override)); MOCK_METHOD(qint64, write, (QFile&, const char*, qint64), (const, override)); MOCK_METHOD(qint64, write, (QFileDevice&, const QByteArray&), (const, override)); @@ -87,7 +86,6 @@ class MockFileOps : public FileOps MOCK_METHOD(void, create_symlink, (const fs::path& to, const fs::path& path, std::error_code& err), (override, const)); MOCK_METHOD(fs::path, read_symlink, (const fs::path& path, std::error_code& err), (override, const)); - MOCK_METHOD(void, permissions, (const fs::path& path, fs::perms perms, std::error_code& err), (override, const)); MOCK_METHOD(fs::file_status, status, (const fs::path& path, std::error_code& err), (override, const)); MOCK_METHOD(fs::file_status, symlink_status, (const fs::path& path, std::error_code& err), (override, const)); MOCK_METHOD(std::unique_ptr, recursive_dir_iterator, diff --git a/tests/test_file_ops.cpp b/tests/test_file_ops.cpp index f8a54caac2..bd78fd379b 100644 --- a/tests/test_file_ops.cpp +++ b/tests/test_file_ops.cpp @@ -108,14 +108,6 @@ TEST_F(FileOps, symlink) EXPECT_FALSE(err); } -TEST_F(FileOps, permissions) -{ - MP_FILEOPS.permissions(temp_file, fs::perms::all, err); - EXPECT_FALSE(err); - MP_FILEOPS.permissions(temp_dir / "nonexistent", fs::perms::all, err); - EXPECT_TRUE(err); -} - TEST_F(FileOps, status) { auto dir_status = MP_FILEOPS.status(temp_dir, err); diff --git a/tests/test_sftp_client.cpp b/tests/test_sftp_client.cpp index fb2366eb81..47f747e5f0 100644 --- a/tests/test_sftp_client.cpp +++ b/tests/test_sftp_client.cpp @@ -18,6 +18,7 @@ #include "common.h" #include "mock_file_ops.h" #include "mock_logger.h" +#include "mock_platform.h" #include "mock_recursive_dir_iterator.h" #include "mock_sftp.h" #include "mock_sftp_dir_iterator.h" @@ -85,6 +86,8 @@ struct SFTPClient : public testing::Test mpt::MockFileOps::GuardedMock mock_file_ops_guard{mpt::MockFileOps::inject()}; mpt::MockFileOps* mock_file_ops{mock_file_ops_guard.first}; + const mpt::MockPlatform::GuardedMock mock_platform_guard{mpt::MockPlatform::inject()}; + const mpt::MockPlatform& mock_platform{*mock_platform_guard.first}; mpt::MockSFTPUtils::GuardedMock mock_sftp_utils_guard{mpt::MockSFTPUtils::inject()}; mpt::MockSFTPUtils* mock_sftp_utils{mock_sftp_utils_guard.first}; mpt::MockLogger::Scope mock_logger_scope{mpt::MockLogger::inject()}; @@ -288,15 +291,19 @@ TEST_F(SFTPClient, pull_file_success) mode_t perms = 0777; REPLACE(sftp_stat, [&](auto...) { return get_dummy_sftp_attr(SSH_FILEXFER_TYPE_REGULAR, "", perms); }); - fs::perms written_perms; - EXPECT_CALL(*mock_file_ops, permissions(target_path, static_cast(perms), _)) - .WillOnce([&](auto, auto perms, auto) { written_perms = perms; }); + mp::Perms written_perms; + EXPECT_CALL(mock_platform, + set_permissions(QString::fromStdWString(target_path.wstring()), static_cast(perms))) + .WillOnce([&](auto, auto perms) { + written_perms = perms; + return true; + }); auto sftp_client = make_sftp_client(); EXPECT_TRUE(sftp_client.pull(source_path, target_path)); EXPECT_EQ(test_data, test_file.str()); - EXPECT_EQ(static_cast(perms), written_perms); + EXPECT_EQ(static_cast(perms), written_perms); } TEST_F(SFTPClient, pull_file_cannot_open_source) @@ -354,7 +361,8 @@ TEST_F(SFTPClient, pull_file_cannot_write_target) }; REPLACE(sftp_read, mocked_sftp_read); REPLACE(sftp_stat, [&](auto...) { return get_dummy_sftp_attr(); }); - EXPECT_CALL(*mock_file_ops, permissions(target_path, _, _)); + EXPECT_CALL(mock_platform, set_permissions(QString::fromStdWString(target_path.wstring()), _)) + .WillOnce(Return(true)); REPLACE(sftp_setstat, [](auto...) { return SSH_FX_OK; }); auto sftp_client = make_sftp_client(); @@ -392,14 +400,14 @@ TEST_F(SFTPClient, pull_file_cannot_set_perms) mode_t perms = 0777; REPLACE(sftp_stat, [&](auto...) { return get_dummy_sftp_attr(SSH_FILEXFER_TYPE_REGULAR, "", perms); }); - auto err = std::make_error_code(std::errc::permission_denied); - EXPECT_CALL(*mock_file_ops, permissions(target_path, static_cast(perms), _)) - .WillOnce([&](auto, auto, std::error_code& e) { e = err; }); + + EXPECT_CALL(mock_platform, + set_permissions(QString::fromStdWString(target_path.wstring()), static_cast(perms))) + .WillOnce(Return(false)); auto sftp_client = make_sftp_client(); - mock_logger->expect_log(mpl::Level::error, - fmt::format("cannot set permissions for local file {}: {}", target_path, err.message())); + mock_logger->expect_log(mpl::Level::error, fmt::format("cannot set permissions for local file {}", target_path)); EXPECT_FALSE(sftp_client.pull(source_path, target_path)); } @@ -739,19 +747,28 @@ TEST_F(SFTPClient, pull_dir_success_regular) return source_path == path ? get_dummy_sftp_attr(SSH_FILEXFER_TYPE_DIRECTORY, "", perms) : get_dummy_sftp_attr(SSH_FILEXFER_TYPE_REGULAR, "", perms); }); - fs::perms file_written_perms; - fs::perms dir_written_perms; - EXPECT_CALL(*mock_file_ops, permissions(target_path / "file", static_cast(perms), _)) - .WillOnce([&](auto, auto perms, auto) { file_written_perms = perms; }); - EXPECT_CALL(*mock_file_ops, permissions(target_path, static_cast(perms), _)) - .WillOnce([&](auto, auto perms, auto) { dir_written_perms = perms; }); + mp::Perms file_written_perms; + mp::Perms dir_written_perms; + EXPECT_CALL( + mock_platform, + set_permissions(QString::fromStdWString((target_path / "file").wstring()), static_cast(perms))) + .WillOnce([&](auto, auto perms) { + file_written_perms = perms; + return true; + }); + EXPECT_CALL(mock_platform, + set_permissions(QString::fromStdWString(target_path.wstring()), static_cast(perms))) + .WillOnce([&](auto, auto perms) { + dir_written_perms = perms; + return true; + }); auto sftp_client = make_sftp_client(); EXPECT_TRUE(sftp_client.pull(source_path, target_path, mp::SFTPClient::Flag::Recursive)); EXPECT_EQ(test_data, test_file.str()); - EXPECT_EQ(static_cast(perms), file_written_perms); - EXPECT_EQ(static_cast(perms), dir_written_perms); + EXPECT_EQ(static_cast(perms), file_written_perms); + EXPECT_EQ(static_cast(perms), dir_written_perms); } TEST_F(SFTPClient, pull_dir_success_dir) @@ -768,7 +785,7 @@ TEST_F(SFTPClient, pull_dir_success_dir) .WillOnce(Return(std::unique_ptr( get_dummy_sftp_attr(SSH_FILEXFER_TYPE_DIRECTORY, "source/path/dir")))); EXPECT_CALL(*mock_file_ops, create_directory(target_path / "dir", _)); - EXPECT_CALL(*mock_file_ops, permissions(_, _, _)).Times(2); + EXPECT_CALL(mock_platform, set_permissions(_, _)).Times(2).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -794,14 +811,14 @@ TEST_F(SFTPClient, pull_dir_fail_dir) e = err; return false; }); - EXPECT_CALL(*mock_file_ops, permissions(_, _, _)); + EXPECT_CALL(mock_platform, set_permissions(_, _)).WillOnce(Return(false)); auto sftp_client = make_sftp_client(); mock_logger->expect_log(mpl::Level::error, fmt::format("cannot create local directory {}: {}", target_path / "dir", err.message())); - mock_logger->expect_log(mpl::Level::error, fmt::format("cannot set permissions for local directory {}: {}", - target_path, err.message())); + mock_logger->expect_log(mpl::Level::error, + fmt::format("cannot set permissions for local directory {}", target_path)); EXPECT_FALSE(sftp_client.pull(source_path, target_path, mp::SFTPClient::Flag::Recursive)); } @@ -823,7 +840,7 @@ TEST_F(SFTPClient, pull_dir_success_symlink) EXPECT_CALL(*mock_file_ops, is_directory).WillOnce(Return(false)); EXPECT_CALL(*mock_file_ops, remove(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, create_symlink(_, target_path / "symlink", _)); - EXPECT_CALL(*mock_file_ops, permissions(_, _, _)); + EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -847,7 +864,7 @@ TEST_F(SFTPClient, pull_dir_cannot_read_symlink) REPLACE(sftp_readlink, [](auto...) { return nullptr; }); auto err = "SFTP server: Permission denied"; REPLACE(ssh_get_error, [&](auto...) { return err; }); - EXPECT_CALL(*mock_file_ops, permissions(_, _, _)); + EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -876,7 +893,7 @@ TEST_F(SFTPClient, pull_dir_cannot_create_symlink) auto err = std::make_error_code(std::errc::permission_denied); EXPECT_CALL(*mock_file_ops, create_symlink(_, target_path / "symlink", _)) .WillOnce([&](auto, auto, std::error_code& e) { e = err; }); - EXPECT_CALL(*mock_file_ops, permissions(_, _, _)).WillOnce([](auto, auto, auto& err) { err.clear(); }); + EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -901,7 +918,7 @@ TEST_F(SFTPClient, pull_dir_symlink_over_dir) REPLACE(sftp_readlink, [](auto...) { return (char*)malloc(10); }); EXPECT_CALL(*mock_file_ops, is_directory).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, permissions(_, _, _)); + EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -923,7 +940,7 @@ TEST_F(SFTPClient, pull_dir_unknown_file_type) EXPECT_CALL(*iter_p, next) .WillOnce(Return(std::unique_ptr( get_dummy_sftp_attr(SSH_FILEXFER_TYPE_UNKNOWN, source_path.u8string() + "/unknown")))); - EXPECT_CALL(*mock_file_ops, permissions(_, _, _)); + EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); diff --git a/tests/test_sftpserver.cpp b/tests/test_sftpserver.cpp index e912f71746..0d74d13e7c 100644 --- a/tests/test_sftpserver.cpp +++ b/tests/test_sftpserver.cpp @@ -544,9 +544,9 @@ TEST_F(SftpServer, handles_mkdir) msg->attr = &attr; const auto [file_ops, mock_file_ops_guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*file_ops, permissions(A(), _, _)).WillOnce([](auto, auto, std::error_code& err) { - err.clear(); - }); + const auto [platform, mock_platform_guard] = mpt::MockPlatform::inject(); + + EXPECT_CALL(*platform, set_permissions(A(), _)).WillOnce(Return(true)); EXPECT_CALL(*file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); EXPECT_CALL(*file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); @@ -600,8 +600,8 @@ TEST_F(SftpServer, mkdir_set_permissions_fails) auto new_dir_name = name_as_char_array(new_dir); const auto [file_ops, mock_file_ops_guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*file_ops, permissions(_, _, _)) - .WillOnce(SetArgReferee<2>(std::make_error_code(std::errc::operation_not_permitted))); + const auto [platform, mock_platform_guard] = mpt::MockPlatform::inject(); + EXPECT_CALL(*platform, set_permissions(_, _)).WillOnce(Return(false)); EXPECT_CALL(*file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); EXPECT_CALL(*file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); @@ -636,6 +636,7 @@ TEST_F(SftpServer, mkdir_chown_failure_fails) const auto [mock_platform, guard] = mpt::MockPlatform::inject(); EXPECT_CALL(*mock_platform, chown(_, _, _)).WillOnce(Return(-1)); + EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_MKDIR); @@ -1930,9 +1931,9 @@ TEST_F(SftpServer, setstat_set_permissions_failure_fails) msg->flags = SSH_FXF_WRITE; const auto [file_ops, mock_file_ops_guard] = mpt::MockFileOps::inject(); + const auto [platform, mock_platform_guard] = mpt::MockPlatform::inject(); EXPECT_CALL(*file_ops, resize).WillOnce(Return(true)); - EXPECT_CALL(*file_ops, permissions(_, _, _)) - .WillOnce(SetArgReferee<2>(std::make_error_code(std::errc::permission_denied))); + EXPECT_CALL(*platform, set_permissions(_, _)).WillOnce(Return(false)); EXPECT_CALL(*file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); EXPECT_CALL(*file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); EXPECT_CALL(*file_ops, exists(A())).WillRepeatedly([](const QFileInfo& file) { @@ -2704,6 +2705,7 @@ TEST_F(SftpServer, DISABLE_ON_WINDOWS(mkdir_chown_honors_maps_in_the_host)) EXPECT_CALL(*mock_platform, chown(_, host_uid, host_gid)).Times(1); EXPECT_CALL(*mock_platform, chown(_, sftp_uid, sftp_gid)).Times(0); + EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); sftp.run(); } @@ -2730,6 +2732,7 @@ TEST_F(SftpServer, DISABLE_ON_WINDOWS(mkdir_chown_works_when_ids_are_not_mapped) QFileInfo parent_dir(temp_dir.path()); EXPECT_CALL(*mock_platform, chown(_, parent_dir.ownerId(), parent_dir.groupId())).Times(1); + EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); sftp.run(); } diff --git a/tests/unix/test_platform_unix.cpp b/tests/unix/test_platform_unix.cpp index e5cf16a5a2..c783d2d109 100644 --- a/tests/unix/test_platform_unix.cpp +++ b/tests/unix/test_platform_unix.cpp @@ -114,7 +114,7 @@ TEST_F(TestPlatformUnix, setServerSocketRestrictionsChmodFailsThrows) MP_EXPECT_THROW_THAT( MP_PLATFORM.Platform::set_server_socket_restrictions(fmt::format("unix:{}", file.name()), false), std::runtime_error, - mpt::match_what(StrEq("Could not set permissions for the multipass socket: Operation not permitted"))); + mpt::match_what(StrEq("Could not set permissions for the multipass socket"))); } TEST_F(TestPlatformUnix, chmodSetsFileModsAndReturns)