diff --git a/src/core/appdir.cpp b/src/core/appdir.cpp index fb52b38..9807e33 100644 --- a/src/core/appdir.cpp +++ b/src/core/appdir.cpp @@ -30,6 +30,68 @@ using namespace linuxdeploy::core::log; using namespace cimg_library; namespace bf = boost::filesystem; +namespace { + // equivalent to 0644 + constexpr bf::perms DEFAULT_PERMS = bf::owner_write | bf::owner_read | bf::group_read | bf::others_read; + // equivalent to 0755 + constexpr bf::perms EXECUTABLE_PERMS = DEFAULT_PERMS | bf::owner_exe | bf::group_exe | bf::others_exe; + + class CopyOperation { + public: + bf::path fromPath; + bf::path toPath; + bf::perms addedPermissions; + }; + + typedef std::map CopyOperationsMap; + + /** + * Stores copy operations. + * This way, the storage logic does not have to be known to the using class. + */ + class CopyOperationsStorage { + private: + // using a map to make sure every target path is there only once + CopyOperationsMap _storedOperations; + + public: + CopyOperationsStorage() = default; + + /** + * Add copy operation. + * @param fromPath path to copy from + * @param toPath path to copy to + * @param addedPermissions permissions to add to the file's permissions + */ + void addOperation(const bf::path& fromPath, const bf::path& toPath, const bf::perms addedPermissions) { + CopyOperation operation{fromPath, toPath, addedPermissions}; + _storedOperations[fromPath] = operation; + } + + /** + * Export operations. + * @return vector containing all operations (random order). + */ + std::vector getOperations() { + std::vector operations; + operations.reserve(_storedOperations.size()); + + for (const auto& operationsPair : _storedOperations) { + operations.emplace_back(operationsPair.second); + } + + return operations; + } + + /** + * Clear internal storage. + */ + void clear() { + _storedOperations.clear(); + } + }; +} + namespace linuxdeploy { namespace core { namespace appdir { @@ -39,7 +101,7 @@ namespace linuxdeploy { // store deferred operations // these can be executed by calling excuteDeferredOperations - std::map copyOperations; + CopyOperationsStorage copyOperationsStorage; std::set stripOperations; std::map setElfRPathOperations; @@ -59,7 +121,7 @@ namespace linuxdeploy { bool disableCopyrightFilesDeployment = false; public: - PrivateData() : copyOperations(), stripOperations(), setElfRPathOperations(), visitedFiles(), appDirPath() { + PrivateData() : copyOperationsStorage(), stripOperations(), setElfRPathOperations(), visitedFiles(), appDirPath() { copyrightFilesManager = copyright::ICopyrightFilesManager::getInstance(); }; @@ -83,7 +145,8 @@ namespace linuxdeploy { // actually copy file // mimics cp command behavior - static bool copyFile(const bf::path& from, bf::path to, bool overwrite = false) { + // also adds minimum file permissions (by default adds 0644 to existing permissions) + static bool copyFile(const bf::path& from, bf::path to, bf::perms addedPerms, bool overwrite = false) { ldLog() << "Copying file" << from << "to" << to << std::endl; try { @@ -101,6 +164,7 @@ namespace linuxdeploy { } bf::copy_file(from, to, bf::copy_option::overwrite_if_exists); + bf::permissions(to, addedPerms | bf::add_perms); } catch (const bf::filesystem_error& e) { ldLog() << LD_ERROR << "Failed to copy file" << from << "to" << to << LD_NO_SPACE << ":" << e.what() << std::endl; return false; @@ -159,16 +223,13 @@ namespace linuxdeploy { bool executeDeferredOperations() { bool success = true; - while (!copyOperations.empty()) { - const auto& pair = *(copyOperations.begin()); - const auto& from = pair.first; - const auto& to = pair.second; - - if (!copyFile(from, to)) + const auto copyOperations = copyOperationsStorage.getOperations(); + std::for_each(copyOperations.begin(), copyOperations.end(), [&success](const CopyOperation& operation) { + if (!copyFile(operation.fromPath, operation.toPath, operation.addedPermissions)) { success = false; - - copyOperations.erase(copyOperations.begin()); - } + } + }); + copyOperationsStorage.clear(); if (!success) return false; @@ -257,7 +318,7 @@ namespace linuxdeploy { for (const auto& file : copyrightFiles) { std::string targetDir = file.string(); targetDir.erase(0, 1); - deployFile(file, appDirPath / targetDir); + deployFile(file, appDirPath / targetDir, DEFAULT_PERMS); } return true; @@ -267,7 +328,7 @@ namespace linuxdeploy { // by compiling a list of files to copy instead of just copying everything, one can ensure that // the files are touched once only // returns the full path of the deployment destination (useful if to is a directory - bf::path deployFile(const bf::path& from, bf::path to, bool verbose = false) { + bf::path deployFile(const bf::path& from, bf::path to, bf::perms addedPerms, bool verbose = false) { // not sure whether this is 100% bullet proof, but it simulates the cp command behavior if (to.string().back() == '/' || bf::is_directory(to)) { to /= from.filename(); @@ -276,7 +337,7 @@ namespace linuxdeploy { if (verbose) ldLog() << "Deploying file" << from << "to" << to << std::endl; - copyOperations[from] = to; + copyOperationsStorage.addOperation(from, to, addedPerms); // mark file as visited visitedFiles.insert(from); @@ -378,7 +439,7 @@ namespace linuxdeploy { } // in case destinationPath is a directory, deployFile will give us the deployed file's path - actualDestination = deployFile(path, actualDestination); + actualDestination = deployFile(path, actualDestination, DEFAULT_PERMS); deployCopyrightFiles(path); std::string rpath = "$ORIGIN"; @@ -426,7 +487,7 @@ namespace linuxdeploy { auto destinationPath = destination.empty() ? appDirPath / "usr/bin/" : destination; - deployFile(path, destinationPath); + deployFile(path, destinationPath, EXECUTABLE_PERMS); deployCopyrightFiles(path); std::string rpath = "$ORIGIN/../" + getLibraryDirName(path); @@ -468,7 +529,7 @@ namespace linuxdeploy { ldLog() << "Deploying desktop file" << desktopFile.path() << std::endl; - deployFile(desktopFile.path(), appDirPath / "usr/share/applications/"); + deployFile(desktopFile.path(), appDirPath / "usr/share/applications/", DEFAULT_PERMS); return true; } @@ -553,7 +614,7 @@ namespace linuxdeploy { } } - deployFile(path, appDirPath / "usr/share/icons/hicolor" / resolution / "apps" / filename); + deployFile(path, appDirPath / "usr/share/icons/hicolor" / resolution / "apps" / filename, DEFAULT_PERMS); deployCopyrightFiles(path); return true; @@ -702,11 +763,11 @@ namespace linuxdeploy { } bf::path AppDir::deployFile(const boost::filesystem::path& from, const boost::filesystem::path& to) { - return d->deployFile(from, to, true); + return d->deployFile(from, to, DEFAULT_PERMS, true); } bool AppDir::copyFile(const bf::path& from, const bf::path& to, bool overwrite) const { - return d->copyFile(from, to, overwrite); + return d->copyFile(from, to, DEFAULT_PERMS, overwrite); } bool AppDir::createRelativeSymlink(const bf::path& target, const bf::path& symlink) const { diff --git a/tests/core/test_appdir.cpp b/tests/core/test_appdir.cpp index 5a052de..1f05a50 100644 --- a/tests/core/test_appdir.cpp +++ b/tests/core/test_appdir.cpp @@ -5,6 +5,23 @@ using namespace linuxdeploy::core::appdir; using namespace linuxdeploy::desktopfile; using namespace boost::filesystem; +namespace { + void assertIsRegularFile(const path& pathToCheck) { + ASSERT_TRUE(is_regular_file(pathToCheck)); + EXPECT_TRUE(status(pathToCheck).permissions() >= 0644); + } + + void assertIsExecutableFile(const path& pathToCheck) { + assertIsRegularFile(pathToCheck); + EXPECT_TRUE(status(pathToCheck).permissions() >= 0755); + } + + void assertIsSymlink(const path& targetPath, const path& symlinkPath) { + const auto resolvedPath = read_symlink(symlinkPath); + EXPECT_TRUE(resolvedPath == targetPath) << resolvedPath << " " << targetPath; + } +} + namespace AppDirTest { class AppDirUnitTestsFixture : public ::testing::Test { public: @@ -86,16 +103,21 @@ namespace AppDirTest { appDir.deployExecutable(SIMPLE_EXECUTABLE_PATH); appDir.executeDeferredOperations(); - ASSERT_TRUE(is_regular_file(tmpAppDir / "usr/bin" / path(SIMPLE_EXECUTABLE_PATH).filename())); - ASSERT_TRUE(is_regular_file(tmpAppDir / "usr/lib" / path(SIMPLE_LIBRARY_PATH).filename())); + const auto binaryTargetPath = tmpAppDir / "usr/bin" / path(SIMPLE_EXECUTABLE_PATH).filename(); + const auto libTargetPath = tmpAppDir / "usr/lib" / path(SIMPLE_LIBRARY_PATH).filename(); + + assertIsExecutableFile(binaryTargetPath); + assertIsRegularFile(libTargetPath); } TEST_F(AppDirUnitTestsFixture, deployDesktopFile) { - DesktopFile desktopFile{SIMPLE_DESKTOP_ENTRY_PATH}; + const DesktopFile desktopFile{SIMPLE_DESKTOP_ENTRY_PATH}; appDir.deployDesktopFile(desktopFile); appDir.executeDeferredOperations(); - ASSERT_TRUE(is_regular_file(tmpAppDir / "usr/share/applications" / path(SIMPLE_DESKTOP_ENTRY_PATH).filename())); + const auto targetPath = tmpAppDir / "usr/share/applications" / path(SIMPLE_DESKTOP_ENTRY_PATH).filename(); + + assertIsRegularFile(targetPath); } @@ -103,37 +125,38 @@ namespace AppDirTest { appDir.deployIcon(SIMPLE_ICON_PATH); appDir.executeDeferredOperations(); - ASSERT_TRUE(is_regular_file(tmpAppDir / "usr/share/icons/hicolor/scalable/apps" / path(SIMPLE_ICON_PATH).filename())); + const auto targetPath = tmpAppDir / "usr/share/icons/hicolor/scalable/apps" / path(SIMPLE_ICON_PATH).filename(); + assertIsRegularFile(targetPath); } TEST_F(AppDirUnitTestsFixture, deployFileToDirectory) { - auto destination = tmpAppDir / "usr/share/doc/simple_application/"; + const auto destination = tmpAppDir / "usr/share/doc/simple_application/"; appDir.deployFile(SIMPLE_FILE_PATH, destination); appDir.executeDeferredOperations(); - ASSERT_TRUE(is_regular_file(destination / path(SIMPLE_FILE_PATH).filename())); + const auto targetPath = destination / path(SIMPLE_FILE_PATH).filename(); + assertIsRegularFile(targetPath); } TEST_F(AppDirUnitTestsFixture, deployFileToAbsoluteFilePath) { - auto destination = tmpAppDir / "usr/share/doc/simple_application/test123"; + const auto destination = tmpAppDir / "usr/share/doc/simple_application/test123"; appDir.deployFile(SIMPLE_FILE_PATH, destination); appDir.executeDeferredOperations(); - ASSERT_TRUE(is_regular_file(destination)); + assertIsRegularFile(destination); } TEST_F(AppDirUnitTestsFixture, createSymlink) { - auto destination = tmpAppDir / "usr/share/doc/simple_application/test123"; + const auto destination = tmpAppDir / "usr/share/doc/simple_application/test123"; appDir.deployFile(SIMPLE_FILE_PATH, destination); appDir.executeDeferredOperations(); - ASSERT_TRUE(is_regular_file(destination)); + assertIsRegularFile(destination); - appDir.createRelativeSymlink(destination, tmpAppDir / "relative_link"); + const auto symlinkDestination = tmpAppDir / "relative_link"; + appDir.createRelativeSymlink(destination, symlinkDestination); - auto res = read_symlink(tmpAppDir / "relative_link"); - auto expected = relative(destination, tmpAppDir); - ASSERT_TRUE(res == expected); + assertIsSymlink(relative(destination, tmpAppDir), symlinkDestination); } }