From f260c5ec6ffad2316ef85d0fbe1c5e24b9fdc653 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Thu, 6 Mar 2025 13:54:54 +0100 Subject: [PATCH 01/42] Reintroduced file listing and navigation. --- .../include/backend/ssh_session_manager.hpp | 7 + .../source/backend/ssh_session_manager.cpp | 281 ++++++++++++------ .../frontend/terminal/executing_engine.hpp | 5 +- .../frontend/terminal/sftp_file_engine.hpp | 6 +- .../include/frontend/terminal/ssh_engine.hpp | 9 +- .../frontend/terminal/terminal_engine.hpp | 3 +- .../frontend/terminal/user_control_engine.hpp | 2 +- frontend/source/frontend/session.cpp | 34 +-- .../frontend/terminal/executing_engine.cpp | 2 +- .../frontend/terminal/sftp_file_engine.cpp | 68 +++-- .../source/frontend/terminal/ssh_engine.cpp | 73 +++-- .../frontend/terminal/user_control_engine.cpp | 2 +- ssh/include/ssh/sftp_error.hpp | 12 + 13 files changed, 320 insertions(+), 184 deletions(-) diff --git a/backend/include/backend/ssh_session_manager.hpp b/backend/include/backend/ssh_session_manager.hpp index 05288df7..77b4d702 100644 --- a/backend/include/backend/ssh_session_manager.hpp +++ b/backend/include/backend/ssh_session_manager.hpp @@ -21,6 +21,8 @@ class SshSessionManager { public: + constexpr static auto futureTimeout = std::chrono::seconds{10}; + SshSessionManager(); ~SshSessionManager(); SshSessionManager(SshSessionManager const&) = delete; @@ -47,11 +49,16 @@ class SshSessionManager void registerRpcChannelWrite(Nui::Window&, Nui::RpcHub& hub); void registerRpcChannelPtyResize(Nui::Window&, Nui::RpcHub& hub); + // Sftp: + void registerRpcSftpListDirectory(Nui::Window&, Nui::RpcHub& hub); + void registerRpcSftpCreateDirectory(Nui::Window&, Nui::RpcHub& hub); + private: std::mutex passwordProvidersMutex_{}; std::mutex addSessionMutex_{}; std::unordered_map, Ids::IdHash> sessions_{}; std::unordered_map, Ids::IdHash> channels_{}; + std::unordered_map, Ids::IdHash> sftpChannels_{}; std::map passwordProviders_{}; std::unique_ptr addSessionThread_{}; std::vector pwCache_{}; diff --git a/backend/source/backend/ssh_session_manager.cpp b/backend/source/backend/ssh_session_manager.cpp index 67f91769..0b1e4d48 100644 --- a/backend/source/backend/ssh_session_manager.cpp +++ b/backend/source/backend/ssh_session_manager.cpp @@ -153,24 +153,56 @@ void SshSessionManager::registerRpcCreateChannel(Nui::Window&, Nui::RpcHub& hub) return; } - const auto sessionOptions = parameters["engine"]["sshSessionOptions"].get(); - auto env = sessionOptions.environment; + bool fileMode = false; + if (parameters.contains("fileMode") && parameters["fileMode"].is_boolean()) + { + fileMode = parameters["fileMode"].get(); + } - const auto weakChannel = sessions_[sessionId]->createPtyChannel({.environment = env}).get(); - if (!weakChannel.has_value()) + if (!fileMode) { - Log::error("Failed to create pty channel: {}", weakChannel.error()); - hub->callRemote(responseId, nlohmann::json{{"error", "Failed to create pty channel"}}); - return; + const auto sessionOptions = + parameters["engine"]["sshSessionOptions"].get(); + auto env = sessionOptions.environment; + + const auto weakChannel = sessions_[sessionId]->createPtyChannel({.environment = env}).get(); + if (!weakChannel.has_value()) + { + Log::error("Failed to create pty channel: {}", weakChannel.error()); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to create pty channel"}}); + return; + } + + const auto channelId = Ids::generateChannelId(); + channels_.emplace(channelId, std::move(weakChannel).value()); + + Log::info( + "Created pty channel with id '{}', channel total is now '{}'.", + channelId.value(), + channels_.size()); + + hub->callRemote(responseId, nlohmann::json{{"id", channelId.value()}}); } + else + { + const auto weakChannel = sessions_[sessionId]->createSftpSession().get(); + if (!weakChannel.has_value()) + { + Log::error("Failed to create sftp channel: {}", weakChannel.error().toString()); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to create sftp channel"}}); + return; + } - const auto channelId = Ids::generateChannelId(); - channels_.emplace(channelId, std::move(weakChannel).value()); + const auto channelId = Ids::generateChannelId(); + sftpChannels_.emplace(channelId, std::move(weakChannel).value()); - Log::info( - "Created pty channel with id '{}', channel total is now '{}'.", channelId.value(), channels_.size()); + Log::info( + "Created sftp channel with id '{}', sftp channel total is now '{}'.", + channelId.value(), + sftpChannels_.size()); - hub->callRemote(responseId, nlohmann::json{{"id", channelId.value()}}); + hub->callRemote(responseId, nlohmann::json{{"id", channelId.value()}}); + } }); } @@ -277,6 +309,18 @@ void SshSessionManager::registerRpcChannelClose(Nui::Window&, Nui::RpcHub& hub) channelId.value(), channels_.size()); } + else if (auto iter = sftpChannels_.find(channelId); iter != sftpChannels_.end()) + { + if (auto channel = iter->second.lock(); channel) + { + channel->close(); + } + sftpChannels_.erase(iter); + Log::info( + "Closed sftp channel with id '{}', now remaining sftp channels total '{}'.", + channelId.value(), + sftpChannels_.size()); + } else { Log::error("No channel found with id: {}", channelId.value()); @@ -414,6 +458,134 @@ void SshSessionManager::registerRpcChannelPtyResize(Nui::Window&, Nui::RpcHub& h }); } +void SshSessionManager::registerRpcSftpListDirectory(Nui::Window&, Nui::RpcHub& hub) +{ + hub.registerFunction( + "SshSessionManager::sftp::listDirectory", + [this, hub = &hub]( + std::string const& responseId, + std::string const& sessionIdString, + std::string const& channelIdString, + std::string const& path) { + try + { + const auto sessionId = Ids::makeSessionId(sessionIdString); + const auto channelId = Ids::makeChannelId(channelIdString); + + if (sessions_.find(sessionId) == sessions_.end()) + { + Log::error("No session found with id: {}", sessionId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "No session found with id"}}); + return; + } + + auto channel = sftpChannels_.find(channelId); + if (channel == sftpChannels_.end()) + { + Log::error("No sftp channel found with id: {}", channelId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "No sftp channel found with id"}}); + return; + } + + auto locked = channel->second.lock(); + if (!locked) + { + Log::error("Failed to lock sftp channel with id: {}", channelId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to lock sftp channel"}}); + return; + } + + auto fut = locked->listDirectory(path); + if (fut.wait_for(futureTimeout) != std::future_status::ready) + { + Log::error("Failed to list directory: timeout"); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to list directory: timeout"}}); + return; + } + + const auto result = fut.get(); + if (!result.has_value()) + { + Log::error("Failed to list directory: {}", result.error().message); + hub->callRemote(responseId, nlohmann::json{{"error", result.error().message}}); + return; + } + + hub->callRemote(responseId, nlohmann::json{{"entries", *result}}); + } + catch (std::exception const& e) + { + Log::error("Error listing directory: {}", e.what()); + hub->callRemote(responseId, nlohmann::json{{"error", e.what()}}); + return; + } + }); +} + +void SshSessionManager::registerRpcSftpCreateDirectory(Nui::Window&, Nui::RpcHub& hub) +{ + hub.registerFunction( + "SshSessionManager::sftp::createDirectory", + [this, hub = &hub]( + std::string const& responseId, + std::string const& sessionIdString, + std::string const& channelIdString, + std::string const& path) { + try + { + const auto sessionId = Ids::makeSessionId(sessionIdString); + const auto channelId = Ids::makeChannelId(channelIdString); + + if (sessions_.find(sessionId) == sessions_.end()) + { + Log::error("No session found with id: {}", sessionId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "No session found with id"}}); + return; + } + + auto channel = sftpChannels_.find(channelId); + if (channel == sftpChannels_.end()) + { + Log::error("No sftp channel found with id: {}", channelId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "No sftp channel found with id"}}); + return; + } + + auto locked = channel->second.lock(); + if (!locked) + { + Log::error("Failed to lock sftp channel with id: {}", channelId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to lock sftp channel"}}); + return; + } + + auto fut = locked->createDirectory(path); + if (fut.wait_for(futureTimeout) != std::future_status::ready) + { + Log::error("Failed to list directory: timeout"); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to list directory: timeout"}}); + return; + } + + const auto result = fut.get(); + if (!result.has_value()) + { + Log::error("Failed to list directory: {}", result.error().message); + hub->callRemote(responseId, nlohmann::json{{"error", result.error().message}}); + return; + } + + hub->callRemote(responseId, nlohmann::json{{"success", true}}); + } + catch (std::exception const& e) + { + Log::error("Error listing directory: {}", e.what()); + hub->callRemote(responseId, nlohmann::json{{"error", e.what()}}); + return; + } + }); +} + void SshSessionManager::registerRpc(Nui::Window& wnd, Nui::RpcHub& hub) { registerRpcConnect(wnd, hub); @@ -423,90 +595,7 @@ void SshSessionManager::registerRpc(Nui::Window& wnd, Nui::RpcHub& hub) registerRpcEndSession(wnd, hub); registerRpcChannelWrite(wnd, hub); registerRpcChannelPtyResize(wnd, hub); - - // hub.registerFunction( - // "SshSessionManager::sftp::listDirectory", - // [this, hub = &hub](std::string const& responseId, std::string const& sessionIdString, std::string const& - // path) { - // try - // { - // const auto sessionId = Ids::makeSessionId(sessionIdString); - - // if (sessions_.find(sessionId) == sessions_.end()) - // { - // Log::error("No session found with id: {}", sessionId.value()); - // hub->callRemote(responseId, nlohmann::json{{"error", "No session found with id"}}); - // return; - // } - - // auto& session = sessions_[sessionId]; - // auto sftpSession = session->getSftpSession(); - // if (!sftpSession) - // { - // Log::error("Failed to create sftp session"); - // hub->callRemote(responseId, nlohmann::json{{"error", "Failed to create sftp session"}}); - // return; - // } - - // auto result = sftpSession->listDirectory(path); - // if (!result.has_value()) - // { - // Log::error("Failed to list directory: {}", result.error().message); - // hub->callRemote(responseId, nlohmann::json{{"error", result.error().message}}); - // return; - // } - - // hub->callRemote(responseId, nlohmann::json{{"entries", *result}}); - // } - // catch (std::exception const& e) - // { - // Log::error("Error listing directory: {}", e.what()); - // hub->callRemote(responseId, nlohmann::json{{"error", e.what()}}); - // return; - // } - // }); - - // hub.registerFunction( - // "SshSessionManager::sftp::createDirectory", - // [this, hub = &hub](std::string const& responseId, std::string const& sessionIdString, std::string const& - // path) { - // try - // { - // const auto sessionId = Ids::makeSessionId(sessionIdString); - - // if (sessions_.find(sessionId) == sessions_.end()) - // { - // Log::error("No session found with id: {}", sessionId.value()); - // hub->callRemote(responseId, nlohmann::json{{"error", "No session found with id"}}); - // return; - // } - - // auto& session = sessions_[sessionId]; - // auto sftpSession = session->getSftpSession(); - // if (!sftpSession) - // { - // Log::error("Failed to create sftp session"); - // hub->callRemote(responseId, nlohmann::json{{"error", "Failed to create sftp session"}}); - // return; - // } - - // auto result = sftpSession->createDirectory(path); - // if (result) - // { - // Log::error("Failed to create directory: {}", result->message); - // hub->callRemote(responseId, nlohmann::json{{"error", result->message}}); - // return; - // } - - // hub->callRemote(responseId, nlohmann::json{{"success", true}}); - // } - // catch (std::exception const& e) - // { - // Log::error("Error creating directory: {}", e.what()); - // hub->callRemote(responseId, nlohmann::json{{"error", e.what()}}); - // return; - // } - // }); + registerRpcSftpListDirectory(wnd, hub); } void SshSessionManager::addPasswordProvider(int priority, PasswordProvider* provider) diff --git a/frontend/include/frontend/terminal/executing_engine.hpp b/frontend/include/frontend/terminal/executing_engine.hpp index 2c62b8b0..c9b1fd11 100644 --- a/frontend/include/frontend/terminal/executing_engine.hpp +++ b/frontend/include/frontend/terminal/executing_engine.hpp @@ -8,9 +8,6 @@ #include #include -#include - -// TODO: https://devblogs.microsoft.com/commandline/windows-command-line-introducing-the-windows-pseudo-console-conpty/ class ExecutingTerminalEngine : public SingleChannelTerminalEngine { @@ -26,7 +23,7 @@ class ExecutingTerminalEngine : public SingleChannelTerminalEngine ExecutingTerminalEngine(Settings settings); ROAR_PIMPL_SPECIAL_FUNCTIONS(ExecutingTerminalEngine); - void open(std::function onOpen, bool fileMode = false) override; + void open(std::function onOpen) override; void dispose(std::function onDisposeComplete) override; void write(std::string const& data) override; void resize(int cols, int rows) override; diff --git a/frontend/include/frontend/terminal/sftp_file_engine.hpp b/frontend/include/frontend/terminal/sftp_file_engine.hpp index 5bd43346..9cca7d9e 100644 --- a/frontend/include/frontend/terminal/sftp_file_engine.hpp +++ b/frontend/include/frontend/terminal/sftp_file_engine.hpp @@ -6,7 +6,7 @@ class SftpFileEngine : public FileEngine { public: - SftpFileEngine(SshTerminalEngine::Settings settings); + SftpFileEngine(SshTerminalEngine* engine); ROAR_PIMPL_SPECIAL_FUNCTIONS(SftpFileEngine); void listDirectory( @@ -15,8 +15,10 @@ class SftpFileEngine : public FileEngine void dispose() override; void createDirectory(std::filesystem::path const& path, std::function onComplete) override; + std::optional release(); + private: - void lazyOpen(std::function const&)> const& onOpen); + void lazyOpen(std::function const&)> const& onOpen); private: struct Implementation; diff --git a/frontend/include/frontend/terminal/ssh_engine.hpp b/frontend/include/frontend/terminal/ssh_engine.hpp index cb84819d..bf99adb5 100644 --- a/frontend/include/frontend/terminal/ssh_engine.hpp +++ b/frontend/include/frontend/terminal/ssh_engine.hpp @@ -27,12 +27,13 @@ class SshTerminalEngine : public MultiChannelTerminalEngine SshTerminalEngine(Settings settings); ROAR_PIMPL_SPECIAL_FUNCTIONS(SshTerminalEngine); - void open(std::function onOpen, bool fileMode = false) override; + void open(std::function onOpen) override; void createChannel( std::function handler, std::function errorHandler, std::function const&)> onCreated) override; + void createSftpChannel(std::function const&)> onCreated) override; void closeChannel(Ids::ChannelId const& channelId, std::function onChannelClosed = []() {}) override; SshChannel* channel(Ids::ChannelId const& channelId) override; @@ -51,6 +52,12 @@ class SshTerminalEngine : public MultiChannelTerminalEngine // Usually indicates that the entire session is closed void onChannelDeath(); + void createChannelImpl( + std::function handler, + std::function errorHandler, + std::function const&)> onCreated, + bool fileMode); + private: struct Implementation; std::unique_ptr impl_; diff --git a/frontend/include/frontend/terminal/terminal_engine.hpp b/frontend/include/frontend/terminal/terminal_engine.hpp index 6571e9a6..09e391a4 100644 --- a/frontend/include/frontend/terminal/terminal_engine.hpp +++ b/frontend/include/frontend/terminal/terminal_engine.hpp @@ -9,7 +9,7 @@ class TerminalEngine { public: - virtual void open(std::function onOpen, bool fileMode = false) = 0; + virtual void open(std::function onOpen) = 0; virtual std::string engineName() const = 0; virtual void dispose(std::function onDisposeComplete) = 0; virtual ~TerminalEngine() = default; @@ -22,6 +22,7 @@ class MultiChannelTerminalEngine : public TerminalEngine std::function handler, std::function errorHandler, std::function const&)> onCreated) = 0; + virtual void createSftpChannel(std::function const&)> onCreated) = 0; virtual void closeChannel(Ids::ChannelId const& channelId, std::function onClose = []() {}) = 0; virtual ChannelInterface* channel(Ids::ChannelId const& channelId) = 0; }; diff --git a/frontend/include/frontend/terminal/user_control_engine.hpp b/frontend/include/frontend/terminal/user_control_engine.hpp index 720a2a0e..0bad9cd7 100644 --- a/frontend/include/frontend/terminal/user_control_engine.hpp +++ b/frontend/include/frontend/terminal/user_control_engine.hpp @@ -23,7 +23,7 @@ class UserControlEngine : public SingleChannelTerminalEngine UserControlEngine(Settings settings); ROAR_PIMPL_SPECIAL_FUNCTIONS(UserControlEngine); - void open(std::function onOpen, bool) override; + void open(std::function onOpen) override; void dispose(std::function onDisposeComplete) override; void write(std::string const& data) override; void resize(int cols, int rows) override; diff --git a/frontend/source/frontend/session.cpp b/frontend/source/frontend/session.cpp index 7058c685..958919ed 100644 --- a/frontend/source/frontend/session.cpp +++ b/frontend/source/frontend/session.cpp @@ -216,7 +216,6 @@ void Session::setupFileGrid() Log::info("New item requested: {}", static_cast(type)); if (type == NuiFileExplorer::FileGrid::Item::Type::Directory) { - // impl_->fileEngine->createDirectory(impl_->currentPath); impl_->newItemAskDialog->open( "New directory", "Enter the name of the new directory", @@ -369,24 +368,21 @@ void Session::navigateTo(std::filesystem::path path) void Session::openSftp() { - Log::warn("SFTP is not implemented yet"); - // if (impl_->terminal.value() && impl_->terminal.value()->engine().engineName() == "ssh") - // { - // auto const& opts = std::get(impl_->engine.engine).sshSessionOptions.value(); - // if (opts.openSftpByDefault) - // { - // Log::info("Opening SFTP by default"); - // impl_->fileEngine = std::make_unique(SshTerminalEngine::Settings{ - // .engineOptions = std::get(impl_->engine.engine), - // .onExit = std::bind(&Session::onFileExplorerConnectionClose, this), - // }); - // navigateTo(opts.defaultDirectory.value_or("/")); - // } - // } - // else - // { - // Log::error("Cannot open SFTP for non-ssh terminal"); - // } + if (impl_->terminal.value() && impl_->terminal.value()->engine().engineName() == "ssh") + { + auto const& opts = std::get(impl_->engine.engine).sshSessionOptions.value(); + if (opts.openSftpByDefault) + { + Log::info("Opening SFTP by default"); + impl_->fileEngine = + std::make_unique(static_cast(&impl_->terminal.value()->engine())); + navigateTo(opts.defaultDirectory.value_or("/")); + } + } + else + { + Log::info("Cannot open SFTP for non-ssh terminal"); + } } void Session::fallbackToUserControlEngine() diff --git a/frontend/source/frontend/terminal/executing_engine.cpp b/frontend/source/frontend/terminal/executing_engine.cpp index b1622a10..583291f0 100644 --- a/frontend/source/frontend/terminal/executing_engine.cpp +++ b/frontend/source/frontend/terminal/executing_engine.cpp @@ -48,7 +48,7 @@ ExecutingTerminalEngine::~ExecutingTerminalEngine() ROAR_PIMPL_SPECIAL_FUNCTIONS_IMPL_NO_DTOR(ExecutingTerminalEngine); -void ExecutingTerminalEngine::open(std::function onOpen, bool) +void ExecutingTerminalEngine::open(std::function onOpen) { impl_->stdoutReceiver = Nui::RpcClient::autoRegisterFunction("execTerminalStdout_" + impl_->id, [this](Nui::val val) { diff --git a/frontend/source/frontend/terminal/sftp_file_engine.cpp b/frontend/source/frontend/terminal/sftp_file_engine.cpp index 5daa101f..206da056 100644 --- a/frontend/source/frontend/terminal/sftp_file_engine.cpp +++ b/frontend/source/frontend/terminal/sftp_file_engine.cpp @@ -6,67 +6,69 @@ struct SftpFileEngine::Implementation { - SshTerminalEngine underlyingEngine; bool wasDisposed = false; + SshTerminalEngine* engine; + std::optional sftpChannelId{std::nullopt}; - Implementation(SshTerminalEngine::Settings settings) - : underlyingEngine{std::move(settings)} + Implementation(SshTerminalEngine* engine) + : engine{engine} {} }; -SftpFileEngine::SftpFileEngine(SshTerminalEngine::Settings settings) - : impl_{std::make_unique(std::move(settings))} +SftpFileEngine::SftpFileEngine(SshTerminalEngine* engine) + : impl_{std::make_unique(engine)} {} SftpFileEngine::~SftpFileEngine() { - if (!moveDetector_.wasMoved() && !impl_->wasDisposed) + if (!moveDetector_.wasMoved()) { dispose(); } } +std::optional SftpFileEngine::release() +{ + return std::move(impl_->sftpChannelId); +} + void SftpFileEngine::dispose() { - impl_->wasDisposed = true; - if (!impl_->underlyingEngine.sshSessionId().isValid()) + if (!impl_->wasDisposed) { - return; + if (impl_->sftpChannelId) + { + Log::info("Closing sftp channel"); + impl_->engine->closeChannel(impl_->sftpChannelId.value(), []() {}); + } } - impl_->underlyingEngine.dispose([]() {}); + impl_->wasDisposed = true; } ROAR_PIMPL_SPECIAL_FUNCTIONS_IMPL_NO_DTOR(SftpFileEngine); -void SftpFileEngine::lazyOpen(std::function const&)> const& onOpen) +void SftpFileEngine::lazyOpen(std::function const&)> const& onOpen) { - if (impl_->underlyingEngine.sshSessionId().isValid()) + if (impl_->sftpChannelId) { - onOpen(impl_->underlyingEngine.sshSessionId()); + onOpen(impl_->sftpChannelId); return; } - impl_->underlyingEngine.open( - [this, onOpen](bool success, std::string const& error) { - if (!success) - { - Log::error("Failed to open SFTP: {}", error); - onOpen(std::nullopt); - return; - } - - onOpen(impl_->underlyingEngine.sshSessionId()); - }, - true); + Log::info("Creating sftp channel"); + impl_->engine->createSftpChannel([this, onOpen](auto const& id) { + impl_->sftpChannelId = id; + onOpen(id); + }); } void SftpFileEngine::listDirectory( std::filesystem::path const& path, std::function> const&)> onComplete) { - lazyOpen([path, onComplete = std::move(onComplete)](auto const& sshSessionId) { - if (!sshSessionId) + lazyOpen([this, path, onComplete = std::move(onComplete)](auto const& channelId) { + if (!channelId) { - Log::error("Cannot list directory, no ssh session"); + Log::error("Cannot list directory, no sftp channel"); return; } @@ -92,15 +94,16 @@ void SftpFileEngine::listDirectory( onComplete(nlohmann::json::parse(Nui::JSON::stringify(val))["entries"]); }, - sshSessionId->value(), + impl_->engine->sshSessionId().value(), + channelId.value().value(), path.generic_string()); }); } void SftpFileEngine::createDirectory(std::filesystem::path const& path, std::function onComplete) { - lazyOpen([path, onComplete = std::move(onComplete)](auto const& sshSessionId) { - if (!sshSessionId) + lazyOpen([this, path, onComplete = std::move(onComplete)](auto const& channelId) { + if (!channelId) { Log::error("Cannot create directory, no ssh session"); return; @@ -121,7 +124,8 @@ void SftpFileEngine::createDirectory(std::filesystem::path const& path, std::fun onComplete(true); }, - sshSessionId->value(), + impl_->engine->sshSessionId().value(), + channelId.value().value(), path.generic_string()); }); } \ No newline at end of file diff --git a/frontend/source/frontend/terminal/ssh_engine.cpp b/frontend/source/frontend/terminal/ssh_engine.cpp index f0b7f2a4..55806745 100644 --- a/frontend/source/frontend/terminal/ssh_engine.cpp +++ b/frontend/source/frontend/terminal/ssh_engine.cpp @@ -14,7 +14,6 @@ struct SshTerminalEngine::Implementation Ids::SessionId sshSessionId; std::unordered_map channels; std::function disposer; - bool fileMode; bool wasDisposed; bool blockedByDestruction; @@ -23,7 +22,6 @@ struct SshTerminalEngine::Implementation , sshSessionId{} , channels{} , disposer{} - , fileMode{false} , wasDisposed{false} , blockedByDestruction{false} {} @@ -44,7 +42,7 @@ SshTerminalEngine::~SshTerminalEngine() ROAR_PIMPL_SPECIAL_FUNCTIONS_IMPL_NO_DTOR(SshTerminalEngine); -void SshTerminalEngine::open(std::function onOpen, bool fileMode) +void SshTerminalEngine::open(std::function onOpen) { if (impl_->blockedByDestruction) { @@ -52,8 +50,6 @@ void SshTerminalEngine::open(std::function onOpe return onOpen(false, "Blocked by destruction"); } - impl_->fileMode = fileMode; - Nui::val obj = Nui::val::object(); obj.set("engine", asVal(impl_->settings.engineOptions)); @@ -105,10 +101,11 @@ void SshTerminalEngine::onChannelDeath() dispose([]() {}); } -void SshTerminalEngine::createChannel( +void SshTerminalEngine::createChannelImpl( std::function handler, std::function errorHandler, - std::function const&)> onCreated) + std::function const&)> onCreated, + bool fileMode) { if (impl_->blockedByDestruction) { @@ -119,11 +116,15 @@ void SshTerminalEngine::createChannel( Nui::val obj = Nui::val::object(); obj.set("engine", asVal(impl_->settings.engineOptions)); obj.set("sessionId", impl_->sshSessionId.value()); + obj.set("fileMode", fileMode); Nui::RpcClient::callWithBackChannel( "SshSessionManager::Session::createChannel", - [this, onCreated = std::move(onCreated), handler = std::move(handler), errorHandler = std::move(errorHandler)]( - Nui::val val) { + [this, + onCreated = std::move(onCreated), + handler = std::move(handler), + errorHandler = std::move(errorHandler), + fileMode](Nui::val val) { if (val.hasOwnProperty("error")) { Log::error("Failed to create channel: {}", val["error"].as()); @@ -151,27 +152,47 @@ void SshTerminalEngine::createChannel( Log::info("Channel died, disconnecting entire session"); onChannelDeath(); }, - impl_->fileMode); - - Nui::RpcClient::callWithBackChannel( - "SshSessionManager::Channel::startReading", - [this, channelId, onCreated](Nui::val val) { - if (val.hasOwnProperty("error")) - { - Log::error("Failed to start reading: {}", val["error"].as()); - closeChannel(channelId, []() {}); - onCreated(std::nullopt); - return; - } - Log::info("Started reading: {}", channelId.value()); - onCreated(channelId); - }, - impl_->sshSessionId.value(), - channelId.value()); + fileMode); + + if (!fileMode) + { + Nui::RpcClient::callWithBackChannel( + "SshSessionManager::Channel::startReading", + [this, channelId, onCreated](Nui::val val) { + if (val.hasOwnProperty("error")) + { + Log::error("Failed to start reading: {}", val["error"].as()); + closeChannel(channelId, []() {}); + onCreated(std::nullopt); + return; + } + Log::info("Started reading: {}", channelId.value()); + onCreated(channelId); + }, + impl_->sshSessionId.value(), + channelId.value()); + } + else + { + onCreated(channelId); + } }, obj); } +void SshTerminalEngine::createChannel( + std::function handler, + std::function errorHandler, + std::function const&)> onCreated) +{ + createChannelImpl(std::move(handler), std::move(errorHandler), std::move(onCreated), false); +} + +void SshTerminalEngine::createSftpChannel(std::function const&)> onCreated) +{ + createChannelImpl([](std::string const&) {}, [](std::string const&) {}, std::move(onCreated), true); +} + Ids::SessionId SshTerminalEngine::sshSessionId() const { return impl_->sshSessionId; diff --git a/frontend/source/frontend/terminal/user_control_engine.cpp b/frontend/source/frontend/terminal/user_control_engine.cpp index 6029eef9..fa635e56 100644 --- a/frontend/source/frontend/terminal/user_control_engine.cpp +++ b/frontend/source/frontend/terminal/user_control_engine.cpp @@ -42,7 +42,7 @@ UserControlEngine::~UserControlEngine() ROAR_PIMPL_SPECIAL_FUNCTIONS_IMPL_NO_DTOR(UserControlEngine); -void UserControlEngine::open(std::function onOpen, bool) +void UserControlEngine::open(std::function onOpen) { onOpen(true, "Never Fails"); } diff --git a/ssh/include/ssh/sftp_error.hpp b/ssh/include/ssh/sftp_error.hpp index 11d09d77..c9039839 100644 --- a/ssh/include/ssh/sftp_error.hpp +++ b/ssh/include/ssh/sftp_error.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include namespace SecureShell @@ -21,5 +23,15 @@ namespace SecureShell int sshError = 0; int sftpError = 0; WrapperErrors wrapperError = WrapperErrors::None; + + inline std::string toString() const + { + return fmt::format( + "SftpError: message: {}, sshError: {}, sftpError: {}, wrapperError: {}", + message, + sshError, + sftpError, + static_cast(wrapperError)); + } }; } \ No newline at end of file From 6f8e1620960e3af9718f2b7142357e0248f953e2 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Thu, 6 Mar 2025 15:58:14 +0100 Subject: [PATCH 02/42] Added create file. --- .../include/backend/ssh_session_manager.hpp | 1 + .../source/backend/ssh_session_manager.cpp | 75 ++++++++++++++++++- .../include/frontend/terminal/file_engine.hpp | 1 + .../frontend/terminal/sftp_file_engine.hpp | 1 + frontend/source/frontend/session.cpp | 28 +++++++ .../frontend/terminal/sftp_file_engine.cpp | 32 +++++++- 6 files changed, 133 insertions(+), 5 deletions(-) diff --git a/backend/include/backend/ssh_session_manager.hpp b/backend/include/backend/ssh_session_manager.hpp index 77b4d702..58a0624d 100644 --- a/backend/include/backend/ssh_session_manager.hpp +++ b/backend/include/backend/ssh_session_manager.hpp @@ -52,6 +52,7 @@ class SshSessionManager // Sftp: void registerRpcSftpListDirectory(Nui::Window&, Nui::RpcHub& hub); void registerRpcSftpCreateDirectory(Nui::Window&, Nui::RpcHub& hub); + void registerRpcSftpCreateFile(Nui::Window&, Nui::RpcHub& hub); private: std::mutex passwordProvidersMutex_{}; diff --git a/backend/source/backend/ssh_session_manager.cpp b/backend/source/backend/ssh_session_manager.cpp index 0b1e4d48..bc2a7d49 100644 --- a/backend/source/backend/ssh_session_manager.cpp +++ b/backend/source/backend/ssh_session_manager.cpp @@ -562,15 +562,15 @@ void SshSessionManager::registerRpcSftpCreateDirectory(Nui::Window&, Nui::RpcHub auto fut = locked->createDirectory(path); if (fut.wait_for(futureTimeout) != std::future_status::ready) { - Log::error("Failed to list directory: timeout"); - hub->callRemote(responseId, nlohmann::json{{"error", "Failed to list directory: timeout"}}); + Log::error("Failed to create directory: timeout"); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to create directory: timeout"}}); return; } const auto result = fut.get(); if (!result.has_value()) { - Log::error("Failed to list directory: {}", result.error().message); + Log::error("Failed to create directory: {}", result.error().message); hub->callRemote(responseId, nlohmann::json{{"error", result.error().message}}); return; } @@ -579,13 +579,77 @@ void SshSessionManager::registerRpcSftpCreateDirectory(Nui::Window&, Nui::RpcHub } catch (std::exception const& e) { - Log::error("Error listing directory: {}", e.what()); + Log::error("Error creating directory: {}", e.what()); hub->callRemote(responseId, nlohmann::json{{"error", e.what()}}); return; } }); } +void SshSessionManager::registerRpcSftpCreateFile(Nui::Window&, Nui::RpcHub& hub) +{ + hub.registerFunction( + "SshSessionManager::sftp::createFile", + [this, hub = &hub]( + std::string const& responseId, + std::string const& sessionIdString, + std::string const& channelIdString, + std::string const& path) { + try + { + const auto sessionId = Ids::makeSessionId(sessionIdString); + const auto channelId = Ids::makeChannelId(channelIdString); + + if (sessions_.find(sessionId) == sessions_.end()) + { + Log::error("No session found with id: {}", sessionId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "No session found with id"}}); + return; + } + + auto channel = sftpChannels_.find(channelId); + if (channel == sftpChannels_.end()) + { + Log::error("No sftp channel found with id: {}", channelId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "No sftp channel found with id"}}); + return; + } + + auto locked = channel->second.lock(); + if (!locked) + { + Log::error("Failed to lock sftp channel with id: {}", channelId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to lock sftp channel"}}); + return; + } + + auto fut = locked->createFile(path); + if (fut.wait_for(futureTimeout) != std::future_status::ready) + { + Log::error("Failed to create file: timeout"); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to create file: timeout"}}); + return; + } + + const auto result = fut.get(); + if (!result.has_value()) + { + Log::error("Failed to create file: {}", result.error().message); + hub->callRemote(responseId, nlohmann::json{{"error", result.error().message}}); + return; + } + + hub->callRemote(responseId, nlohmann::json{{"success", true}}); + } + catch (std::exception const& exc) + { + Log::error("Error creating file: {}", exc.what()); + hub->callRemote(responseId, nlohmann::json{{"error", exc.what()}}); + return; + } + }); +} + void SshSessionManager::registerRpc(Nui::Window& wnd, Nui::RpcHub& hub) { registerRpcConnect(wnd, hub); @@ -595,7 +659,10 @@ void SshSessionManager::registerRpc(Nui::Window& wnd, Nui::RpcHub& hub) registerRpcEndSession(wnd, hub); registerRpcChannelWrite(wnd, hub); registerRpcChannelPtyResize(wnd, hub); + registerRpcSftpListDirectory(wnd, hub); + registerRpcSftpCreateDirectory(wnd, hub); + registerRpcSftpCreateFile(wnd, hub); } void SshSessionManager::addPasswordProvider(int priority, PasswordProvider* provider) diff --git a/frontend/include/frontend/terminal/file_engine.hpp b/frontend/include/frontend/terminal/file_engine.hpp index b66fbe9f..5961cc67 100644 --- a/frontend/include/frontend/terminal/file_engine.hpp +++ b/frontend/include/frontend/terminal/file_engine.hpp @@ -21,6 +21,7 @@ class FileEngine std::function> const&)> onComplete) = 0; virtual void createDirectory(std::filesystem::path const& path, std::function onComplete) = 0; + virtual void createFile(std::filesystem::path const& path, std::function onComplete) = 0; virtual void dispose() = 0; }; \ No newline at end of file diff --git a/frontend/include/frontend/terminal/sftp_file_engine.hpp b/frontend/include/frontend/terminal/sftp_file_engine.hpp index 9cca7d9e..9f86f43c 100644 --- a/frontend/include/frontend/terminal/sftp_file_engine.hpp +++ b/frontend/include/frontend/terminal/sftp_file_engine.hpp @@ -14,6 +14,7 @@ class SftpFileEngine : public FileEngine std::function> const&)> onComplete) override; void dispose() override; void createDirectory(std::filesystem::path const& path, std::function onComplete) override; + void createFile(std::filesystem::path const& path, std::function onComplete) override; std::optional release(); diff --git a/frontend/source/frontend/session.cpp b/frontend/source/frontend/session.cpp index 958919ed..c5908221 100644 --- a/frontend/source/frontend/session.cpp +++ b/frontend/source/frontend/session.cpp @@ -242,6 +242,34 @@ void Session::setupFileGrid() }); }); } + else if (type == NuiFileExplorer::FileGrid::Item::Type::Regular) + { + impl_->newItemAskDialog->open( + "New file", + "Enter the name of the new file", + "Create a new file", + false, + [this](std::optional const& name) { + if (!name) + return; + + Log::info("Creating new file: {}", *name); + if (name->find('/') != std::string::npos) + { + Log::error("Invalid file name (cannot contain slashes): {}", *name); + return; + } + impl_->fileEngine->createFile(impl_->currentPath / *name, [this](bool success) { + if (!success) + { + Log::error("Failed to create file"); + return; + } + // Refresh list from server: + navigateTo(impl_->currentPath); + }); + }); + } else { // TODO: create file diff --git a/frontend/source/frontend/terminal/sftp_file_engine.cpp b/frontend/source/frontend/terminal/sftp_file_engine.cpp index 206da056..f9001cb2 100644 --- a/frontend/source/frontend/terminal/sftp_file_engine.cpp +++ b/frontend/source/frontend/terminal/sftp_file_engine.cpp @@ -105,7 +105,7 @@ void SftpFileEngine::createDirectory(std::filesystem::path const& path, std::fun lazyOpen([this, path, onComplete = std::move(onComplete)](auto const& channelId) { if (!channelId) { - Log::error("Cannot create directory, no ssh session"); + Log::error("Cannot create directory, no channel"); return; } @@ -128,4 +128,34 @@ void SftpFileEngine::createDirectory(std::filesystem::path const& path, std::fun channelId.value().value(), path.generic_string()); }); +} + +void SftpFileEngine::createFile(std::filesystem::path const& path, std::function onComplete) +{ + lazyOpen([this, path, onComplete = std::move(onComplete)](auto const& channelId) { + if (!channelId) + { + Log::error("Cannot create file, no channel"); + return; + } + + Log::info("Creating file: {}", path.generic_string()); + Nui::RpcClient::callWithBackChannel( + "SshSessionManager::sftp::createFile", + [onComplete = std::move(onComplete)](Nui::val val) { + Nui::Console::log(val); + + if (val.hasOwnProperty("error")) + { + Log::error("(Frontend) Failed to create file: {}", val["error"].as()); + onComplete(false); + return; + } + + onComplete(true); + }, + impl_->engine->sshSessionId().value(), + channelId.value().value(), + path.generic_string()); + }); } \ No newline at end of file From 6a1108a961c1a3c40d5676286d88956a10a3bb71 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Fri, 7 Mar 2025 14:35:48 +0100 Subject: [PATCH 03/42] Improved channel shutdowns. --- ssh/include/ssh/async/processing_thread.hpp | 9 +++- ssh/include/ssh/channel.hpp | 2 + ssh/include/ssh/file_stream.hpp | 7 ++- ssh/include/ssh/sftp_session.hpp | 19 +++++-- ssh/source/ssh/async/processing_thread.cpp | 10 ++++ ssh/source/ssh/channel.cpp | 28 ++++++---- ssh/source/ssh/file_stream.cpp | 8 +++ ssh/source/ssh/session.cpp | 15 +++--- ssh/source/ssh/sftp_session.cpp | 48 ++++++++++++++--- ssh/test/ssh/main.cpp | 4 +- ssh/test/ssh/test_processing_thread.hpp | 17 ++++++ ssh/test/ssh/test_sftp.hpp | 58 ++++++++++++++------- ssh/test/ssh/test_ssh_session.hpp | 3 +- ssh/test/ssh/utility/node.cpp | 24 ++++++++- ssh/test/ssh/utility/node.hpp | 1 + 15 files changed, 199 insertions(+), 54 deletions(-) diff --git a/ssh/include/ssh/async/processing_thread.hpp b/ssh/include/ssh/async/processing_thread.hpp index 76c4edcd..94eadf9c 100644 --- a/ssh/include/ssh/async/processing_thread.hpp +++ b/ssh/include/ssh/async/processing_thread.hpp @@ -14,7 +14,7 @@ namespace SecureShell class ProcessingThread { public: - constexpr static unsigned int maximumTasksProcessableAtOnce = 10; + constexpr static unsigned int maximumTasksProcessableAtOnce = 100; struct PermanentTaskId { @@ -65,6 +65,13 @@ namespace SecureShell int permanentTaskCount() const; void clearPermanentTasks(); + bool awaitCycle(std::chrono::milliseconds maxWait = std::chrono::seconds{5}); + + bool withinProcessingThread() const + { + return processingThreadId_ == std::this_thread::get_id(); + } + private: void run(std::chrono::milliseconds const& waitCycleTimeout, std::chrono::milliseconds const& minimumCycleWait); diff --git a/ssh/include/ssh/channel.hpp b/ssh/include/ssh/channel.hpp index e7e0959f..d0af51e3 100644 --- a/ssh/include/ssh/channel.hpp +++ b/ssh/include/ssh/channel.hpp @@ -28,6 +28,8 @@ namespace SecureShell return *channel_; } + void shutdown(); + void close(); /** diff --git a/ssh/include/ssh/file_stream.hpp b/ssh/include/ssh/file_stream.hpp index b3daa1dd..dc6c4a81 100644 --- a/ssh/include/ssh/file_stream.hpp +++ b/ssh/include/ssh/file_stream.hpp @@ -14,7 +14,7 @@ namespace SecureShell class Session; class SftpSession; - class FileStream + class FileStream : public std::enable_shared_from_this { public: FileStream(std::shared_ptr sftp, sftp_file file, sftp_limits_struct limits); @@ -95,6 +95,11 @@ namespace SecureShell */ sftp_file release(); + /** + * @brief Closes the file and removes itself from the sftp session. + */ + void close(); + private: std::function makeFileDeleter(); diff --git a/ssh/include/ssh/sftp_session.hpp b/ssh/include/ssh/sftp_session.hpp index 1d437eb4..8e9085e0 100644 --- a/ssh/include/ssh/sftp_session.hpp +++ b/ssh/include/ssh/sftp_session.hpp @@ -25,6 +25,7 @@ namespace SecureShell { public: using Error = SftpError; + friend class FileStream; SftpSession(Session* owner, sftp_session session); ~SftpSession(); @@ -38,7 +39,7 @@ namespace SecureShell return session_; } - void close(); + void close(bool removeSelf = true); struct DirectoryEntry : public SharedData::DirectoryEntry { @@ -70,6 +71,14 @@ namespace SecureShell return promise.get_future(); } + bool awaitCycle() + { + std::scoped_lock lock{ownerMutex_}; + if (owner_) + return owner_->processingThread_.awaitCycle(); + return false; + } + /** * @brief Retrieves the last error that occurred. May contain success. */ @@ -174,15 +183,19 @@ namespace SecureShell Exclusive = O_EXCL, }; - std::future> + std::future, Error>> openFile(std::filesystem::path const& path, OpenType openType, std::filesystem::perms permissions); std::future> limits(); + private: + void fileStreamRemoveItself(FileStream* stream); + void removeAllFileStreams(); + private: std::mutex ownerMutex_; Session* owner_; sftp_session session_; - // std::optional readTaskId_{std::nullopt}; + std::vector> fileStreams_; }; } \ No newline at end of file diff --git a/ssh/source/ssh/async/processing_thread.cpp b/ssh/source/ssh/async/processing_thread.cpp index 46208fb7..bdadf8ed 100644 --- a/ssh/source/ssh/async/processing_thread.cpp +++ b/ssh/source/ssh/async/processing_thread.cpp @@ -224,4 +224,14 @@ namespace SecureShell std::lock_guard lock{taskMutex_}; return permanentTasks_.size(); } + bool ProcessingThread::awaitCycle(std::chrono::milliseconds maxWait) + { + if (!withinProcessingThread() && running_) + { + return pushPromiseTask([]() { + return true; + }).wait_for(maxWait) == std::future_status::ready; + } + return false; + } } \ No newline at end of file diff --git a/ssh/source/ssh/channel.cpp b/ssh/source/ssh/channel.cpp index 397a1458..41b01656 100644 --- a/ssh/source/ssh/channel.cpp +++ b/ssh/source/ssh/channel.cpp @@ -24,19 +24,29 @@ namespace SecureShell } return *this; } - void Channel::close() + void Channel::shutdown() { - std::scoped_lock lock{ownerMutex_}; - if (owner_) { - if (readTaskId_) - { + std::scoped_lock lock{ownerMutex_}; + if (owner_ && readTaskId_) owner_->processingThread_.removePermanentTask(*readTaskId_); - } - - owner_->channelRemoveItself(this); - owner_ = nullptr; } + if (channel_ && channel_->isOpen()) + { + channel_->sendEof(); + channel_->close(); + } + channel_.reset(); + } + void Channel::close() + { + shutdown(); + decltype(owner_) cpy = nullptr; + { + std::scoped_lock lock{ownerMutex_}; + cpy = std::exchange(owner_, nullptr); + } + cpy->channelRemoveItself(this); } void Channel::write(std::string data) { diff --git a/ssh/source/ssh/file_stream.cpp b/ssh/source/ssh/file_stream.cpp index edc34de6..585a23e5 100644 --- a/ssh/source/ssh/file_stream.cpp +++ b/ssh/source/ssh/file_stream.cpp @@ -38,11 +38,19 @@ namespace SecureShell , file_{other.file_.release(), makeFileDeleter()} , limits_{other.limits_} {} + void FileStream::close() + { + if (auto sftp = sftp_.lock(); sftp) + { + sftp->fileStreamRemoveItself(this); + } + } std::function FileStream::makeFileDeleter() { return [this](sftp_file file) { if (auto sftp = this->sftp_.lock(); sftp) { + // This is safe because file is just a pointer, even if 'this' is deleted by this point. sftp->perform([file]() { sftp_close(file); }); diff --git a/ssh/source/ssh/session.cpp b/ssh/source/ssh/session.cpp index 1917896f..ec9ea429 100644 --- a/ssh/source/ssh/session.cpp +++ b/ssh/source/ssh/session.cpp @@ -66,9 +66,7 @@ namespace SecureShell { channelsToRemove_.push_back(*it); channels_.erase(it); - processingThread_.pushTask([this]() { - removalTask(); - }); + removalTask(); } else { @@ -82,12 +80,7 @@ namespace SecureShell { for (const auto& toRemove : channelsToRemove_) { - auto& channel = static_cast(*toRemove); - if (channel.isOpen()) - { - channel.sendEof(); - channel.close(); - } + toRemove->shutdown(); } channelsToRemove_.clear(); } @@ -111,6 +104,10 @@ namespace SecureShell void Session::removeAllSftpSessions() { processingThread_.pushTask([this]() { + for (auto& sftp : sftpSessions_) + { + sftp->close(false); + } sftpSessions_.clear(); }); } diff --git a/ssh/source/ssh/sftp_session.cpp b/ssh/source/ssh/sftp_session.cpp index 88d1470a..7143706d 100644 --- a/ssh/source/ssh/sftp_session.cpp +++ b/ssh/source/ssh/sftp_session.cpp @@ -11,21 +11,50 @@ namespace SecureShell : ownerMutex_{} , owner_{owner} , session_{session} + , fileStreams_{} {} SftpSession::~SftpSession() { /* close makes no sense, since this lives in a shared_ptr and will only ever end here when it was already * removed */ } - void SftpSession::close() + void SftpSession::close(bool removeSelf) { - std::scoped_lock lock{ownerMutex_}; - if (owner_) + removeAllFileStreams(); { - owner_->sftpSessionRemoveItself(this); - owner_ = nullptr; + std::scoped_lock lock{ownerMutex_}; + if (owner_) + { + if (removeSelf) + owner_->sftpSessionRemoveItself(this); + owner_ = nullptr; + } } } + void SftpSession::fileStreamRemoveItself(FileStream* stream) + { + if (stream) + { + perform([this, stream]() { + fileStreams_.erase( + std::remove_if( + fileStreams_.begin(), + fileStreams_.end(), + [stream](auto const& item) { + return item.get() == stream; + }), + fileStreams_.end()); + }); + awaitCycle(); + } + } + void SftpSession::removeAllFileStreams() + { + perform([this]() { + fileStreams_.clear(); + }); + awaitCycle(); + } SftpSession::DirectoryEntry SftpSession::DirectoryEntry::fromSftpAttributes(sftp_attributes attributes) { return DirectoryEntry{ @@ -256,11 +285,11 @@ namespace SecureShell }); } - std::future> + std::future, SftpSession::Error>> SftpSession::openFile(std::filesystem::path const& path, OpenType openType, std::filesystem::perms permissions) { return performPromise( - [this, path = std::move(path), openType, permissions]() -> std::expected { + [this, path = std::move(path), openType, permissions]() -> std::expected, Error> { std::unique_ptr> file{ sftp_open( session_, @@ -281,7 +310,10 @@ namespace SecureShell if (limits == nullptr) return std::unexpected(lastError()); - return FileStream{shared_from_this(), file.release(), *limits}; + auto stream = std::make_shared(shared_from_this(), file.release(), *limits); + fileStreams_.push_back(stream); + + return std::weak_ptr{stream}; }); } } \ No newline at end of file diff --git a/ssh/test/ssh/main.cpp b/ssh/test/ssh/main.cpp index 89427dab..81728d8a 100644 --- a/ssh/test/ssh/main.cpp +++ b/ssh/test/ssh/main.cpp @@ -1,5 +1,5 @@ -// #include "test_processing_thread.hpp" -// #include "test_ssh_session.hpp" +#include "test_processing_thread.hpp" +#include "test_ssh_session.hpp" #include "test_sftp.hpp" #include "utility/node.hpp" diff --git a/ssh/test/ssh/test_processing_thread.hpp b/ssh/test/ssh/test_processing_thread.hpp index b675327a..b3dbaad2 100644 --- a/ssh/test/ssh/test_processing_thread.hpp +++ b/ssh/test/ssh/test_processing_thread.hpp @@ -339,4 +339,21 @@ namespace SecureShell::Test processingThread.clearPermanentTasks(); EXPECT_EQ(0, processingThread.permanentTaskCount()); } + + TEST_F(ProcessingThreadTest, CanWaitForCycle) + { + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + EXPECT_TRUE(processingThread.awaitCycle()); + } + + TEST_F(ProcessingThreadTest, CanWaitForCyclesMultipleTimes) + { + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + for (int i = 0; i < 5; ++i) + { + EXPECT_TRUE(processingThread.awaitCycle()); + } + } } \ No newline at end of file diff --git a/ssh/test/ssh/test_sftp.hpp b/ssh/test/ssh/test_sftp.hpp index 43840dd5..8ad0b7b3 100644 --- a/ssh/test/ssh/test_sftp.hpp +++ b/ssh/test/ssh/test_sftp.hpp @@ -260,10 +260,12 @@ namespace SecureShell::Test auto result = fut.get(); ASSERT_TRUE(result.has_value()); - auto file = std::move(result).value(); - std::byte byte; + auto fileWeak = std::move(result).value(); + auto file = fileWeak.lock(); + ASSERT_TRUE(file); - auto readFut = file.read(&byte, 1); + std::byte byte; + auto readFut = file->read(&byte, 1); ASSERT_EQ(readFut.wait_for(1s), std::future_status::ready); auto readResult = readFut.get(); ASSERT_TRUE(readResult.has_value()); @@ -282,9 +284,12 @@ namespace SecureShell::Test auto result = fut.get(); ASSERT_TRUE(result.has_value()); - auto file = std::move(result).value(); + auto fileWeak = std::move(result).value(); + auto file = fileWeak.lock(); + ASSERT_TRUE(file); + std::vector buffer(1024); - auto readFut = file.read(buffer.data(), buffer.size()); + auto readFut = file->read(buffer.data(), buffer.size()); ASSERT_EQ(readFut.wait_for(1s), std::future_status::ready); auto readResult = readFut.get(); @@ -305,9 +310,12 @@ namespace SecureShell::Test auto result = fut.get(); ASSERT_TRUE(result.has_value()); - auto file = std::move(result).value(); + auto fileWeak = std::move(result).value(); + auto file = fileWeak.lock(); + ASSERT_TRUE(file); + std::string data; - auto readFut = file.read([&data](std::string_view chunk) { + auto readFut = file->read([&data](std::string_view chunk) { data.append(chunk); return true; }); @@ -330,9 +338,12 @@ namespace SecureShell::Test auto result = fut.get(); ASSERT_TRUE(result.has_value()); - auto file = std::move(result).value(); + auto fileWeak = std::move(result).value(); + auto file = fileWeak.lock(); + ASSERT_TRUE(file); + std::string data; - auto readFut = file.read([&data](std::string_view chunk) { + auto readFut = file->read([&data](std::string_view chunk) { data.append(chunk); return true; }); @@ -355,10 +366,13 @@ namespace SecureShell::Test auto result = fut.get(); ASSERT_TRUE(result.has_value()); - auto file = std::move(result).value(); - ASSERT_EQ(file.seek(2).wait_for(1s), std::future_status::ready); + auto fileWeak = std::move(result).value(); + auto file = fileWeak.lock(); + ASSERT_TRUE(file); + + ASSERT_EQ(file->seek(2).wait_for(1s), std::future_status::ready); std::vector buffer(1024); - auto readFut2 = file.read(buffer.data(), buffer.size()); + auto readFut2 = file->read(buffer.data(), buffer.size()); ASSERT_EQ(readFut2.wait_for(1s), std::future_status::ready); auto readResult2 = readFut2.get(); @@ -379,11 +393,14 @@ namespace SecureShell::Test auto result = fut.get(); ASSERT_TRUE(result.has_value()); - auto file = std::move(result).value(); + auto fileWeak = std::move(result).value(); + auto file = fileWeak.lock(); + ASSERT_TRUE(file); + std::vector buffer(1024); - ASSERT_EQ(file.seek(2).wait_for(1s), std::future_status::ready); - ASSERT_EQ(file.rewind().wait_for(1s), std::future_status::ready); - auto readFut2 = file.read(buffer.data(), buffer.size()); + ASSERT_EQ(file->seek(2).wait_for(1s), std::future_status::ready); + ASSERT_EQ(file->rewind().wait_for(1s), std::future_status::ready); + auto readFut2 = file->read(buffer.data(), buffer.size()); ASSERT_EQ(readFut2.wait_for(1s), std::future_status::ready); auto readResult2 = readFut2.get(); @@ -404,9 +421,12 @@ namespace SecureShell::Test auto result = fut.get(); ASSERT_TRUE(result.has_value()); - auto file = std::move(result).value(); - ASSERT_EQ(file.seek(2).wait_for(1s), std::future_status::ready); - auto tellFut = file.tell(); + auto fileWeak = std::move(result).value(); + auto file = fileWeak.lock(); + ASSERT_TRUE(file); + + ASSERT_EQ(file->seek(2).wait_for(1s), std::future_status::ready); + auto tellFut = file->tell(); ASSERT_EQ(tellFut.wait_for(1s), std::future_status::ready); auto tellResult = tellFut.get(); diff --git a/ssh/test/ssh/test_ssh_session.hpp b/ssh/test/ssh/test_ssh_session.hpp index 2dd9687d..04f86e3c 100644 --- a/ssh/test/ssh/test_ssh_session.hpp +++ b/ssh/test/ssh/test_ssh_session.hpp @@ -268,7 +268,8 @@ namespace SecureShell::Test auto [result, processThread] = createSshServer(); ASSERT_TRUE(result); auto joiner = Nui::ScopeExit{[&]() noexcept { - result->command("exit"); + // result->command("exit"); + result->terminate(); if (processThread.joinable()) processThread.join(); }}; diff --git a/ssh/test/ssh/utility/node.cpp b/ssh/test/ssh/utility/node.cpp index 00c3db6e..5383945f 100644 --- a/ssh/test/ssh/utility/node.cpp +++ b/ssh/test/ssh/utility/node.cpp @@ -35,6 +35,21 @@ namespace SecureShell::Test } } + void NodeProcessResult::terminate() + { + if (killed) + return; + killed = true; + try + { + mainModule->terminate(); + } + catch (std::exception const& e) + { + std::cerr << "Failed to terminate process: " << e.what() << std::endl; + } + } + void npmInstall( boost::asio::any_io_executor executor, std::filesystem::path const& directory, @@ -127,7 +142,14 @@ namespace SecureShell::Test if (auto result = weak.lock()) { result->killed = true; - result->mainModule->terminate(); + try + { + result->mainModule->terminate(); + } + catch (std::exception const& e) + { + std::cerr << "Failed to terminate process: " << e.what() << std::endl; + } } }); diff --git a/ssh/test/ssh/utility/node.hpp b/ssh/test/ssh/utility/node.hpp index 4f1e3a4a..8e3e670f 100644 --- a/ssh/test/ssh/utility/node.hpp +++ b/ssh/test/ssh/utility/node.hpp @@ -32,6 +32,7 @@ namespace SecureShell::Test int code = 0; void command(std::string const& command); + void terminate(); }; void npmInstall( From 0163757f510491c94c72f81b59fb1fbc5b29ac32 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Sat, 8 Mar 2025 03:31:32 +0100 Subject: [PATCH 04/42] Improved synchronization of ssh channels and sessions using strands. --- ssh/include/ssh/async/processing_strand.hpp | 88 ++++++++++++++ ssh/include/ssh/async/processing_thread.hpp | 5 + ssh/include/ssh/channel.hpp | 8 +- ssh/include/ssh/session.hpp | 3 - ssh/include/ssh/sftp_session.hpp | 32 +---- ssh/source/ssh/async/processing_thread.cpp | 31 ++++- ssh/source/ssh/channel.cpp | 55 +++------ ssh/source/ssh/session.cpp | 77 ++++-------- ssh/source/ssh/sftp_session.cpp | 50 +++----- ssh/test/ssh/test_processing_thread.hpp | 128 +++++++++++++++++++- ssh/test/ssh/test_sftp.hpp | 2 +- 11 files changed, 318 insertions(+), 161 deletions(-) create mode 100644 ssh/include/ssh/async/processing_strand.hpp diff --git a/ssh/include/ssh/async/processing_strand.hpp b/ssh/include/ssh/async/processing_strand.hpp new file mode 100644 index 00000000..996df09f --- /dev/null +++ b/ssh/include/ssh/async/processing_strand.hpp @@ -0,0 +1,88 @@ +#pragma once + +#include + +#include +#include +#include +#include + +namespace SecureShell +{ + class ProcessingStrand + { + public: + ProcessingStrand(ProcessingThread* processingThread) + : processingThread_(processingThread) + {} + + bool pushTask(std::function task) + { + std::scoped_lock lock(mutex_); + if (finalized_) + { + return false; + } + return processingThread_->pushTask(std::move(task)); + } + + std::pair pushPermanentTask(std::function task) + { + std::scoped_lock lock(mutex_); + if (finalized_) + { + return {false, ProcessingThread::PermanentTaskId{-1}}; + } + auto result = processingThread_->pushPermanentTask(std::move(task)); + this->permanentTasks_.insert(result.second); + return result; + } + + void pushFinalTask(std::function task) + { + std::scoped_lock lock(mutex_); + if (finalized_) + { + return; + } + finalized_ = true; + for (auto const& id : permanentTasks_) + { + processingThread_->removePermanentTask(id); + } + processingThread_->pushTask(std::move(task)); + } + + void doFinalSync(std::function const& task) + { + std::scoped_lock lock(mutex_); + if (finalized_) + { + return; + } + finalized_ = true; + for (auto const& id : permanentTasks_) + { + processingThread_->removePermanentTask(id); + } + task(); + } + + template + auto pushPromiseTask(Func&& func) -> std::future>> + { + std::scoped_lock lock(mutex_); + if (finalized_) + { + throw std::runtime_error("Cannot push task to finalized strand."); + } + return processingThread_->pushPromiseTask(std::forward(func)); + } + + private: + std::recursive_mutex mutex_{}; + bool finalized_ = false; + ProcessingThread* processingThread_{}; + std::set permanentTasks_{}; + }; +} \ No newline at end of file diff --git a/ssh/include/ssh/async/processing_thread.hpp b/ssh/include/ssh/async/processing_thread.hpp index 94eadf9c..f90f3ab6 100644 --- a/ssh/include/ssh/async/processing_thread.hpp +++ b/ssh/include/ssh/async/processing_thread.hpp @@ -8,9 +8,12 @@ #include #include #include +#include namespace SecureShell { + class ProcessingStrand; + class ProcessingThread { public: @@ -72,6 +75,8 @@ namespace SecureShell return processingThreadId_ == std::this_thread::get_id(); } + std::unique_ptr createStrand(); + private: void run(std::chrono::milliseconds const& waitCycleTimeout, std::chrono::milliseconds const& minimumCycleWait); diff --git a/ssh/include/ssh/channel.hpp b/ssh/include/ssh/channel.hpp index d0af51e3..1ba15d52 100644 --- a/ssh/include/ssh/channel.hpp +++ b/ssh/include/ssh/channel.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -16,7 +17,7 @@ namespace SecureShell class Channel : public std::enable_shared_from_this { public: - Channel(Session* owner, std::unique_ptr); + Channel(Session* owner, std::unique_ptr strand, std::unique_ptr); ~Channel(); Channel(Channel const&) = delete; Channel& operator=(Channel const&) = delete; @@ -28,8 +29,6 @@ namespace SecureShell return *channel_; } - void shutdown(); - void close(); /** @@ -79,12 +78,11 @@ namespace SecureShell void readTask(std::chrono::milliseconds pollTimeout = std::chrono::milliseconds{0}); private: - std::mutex ownerMutex_; Session* owner_; + std::unique_ptr strand_; std::unique_ptr channel_; std::function onStdout_{}; std::function onStderr_{}; std::function onExit_{}; - std::optional readTaskId_{std::nullopt}; }; } \ No newline at end of file diff --git a/ssh/include/ssh/session.hpp b/ssh/include/ssh/session.hpp index 204f9110..a3d51d6e 100644 --- a/ssh/include/ssh/session.hpp +++ b/ssh/include/ssh/session.hpp @@ -86,8 +86,6 @@ namespace SecureShell void sftpSessionRemoveItself(SftpSession* sftpSession); void removeAllSftpSessions(); - void removalTask(); - /** * @brief Shuts down the session and closes all channels. * The session is not usable after this. @@ -98,7 +96,6 @@ namespace SecureShell SecureShell::ProcessingThread processingThread_; ssh::Session session_; std::vector> channels_; - std::vector> channelsToRemove_; std::vector> sftpSessions_; }; diff --git a/ssh/include/ssh/sftp_session.hpp b/ssh/include/ssh/sftp_session.hpp index 8e9085e0..797620be 100644 --- a/ssh/include/ssh/sftp_session.hpp +++ b/ssh/include/ssh/sftp_session.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -27,7 +28,7 @@ namespace SecureShell using Error = SftpError; friend class FileStream; - SftpSession(Session* owner, sftp_session session); + SftpSession(Session* owner, std::unique_ptr strand, sftp_session session); ~SftpSession(); SftpSession(SftpSession const&) = delete; SftpSession& operator=(SftpSession const&) = delete; @@ -39,7 +40,7 @@ namespace SecureShell return session_; } - void close(bool removeSelf = true); + void close(); struct DirectoryEntry : public SharedData::DirectoryEntry { @@ -49,34 +50,13 @@ namespace SecureShell template void perform(FunctionT&& func) { - std::scoped_lock lock{ownerMutex_}; - if (owner_) - owner_->processingThread_.pushTask(std::forward(func)); + strand_->pushTask(std::forward(func)); } template auto performPromise(FunctionT&& func) -> std::future>> { - using ResultType = std::invoke_result_t>; - { - std::scoped_lock lock{ownerMutex_}; - if (owner_) - return owner_->processingThread_.pushPromiseTask(std::forward(func)); - } - std::promise promise{}; - promise.set_value(std::unexpected(SftpError{ - .message = "Owner is null", - .wrapperError = WrapperErrors::OwnerNull, - })); - return promise.get_future(); - } - - bool awaitCycle() - { - std::scoped_lock lock{ownerMutex_}; - if (owner_) - return owner_->processingThread_.awaitCycle(); - return false; + return strand_->pushPromiseTask(std::forward(func)); } /** @@ -193,8 +173,8 @@ namespace SecureShell void removeAllFileStreams(); private: - std::mutex ownerMutex_; Session* owner_; + std::unique_ptr strand_; sftp_session session_; std::vector> fileStreams_; }; diff --git a/ssh/source/ssh/async/processing_thread.cpp b/ssh/source/ssh/async/processing_thread.cpp index bdadf8ed..bb86475e 100644 --- a/ssh/source/ssh/async/processing_thread.cpp +++ b/ssh/source/ssh/async/processing_thread.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -68,6 +69,10 @@ namespace SecureShell taskCondition_.notify_one(); return true; } + std::unique_ptr ProcessingThread::createStrand() + { + return std::make_unique(this); + } std::pair ProcessingThread::pushPermanentTask(std::function task) { if (!task) @@ -107,14 +112,28 @@ namespace SecureShell } bool ProcessingThread::removePermanentTask(PermanentTaskId const& id) { - std::lock_guard lock{taskMutex_}; + std::unique_lock lock{taskMutex_}; if (processingPermanents_) { - deferredTaskModification_.push_back([this, id]() { - permanentTasks_.erase(id); - permanentTasksAvailable_ = !permanentTasks_.empty(); - }); - return true; + if (withinProcessingThread()) + { + deferredTaskModification_.push_back([this, id]() { + permanentTasks_.erase(id); + permanentTasksAvailable_ = !permanentTasks_.empty(); + }); + return true; + } + else + { + std::promise promise{}; + deferredTaskModification_.push_back([this, id, &promise]() { + const bool result = permanentTasks_.erase(id) > 0; + permanentTasksAvailable_ = !permanentTasks_.empty(); + promise.set_value(result); + }); + lock.unlock(); + return promise.get_future().wait_for(std::chrono::seconds{5}) == std::future_status::ready; + } } auto result = permanentTasks_.erase(id); diff --git a/ssh/source/ssh/channel.cpp b/ssh/source/ssh/channel.cpp index 41b01656..46dad315 100644 --- a/ssh/source/ssh/channel.cpp +++ b/ssh/source/ssh/channel.cpp @@ -3,8 +3,9 @@ namespace SecureShell { - Channel::Channel(Session* owner, std::unique_ptr channel) + Channel::Channel(Session* owner, std::unique_ptr strand, std::unique_ptr channel) : owner_{owner} + , strand_{std::move(strand)} , channel_{std::move(channel)} {} Channel::~Channel() @@ -14,53 +15,42 @@ namespace SecureShell } Channel::Channel(Channel&& other) : owner_{std::exchange(other.owner_, nullptr)} + , strand_{std::move(other.strand_)} + , channel_{std::move(other.channel_)} {} Channel& Channel::operator=(Channel&& other) { - std::scoped_lock lock{ownerMutex_, other.ownerMutex_}; if (this != &other) { owner_ = std::exchange(other.owner_, nullptr); + strand_ = std::exchange(other.strand_, nullptr); + channel_ = std::exchange(other.channel_, nullptr); } return *this; } - void Channel::shutdown() - { - { - std::scoped_lock lock{ownerMutex_}; - if (owner_ && readTaskId_) - owner_->processingThread_.removePermanentTask(*readTaskId_); - } - if (channel_ && channel_->isOpen()) - { - channel_->sendEof(); - channel_->close(); - } - channel_.reset(); - } void Channel::close() { - shutdown(); - decltype(owner_) cpy = nullptr; - { - std::scoped_lock lock{ownerMutex_}; - cpy = std::exchange(owner_, nullptr); - } - cpy->channelRemoveItself(this); + strand_->doFinalSync([this]() { + if (channel_ && channel_->isOpen()) + { + channel_->sendEof(); + channel_->close(); + } + channel_.reset(); + owner_->channelRemoveItself(this); + }); } void Channel::write(std::string data) { - std::scoped_lock lock{ownerMutex_}; - owner_->processingThread_.pushTask([this, data = std::move(data)]() { + strand_->pushTask([this, data = std::move(data)]() { if (channel_) channel_->write(data.data(), data.size()); }); } std::future Channel::resizePty(int cols, int rows) { - std::scoped_lock lock{ownerMutex_}; auto promise = std::make_shared>(); - owner_->processingThread_.pushTask([this, cols, rows, promise]() { + strand_->pushTask([this, cols, rows, promise]() { if (channel_) promise->set_value(channel_->changePtySize(cols, rows)); else @@ -120,22 +110,17 @@ namespace SecureShell std::function onStderr, std::function onExit) { - std::scoped_lock lock{ownerMutex_}; - onStdout_ = std::move(onStdout); onStderr_ = std::move(onStderr); onExit_ = std::move(onExit); - auto [success, id] = owner_->processingThread_.pushPermanentTask([this]() { + auto [success, id] = strand_->pushPermanentTask([this]() { readTask(); }); if (!success) { - // TODO: ??? - } - else - { - readTaskId_ = id; + // TODO: Do I want to throw here? + throw std::runtime_error("Failed to start reading."); } } } \ No newline at end of file diff --git a/ssh/source/ssh/session.cpp b/ssh/source/ssh/session.cpp index ec9ea429..b7f29863 100644 --- a/ssh/source/ssh/session.cpp +++ b/ssh/source/ssh/session.cpp @@ -11,7 +11,6 @@ namespace SecureShell : processingThread_{} , session_{} , channels_{} - , channelsToRemove_{} {} void Session::start() @@ -47,68 +46,37 @@ namespace SecureShell void Session::removeAllChannels() { processingThread_.pushTask([this]() { - if (!channelsToRemove_.empty()) - removalTask(); - channelsToRemove_ = std::move(channels_); - removalTask(); + for (auto& channel : channels_) + channel->close(); }); } - void Session::channelRemoveItself(Channel* channel) + void Session::removeAllSftpSessions() { - if (channel) - { - processingThread_.pushTask([this, channel]() { - auto it = std::find_if(channels_.begin(), channels_.end(), [channel](const auto& c) { - return c.get() == channel; - }); - if (it != channels_.end()) - { - channelsToRemove_.push_back(*it); - channels_.erase(it); - removalTask(); - } - else - { - // Possibly already flagged for removal - } - }); - } + processingThread_.pushTask([this]() { + for (auto& sftp : sftpSessions_) + sftp->close(); + }); } - void Session::removalTask() + void Session::channelRemoveItself(Channel* channel) { - for (const auto& toRemove : channelsToRemove_) - { - toRemove->shutdown(); - } - channelsToRemove_.clear(); + processingThread_.pushTask([this, channel]() { + auto it = std::find_if(channels_.begin(), channels_.end(), [channel](const auto& c) { + return c.get() == channel; + }); + if (it != channels_.end()) + channels_.erase(it); + }); } - void Session::sftpSessionRemoveItself(SftpSession* sftpSession) { - if (sftpSession) - { - processingThread_.pushTask([this, sftpSession]() { - auto it = std::find_if(sftpSessions_.begin(), sftpSessions_.end(), [sftpSession](const auto& s) { - return s.get() == sftpSession; - }); - if (it != sftpSessions_.end()) - { - sftpSessions_.erase(it); - } + processingThread_.pushTask([this, sftpSession]() { + auto it = std::find_if(sftpSessions_.begin(), sftpSessions_.end(), [sftpSession](const auto& s) { + return s.get() == sftpSession; }); - } - } - - void Session::removeAllSftpSessions() - { - processingThread_.pushTask([this]() { - for (auto& sftp : sftpSessions_) - { - sftp->close(false); - } - sftpSessions_.clear(); + if (it != sftpSessions_.end()) + sftpSessions_.erase(it); }); } @@ -149,7 +117,8 @@ namespace SecureShell return promise->set_value(std::unexpected(session_.getErrorCode())); } - auto sharedChannel = std::make_shared(this, std::move(ptyChannel)); + auto sharedChannel = + std::make_shared(this, processingThread_.createStrand(), std::move(ptyChannel)); channels_.push_back(sharedChannel); return promise->set_value(sharedChannel); }); @@ -182,7 +151,7 @@ namespace SecureShell sftp_free(sftp); } - auto sftpSession = std::make_shared(this, sftp); + auto sftpSession = std::make_shared(this, processingThread_.createStrand(), sftp); promise->set_value(sftpSession); sftpSessions_.push_back(sftpSession); }); diff --git a/ssh/source/ssh/sftp_session.cpp b/ssh/source/ssh/sftp_session.cpp index 7143706d..a32b6103 100644 --- a/ssh/source/ssh/sftp_session.cpp +++ b/ssh/source/ssh/sftp_session.cpp @@ -7,9 +7,9 @@ namespace SecureShell { - SftpSession::SftpSession(Session* owner, sftp_session session) - : ownerMutex_{} - , owner_{owner} + SftpSession::SftpSession(Session* owner, std::unique_ptr strand, sftp_session session) + : owner_{owner} + , strand_{std::move(strand)} , session_{session} , fileStreams_{} {} @@ -18,42 +18,32 @@ namespace SecureShell /* close makes no sense, since this lives in a shared_ptr and will only ever end here when it was already * removed */ } - void SftpSession::close(bool removeSelf) + void SftpSession::close() { - removeAllFileStreams(); - { - std::scoped_lock lock{ownerMutex_}; - if (owner_) - { - if (removeSelf) - owner_->sftpSessionRemoveItself(this); - owner_ = nullptr; - } - } + strand_->doFinalSync([this]() { + removeAllFileStreams(); + owner_->sftpSessionRemoveItself(this); + }); } void SftpSession::fileStreamRemoveItself(FileStream* stream) { - if (stream) - { - perform([this, stream]() { - fileStreams_.erase( - std::remove_if( - fileStreams_.begin(), - fileStreams_.end(), - [stream](auto const& item) { - return item.get() == stream; - }), - fileStreams_.end()); - }); - awaitCycle(); - } + perform([this, stream]() { + fileStreams_.erase( + std::remove_if( + fileStreams_.begin(), + fileStreams_.end(), + [stream](auto const& item) { + return item.get() == stream; + }), + fileStreams_.end()); + }); } void SftpSession::removeAllFileStreams() { perform([this]() { - fileStreams_.clear(); + for (auto& stream : fileStreams_) + stream->close(); }); - awaitCycle(); } SftpSession::DirectoryEntry SftpSession::DirectoryEntry::fromSftpAttributes(sftp_attributes attributes) { diff --git a/ssh/test/ssh/test_processing_thread.hpp b/ssh/test/ssh/test_processing_thread.hpp index b3dbaad2..a6d5706d 100644 --- a/ssh/test/ssh/test_processing_thread.hpp +++ b/ssh/test/ssh/test_processing_thread.hpp @@ -2,10 +2,10 @@ #include "utility/awaiter.hpp" #include +#include #include -#include #include #include @@ -356,4 +356,130 @@ namespace SecureShell::Test EXPECT_TRUE(processingThread.awaitCycle()); } } + + TEST_F(ProcessingThreadTest, CanMakeStrandAndPushTaskThroughIt) + { + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushTask([] {}); + processingThread.stop(); + } + + TEST_F(ProcessingThreadTest, CanMakeStrandAndPushPermanentTaskThroughIt) + { + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushPermanentTask([] {}); + processingThread.stop(); + } + + TEST_F(ProcessingThreadTest, TaskInStrandIsExecuted) + { + Awaiter awaiter{}; + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushTask([&awaiter] { + awaiter.arrive(); + }); + ASSERT_TRUE(awaiter.waitFor()); + } + + TEST_F(ProcessingThreadTest, PermanentTaskInStrandIsExecuted) + { + Awaiter awaiter{}; + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + auto result = strand->pushPermanentTask([&awaiter] { + awaiter.arrive(); + }); + ASSERT_TRUE(result.first); + ASSERT_TRUE(awaiter.waitFor()); + } + + TEST_F(ProcessingThreadTest, CannotPushTaskAfterFinalTask) + { + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushFinalTask([] {}); + EXPECT_FALSE(strand->pushTask([] {})); + } + + TEST_F(ProcessingThreadTest, CannotPushPermanentTaskAfterFinalTask) + { + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushFinalTask([] {}); + EXPECT_FALSE(strand->pushPermanentTask([] {}).first); + } + + TEST_F(ProcessingThreadTest, CanPushFinalTaskAfterFinalTask) + { + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushFinalTask([] {}); + EXPECT_NO_THROW(strand->pushFinalTask([] {})); + } + + TEST_F(ProcessingThreadTest, FinalTaskRemovesAllPermanentTasks) + { + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushPermanentTask([] {}); + strand->pushPermanentTask([] {}); + strand->pushPermanentTask([] {}); + strand->pushFinalTask([] {}); + EXPECT_EQ(0, processingThread.permanentTaskCount()); + } + + TEST_F(ProcessingThreadTest, CanPushFinalFromTask) + { + Awaiter awaiter{}; + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushTask([&strand, &awaiter] { + strand->pushFinalTask([&awaiter] { + awaiter.arrive(); + }); + }); + ASSERT_TRUE(awaiter.waitFor()); + } + + TEST_F(ProcessingThreadTest, CanPushFinalFromPermanentTask) + { + Awaiter awaiter{}; + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushPermanentTask([&strand, &awaiter] { + strand->pushFinalTask([&awaiter] { + awaiter.arrive(); + }); + }); + ASSERT_TRUE(awaiter.waitFor()); + } + + TEST_F(ProcessingThreadTest, CanPushFinalFromTaskFromPermanentTask) + { + Awaiter awaiter{}; + ProcessingThread processingThread; + processingThread.start(std::chrono::milliseconds{1}); + auto strand = processingThread.createStrand(); + strand->pushPermanentTask([&strand, &awaiter] { + strand->pushTask([&strand, &awaiter] { + strand->pushFinalTask([&awaiter] { + awaiter.arrive(); + }); + }); + }); + ASSERT_TRUE(awaiter.waitFor()); + } } \ No newline at end of file diff --git a/ssh/test/ssh/test_sftp.hpp b/ssh/test/ssh/test_sftp.hpp index 8ad0b7b3..1a487978 100644 --- a/ssh/test/ssh/test_sftp.hpp +++ b/ssh/test/ssh/test_sftp.hpp @@ -79,7 +79,7 @@ namespace SecureShell::Test ASSERT_EQ(fut.wait_for(1s), std::future_status::ready); auto result = fut.get(); - ASSERT_TRUE(result.has_value()); + ASSERT_TRUE(result.has_value()) << result.error().toString(); EXPECT_GT(result.value().size(), 0); auto it = std::find_if(result.value().begin(), result.value().end(), [](const auto& entry) { return entry.path == "file1.txt"; From 9ee44191f98f7b4b261081e9657e4c613ca4d36d Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Sat, 8 Mar 2025 03:40:00 +0100 Subject: [PATCH 05/42] Added some docs comments. --- ssh/include/ssh/async/processing_strand.hpp | 50 +++++++++++++++++---- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/ssh/include/ssh/async/processing_strand.hpp b/ssh/include/ssh/async/processing_strand.hpp index 996df09f..bf0d818a 100644 --- a/ssh/include/ssh/async/processing_strand.hpp +++ b/ssh/include/ssh/async/processing_strand.hpp @@ -9,42 +9,65 @@ namespace SecureShell { + /** + * @brief This class makes it impossible to push tasks to a strand after it has been finalized. + * Finalization usually means some close operation is happening and no more tasks should be pushed. + * Like reading a file when the session is closed. + */ class ProcessingStrand { public: + /** + * @brief Construct a new Processing Strand object living on top of a processing thread. + * + * @param processingThread The processing thread to push tasks to. + */ ProcessingStrand(ProcessingThread* processingThread) : processingThread_(processingThread) {} + /** + * @brief Pushes a task, but not if the strand has been finalized and not simultaneously with any other push. + * + * @param task The task to push. + * @return true If the task was pushed. + * @return false If the strand has been finalized or the task was empty. + */ bool pushTask(std::function task) { std::scoped_lock lock(mutex_); if (finalized_) - { return false; - } return processingThread_->pushTask(std::move(task)); } + /** + * @brief Pushes a task that will be executed until the strand is finalized. + * But not if the strand has been finalized and not simultaneously with any other push. + * + * @param task The task to push. + * @return std::pair If the task was pushed and the id of the task. + */ std::pair pushPermanentTask(std::function task) { std::scoped_lock lock(mutex_); if (finalized_) - { return {false, ProcessingThread::PermanentTaskId{-1}}; - } auto result = processingThread_->pushPermanentTask(std::move(task)); this->permanentTasks_.insert(result.second); return result; } + /** + * @brief Pushes a task that makes any further pushes impossible. Also removes all permanent tasks. + * + * @param task The task to push. + */ void pushFinalTask(std::function task) { std::scoped_lock lock(mutex_); if (finalized_) - { return; - } finalized_ = true; for (auto const& id : permanentTasks_) { @@ -53,13 +76,17 @@ namespace SecureShell processingThread_->pushTask(std::move(task)); } + /** + * @brief Does a final task but runs it on the current calling thread. + * Otherwise behaves like pushFinalTask. + * + * @param task The task to run. + */ void doFinalSync(std::function const& task) { std::scoped_lock lock(mutex_); if (finalized_) - { return; - } finalized_ = true; for (auto const& id : permanentTasks_) { @@ -68,6 +95,13 @@ namespace SecureShell task(); } + /** + * @brief Pushes a task the return of which is returned for a future. + * + * @tparam Func The type of the task. + * @param func The task to push. + * @return std::future>> The future of the return value of the task. + */ template auto pushPromiseTask(Func&& func) -> std::future>> { From 5f410f33f88992314cc5759de65e5c80e7364ccb Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Sat, 8 Mar 2025 03:46:30 +0100 Subject: [PATCH 06/42] Added documentation to processing thread class. --- ssh/include/ssh/async/processing_thread.hpp | 81 +++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/ssh/include/ssh/async/processing_thread.hpp b/ssh/include/ssh/async/processing_thread.hpp index f90f3ab6..bab8ea4a 100644 --- a/ssh/include/ssh/async/processing_thread.hpp +++ b/ssh/include/ssh/async/processing_thread.hpp @@ -14,11 +14,18 @@ namespace SecureShell { class ProcessingStrand; + /** + * @brief A processing thread that can be used to execute tasks sequentially in a separate thread. + * This allows for complex simultaneous operations to be executed in a single thread. + */ class ProcessingThread { public: constexpr static unsigned int maximumTasksProcessableAtOnce = 100; + /** + * @brief A permanent task id that can be used to remove permanent tasks. + */ struct PermanentTaskId { int id = -1; @@ -35,15 +42,47 @@ namespace SecureShell ProcessingThread(ProcessingThread&&) = delete; ProcessingThread& operator=(ProcessingThread&&) = delete; + /** + * @brief Starts the processing thread. + * + * @param waitCycleTimeout The time to wait for a task to become available before checking if the thread should + * stop. + * @param minimumCycleWait The minimum wait time between cycles to throttle the processing thread in case of the + * existence of permanent tasks. + */ void start( std::chrono::milliseconds const& waitCycleTimeout = std::chrono::seconds{1}, std::chrono::milliseconds const& minimumCycleWait = std::chrono::milliseconds{0}); + + /** + * @brief Stops the processing thread. Executes all pending tasks. + */ void stop(); + /** + * @brief Returns true if the processing thread is running. + * + * @return true If the processing thread is running. + * @return false If the processing thread is not running. + */ bool isRunning() const; + /** + * @brief Pushes a task to the processing thread. + * + * @param task The task to push. + * @return true If the task was pushed. + * @return false If the task was empty or the processing thread is shutting down. + */ bool pushTask(std::function task); + /** + * @brief Pushes a task that has a return value to the processing thread. + * The return value is then accessible through the returned future. + * + * @param func The function to execute. + * @return std::future>> The future that will contain the return value. + */ template auto pushPromiseTask(Func&& func) -> std::future>> { @@ -63,18 +102,60 @@ namespace SecureShell return promise->get_future(); } + /** + * @brief Pushes a task that is not removed upon execution. Will be executed every cycle. + * + * @param task The task to push. + * @return std::pair If the task was pushed and the id of the task. + */ std::pair pushPermanentTask(std::function task); + + /** + * @brief Removes a permanent task. + * + * @param id The id of the task to remove. + * @return true If the task was removed. + * @return false If the task was not found. + */ bool removePermanentTask(PermanentTaskId const& id); + + /** + * @brief Returns the number of permanent tasks. + * + * @return int The number of permanent tasks. + */ int permanentTaskCount() const; + + /** + * @brief Removes all permanent tasks. + */ void clearPermanentTasks(); + /** + * @brief Waits for a cycle to complete. + * + * @param maxWait The maximum time to wait for a cycle. + * @return true If a cycle was completed. + * @return false If the maximum wait time was reached. + */ bool awaitCycle(std::chrono::milliseconds maxWait = std::chrono::seconds{5}); + /** + * @brief Returns true if the current thread is the processing thread. + * + * @return true If the current thread is the processing thread. + * @return false If the current thread is not the processing thread. + */ bool withinProcessingThread() const { return processingThreadId_ == std::this_thread::get_id(); } + /** + * @brief Creates a new processing strand. + * + * @return std::unique_ptr The processing strand. + */ std::unique_ptr createStrand(); private: From 1d0b3f30daed2b2e901ee1a013e97084da21ecf0 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Sat, 8 Mar 2025 17:31:49 +0100 Subject: [PATCH 07/42] Improved multi threading behaviour of close. --- ssh/include/ssh/async/processing_strand.hpp | 39 ++++++- ssh/include/ssh/async/processing_thread.hpp | 18 +-- ssh/include/ssh/channel.hpp | 4 +- ssh/include/ssh/file_stream.hpp | 2 +- ssh/include/ssh/session.hpp | 4 +- ssh/include/ssh/sftp_error.hpp | 3 +- ssh/include/ssh/sftp_session.hpp | 4 +- ssh/source/ssh/async/processing_thread.cpp | 97 ++++++++------- ssh/source/ssh/channel.cpp | 51 +++++--- ssh/source/ssh/file_stream.cpp | 37 +++++- ssh/source/ssh/session.cpp | 68 ++++++----- ssh/source/ssh/sftp_session.cpp | 42 ++++--- ssh/test/ssh/test_processing_thread.hpp | 2 +- ssh/test/ssh/test_sftp.hpp | 32 +++++ ssh/test/ssh/test_ssh_session.hpp | 123 ++++++++++++++++++++ 15 files changed, 385 insertions(+), 141 deletions(-) diff --git a/ssh/include/ssh/async/processing_strand.hpp b/ssh/include/ssh/async/processing_strand.hpp index bf0d818a..bb2e38c1 100644 --- a/ssh/include/ssh/async/processing_strand.hpp +++ b/ssh/include/ssh/async/processing_strand.hpp @@ -76,6 +76,30 @@ namespace SecureShell processingThread_->pushTask(std::move(task)); } + /** + * @brief Works like pushFinalTask but returns a future for the return value of the task. + * + * @param task The task to run. + */ + template + auto pushFinalPromiseTask(FunctionT&& func) -> std::future>> + { + std::scoped_lock lock(mutex_); + if (finalized_) + { + std::promise>> promise{}; + promise.set_exception( + std::make_exception_ptr(std::runtime_error("Cannot push task to finalized strand."))); + return promise.get_future(); + } + finalized_ = true; + for (auto const& id : permanentTasks_) + { + processingThread_->removePermanentTask(id); + } + return processingThread_->pushPromiseTask(std::forward(func)); + } + /** * @brief Does a final task but runs it on the current calling thread. * Otherwise behaves like pushFinalTask. @@ -108,11 +132,24 @@ namespace SecureShell std::scoped_lock lock(mutex_); if (finalized_) { - throw std::runtime_error("Cannot push task to finalized strand."); + std::promise>> promise{}; + promise.set_exception( + std::make_exception_ptr(std::runtime_error("Cannot push task to finalized strand."))); + return promise.get_future(); } return processingThread_->pushPromiseTask(std::forward(func)); } + bool withinProcessingThread() const noexcept + { + return processingThread_->withinProcessingThread(); + } + + bool isFinalized() const noexcept + { + return finalized_; + } + private: std::recursive_mutex mutex_{}; bool finalized_ = false; diff --git a/ssh/include/ssh/async/processing_thread.hpp b/ssh/include/ssh/async/processing_thread.hpp index bab8ea4a..1dd3b60f 100644 --- a/ssh/include/ssh/async/processing_thread.hpp +++ b/ssh/include/ssh/async/processing_thread.hpp @@ -2,7 +2,7 @@ #include #include -#include +#include #include #include #include @@ -21,7 +21,7 @@ namespace SecureShell class ProcessingThread { public: - constexpr static unsigned int maximumTasksProcessableAtOnce = 100; + constexpr static std::size_t maximumTasksProcessableAtOnce = 100ull; /** * @brief A permanent task id that can be used to remove permanent tasks. @@ -50,9 +50,7 @@ namespace SecureShell * @param minimumCycleWait The minimum wait time between cycles to throttle the processing thread in case of the * existence of permanent tasks. */ - void start( - std::chrono::milliseconds const& waitCycleTimeout = std::chrono::seconds{1}, - std::chrono::milliseconds const& minimumCycleWait = std::chrono::milliseconds{0}); + void start(std::chrono::milliseconds const& minimumCycleWait = std::chrono::milliseconds{0}); /** * @brief Stops the processing thread. Executes all pending tasks. @@ -146,7 +144,7 @@ namespace SecureShell * @return true If the current thread is the processing thread. * @return false If the current thread is not the processing thread. */ - bool withinProcessingThread() const + bool withinProcessingThread() const noexcept { return processingThreadId_ == std::this_thread::get_id(); } @@ -159,7 +157,7 @@ namespace SecureShell std::unique_ptr createStrand(); private: - void run(std::chrono::milliseconds const& waitCycleTimeout, std::chrono::milliseconds const& minimumCycleWait); + void run(std::chrono::milliseconds const& minimumCycleWait); private: std::thread thread_{}; @@ -172,11 +170,7 @@ namespace SecureShell std::atomic_bool processingPermanents_{false}; std::vector> deferredTaskModification_{}; - std::mutex taskWaitMutex_{}; - std::condition_variable taskCondition_{}; - bool taskAvailable_ = false; - - std::queue> tasks_{}; + std::deque> tasks_{}; std::map> permanentTasks_{}; }; } \ No newline at end of file diff --git a/ssh/include/ssh/channel.hpp b/ssh/include/ssh/channel.hpp index 1ba15d52..65a337ca 100644 --- a/ssh/include/ssh/channel.hpp +++ b/ssh/include/ssh/channel.hpp @@ -29,14 +29,14 @@ namespace SecureShell return *channel_; } - void close(); + bool close(bool isBackElement = false); /** * @brief Writes data to the channel. * * @param data The data to write */ - void write(std::string data); + bool write(std::string data); struct Dimensions { diff --git a/ssh/include/ssh/file_stream.hpp b/ssh/include/ssh/file_stream.hpp index dc6c4a81..deb89aaa 100644 --- a/ssh/include/ssh/file_stream.hpp +++ b/ssh/include/ssh/file_stream.hpp @@ -98,7 +98,7 @@ namespace SecureShell /** * @brief Closes the file and removes itself from the sftp session. */ - void close(); + void close(bool isBackElement = false); private: std::function makeFileDeleter(); diff --git a/ssh/include/ssh/session.hpp b/ssh/include/ssh/session.hpp index a3d51d6e..b23ef044 100644 --- a/ssh/include/ssh/session.hpp +++ b/ssh/include/ssh/session.hpp @@ -80,10 +80,10 @@ namespace SecureShell std::future, SftpError>> createSftpSession(); private: - void channelRemoveItself(Channel* channel); + void channelRemoveItself(Channel* channel, bool isBackElement); void removeAllChannels(); - void sftpSessionRemoveItself(SftpSession* sftpSession); + void sftpSessionRemoveItself(SftpSession* sftpSession, bool isBackElement); void removeAllSftpSessions(); /** diff --git a/ssh/include/ssh/sftp_error.hpp b/ssh/include/ssh/sftp_error.hpp index c9039839..77cd38f8 100644 --- a/ssh/include/ssh/sftp_error.hpp +++ b/ssh/include/ssh/sftp_error.hpp @@ -13,7 +13,8 @@ namespace SecureShell SharedPtrDestroyed, // This happens when the client does not respect the server max_write_length. // See: https://api.libssh.org/stable/structsftp__limits__struct.html - ShortWrite + ShortWrite, + FileNull, }; struct SftpError diff --git a/ssh/include/ssh/sftp_session.hpp b/ssh/include/ssh/sftp_session.hpp index 797620be..3f3b654e 100644 --- a/ssh/include/ssh/sftp_session.hpp +++ b/ssh/include/ssh/sftp_session.hpp @@ -40,7 +40,7 @@ namespace SecureShell return session_; } - void close(); + bool close(bool isBackElement = false); struct DirectoryEntry : public SharedData::DirectoryEntry { @@ -169,7 +169,7 @@ namespace SecureShell std::future> limits(); private: - void fileStreamRemoveItself(FileStream* stream); + void fileStreamRemoveItself(FileStream* stream, bool isBackElement); void removeAllFileStreams(); private: diff --git a/ssh/source/ssh/async/processing_thread.cpp b/ssh/source/ssh/async/processing_thread.cpp index bb86475e..c3a472f9 100644 --- a/ssh/source/ssh/async/processing_thread.cpp +++ b/ssh/source/ssh/async/processing_thread.cpp @@ -16,24 +16,28 @@ namespace SecureShell { return running_; } - void ProcessingThread::start( - std::chrono::milliseconds const& waitCycleTimeout, - std::chrono::milliseconds const& minimumCycleWait) + void ProcessingThread::start(std::chrono::milliseconds const& minimumCycleWait) { - running_ = true; + { + std::lock_guard lock{taskMutex_}; + running_ = true; + } shuttingDown_ = false; std::promise awaitThreadStart{}; - thread_ = std::thread([this, &awaitThreadStart, waitCycleTimeout, minimumCycleWait] { + thread_ = std::thread([this, &awaitThreadStart, minimumCycleWait] { awaitThreadStart.set_value(); processingThreadId_.store(std::this_thread::get_id()); - run(waitCycleTimeout, minimumCycleWait); + run(minimumCycleWait); }); awaitThreadStart.get_future().wait(); } void ProcessingThread::stop() { shuttingDown_ = true; - running_ = false; + { + std::lock_guard lock{taskMutex_}; + running_ = false; + } if (thread_.joinable()) thread_.join(); @@ -41,8 +45,8 @@ namespace SecureShell std::lock_guard lock{taskMutex_}; while (!tasks_.empty()) { - tasks_.front()(); - tasks_.pop(); + tasks_.back()(); + tasks_.pop_back(); } shuttingDown_ = false; } @@ -60,13 +64,8 @@ namespace SecureShell { std::lock_guard lock{taskMutex_}; - tasks_.push(std::move(task)); + tasks_.push_back(std::move(task)); } - { - std::lock_guard lock{taskWaitMutex_}; - taskAvailable_ = true; - } - taskCondition_.notify_one(); return true; } std::unique_ptr ProcessingThread::createStrand() @@ -101,6 +100,7 @@ namespace SecureShell if (processingPermanents_) { deferredTaskModification_.push_back([this]() { + /* taskMutex_ is held here */ permanentTasks_.clear(); permanentTasksAvailable_ = false; }); @@ -118,6 +118,7 @@ namespace SecureShell if (withinProcessingThread()) { deferredTaskModification_.push_back([this, id]() { + /* taskMutex_ is held here */ permanentTasks_.erase(id); permanentTasksAvailable_ = !permanentTasks_.empty(); }); @@ -125,14 +126,15 @@ namespace SecureShell } else { - std::promise promise{}; - deferredTaskModification_.push_back([this, id, &promise]() { + auto promise = std::make_shared>(); + deferredTaskModification_.push_back([this, id, promise]() { + /* taskMutex_ is held here */ const bool result = permanentTasks_.erase(id) > 0; permanentTasksAvailable_ = !permanentTasks_.empty(); - promise.set_value(result); + promise->set_value(result); }); lock.unlock(); - return promise.get_future().wait_for(std::chrono::seconds{5}) == std::future_status::ready; + return promise->get_future().wait_for(std::chrono::seconds{5}) == std::future_status::ready; } } @@ -140,9 +142,7 @@ namespace SecureShell permanentTasksAvailable_ = !permanentTasks_.empty(); return result > 0; } - void ProcessingThread::run( - std::chrono::milliseconds const& waitCycleTimeout, - std::chrono::milliseconds const& minimumCycleWait) + void ProcessingThread::run(std::chrono::milliseconds const& minimumCycleWait) { auto timePoint = std::chrono::steady_clock::now(); @@ -188,40 +188,33 @@ namespace SecureShell modify(); } } - else + { - std::unique_lock lock(taskWaitMutex_); - if (!taskAvailable_) + std::vector> tasks{}; { - bool result = taskCondition_.wait_for(lock, waitCycleTimeout, [this] { - return taskAvailable_; - }); - if (!result) - continue; + std::lock_guard lock(taskMutex_); + tasks.reserve(std::min(tasks_.size(), maximumTasksProcessableAtOnce)); + std::move( + tasks_.begin(), + tasks_.begin() + std::min(tasks_.size(), maximumTasksProcessableAtOnce), + std::back_inserter(tasks)); + tasks_.erase( + tasks_.begin(), tasks_.begin() + std::min(tasks_.size(), maximumTasksProcessableAtOnce)); } - } - std::vector> tasks{}; - { - std::lock_guard lock(taskMutex_); - for (int i = 0; i != maximumTasksProcessableAtOnce && !tasks_.empty(); ++i) + for (auto const& task : tasks) { - tasks.push_back(std::move(tasks_.front())); - tasks_.pop(); + // Task is checked before adding, shouldnt possibly be empty: +#ifndef N_DEBUG + if (!task) + { + throw std::runtime_error("Task must not be empty."); + } +#endif + task(); } } - for (auto& task : tasks) - { - // Task is checked before adding, shouldnt possibly be empty: - task(); - } - - { - std::lock_guard lock{taskWaitMutex_}; - taskAvailable_ = !tasks_.empty(); - } - if (minimumCycleWait.count() > 0) { auto now = std::chrono::steady_clock::now(); @@ -235,6 +228,7 @@ namespace SecureShell } catch (...) { + std::lock_guard lock{taskMutex_}; running_ = false; } } @@ -245,7 +239,12 @@ namespace SecureShell } bool ProcessingThread::awaitCycle(std::chrono::milliseconds maxWait) { - if (!withinProcessingThread() && running_) + bool running = false; + { + std::lock_guard lock{taskMutex_}; + running = running_; + } + if (!withinProcessingThread() && running) { return pushPromiseTask([]() { return true; diff --git a/ssh/source/ssh/channel.cpp b/ssh/source/ssh/channel.cpp index 46dad315..24cd8438 100644 --- a/ssh/source/ssh/channel.cpp +++ b/ssh/source/ssh/channel.cpp @@ -28,21 +28,27 @@ namespace SecureShell } return *this; } - void Channel::close() + bool Channel::close(bool isBackElement) { - strand_->doFinalSync([this]() { - if (channel_ && channel_->isOpen()) - { - channel_->sendEof(); - channel_->close(); - } - channel_.reset(); - owner_->channelRemoveItself(this); - }); + if (strand_->isFinalized()) + return false; + + return strand_ + ->pushFinalPromiseTask([this, isBackElement]() { + if (channel_ && channel_->isOpen()) + { + channel_->sendEof(); + channel_->close(); + } + channel_.reset(); + owner_->channelRemoveItself(this, isBackElement); + return true; + }) + .get(); } - void Channel::write(std::string data) + bool Channel::write(std::string data) { - strand_->pushTask([this, data = std::move(data)]() { + return strand_->pushTask([this, data = std::move(data)]() { if (channel_) channel_->write(data.data(), data.size()); }); @@ -50,12 +56,15 @@ namespace SecureShell std::future Channel::resizePty(int cols, int rows) { auto promise = std::make_shared>(); - strand_->pushTask([this, cols, rows, promise]() { - if (channel_) - promise->set_value(channel_->changePtySize(cols, rows)); - else - promise->set_value(SSH_ERROR); - }); + if (!strand_->pushTask([this, cols, rows, promise]() { + if (channel_) + promise->set_value(channel_->changePtySize(cols, rows)); + else + promise->set_value(SSH_ERROR); + })) + { + promise->set_value(SSH_ERROR); + } return promise->get_future(); } void Channel::readTask(std::chrono::milliseconds pollTimeout) @@ -63,6 +72,12 @@ namespace SecureShell if (!onStdout_ || !onStderr_ || !onExit_) return; + if (!channel_) + { + std::this_thread::sleep_for(pollTimeout); + return; + } + constexpr static int bufferSize = 1024; char buffer[bufferSize]; diff --git a/ssh/source/ssh/file_stream.cpp b/ssh/source/ssh/file_stream.cpp index 585a23e5..d8195d2a 100644 --- a/ssh/source/ssh/file_stream.cpp +++ b/ssh/source/ssh/file_stream.cpp @@ -5,6 +5,10 @@ namespace SecureShell { +#define VERIFY_FILE_STREAM() \ + if (!file_) \ + return std::unexpected(SftpError{.message = "File is null", .wrapperError = WrapperErrors::FileNull}) + template void FileStream::perform(FunctionT&& func) { @@ -38,11 +42,24 @@ namespace SecureShell , file_{other.file_.release(), makeFileDeleter()} , limits_{other.limits_} {} - void FileStream::close() + void FileStream::close(bool isBackElement) { if (auto sftp = sftp_.lock(); sftp) { - sftp->fileStreamRemoveItself(this); + if (!sftp->strand_->withinProcessingThread()) + { + sftp->performPromise([this, isBackElement, sftp]() -> bool { + file_.reset(); + sftp->fileStreamRemoveItself(this, isBackElement); + return true; + }) + .get(); + } + else + { + file_.reset(); + sftp->fileStreamRemoveItself(this, isBackElement); + } } } std::function FileStream::makeFileDeleter() @@ -70,6 +87,7 @@ namespace SecureShell std::future> FileStream::seek(std::size_t pos) { return performPromise([this, pos]() -> std::expected { + VERIFY_FILE_STREAM(); sftp_seek64(file_.get(), pos); return {}; }); @@ -77,12 +95,14 @@ namespace SecureShell std::future> FileStream::tell() { return performPromise([this]() -> std::expected { + VERIFY_FILE_STREAM(); return static_cast(sftp_tell64(file_.get())); }); } std::future> FileStream::rewind() { return performPromise([this]() -> std::expected { + VERIFY_FILE_STREAM(); sftp_rewind(file_.get()); return {}; }); @@ -104,6 +124,7 @@ namespace SecureShell std::future> FileStream::read(std::byte* buffer, std::size_t bufferSize) { return performPromise([this, buffer, bufferSize]() -> std::expected { + VERIFY_FILE_STREAM(); const auto result = sftp_read(file_.get(), buffer, bufferSize); if (result < 0) return std::unexpected(lastError()); @@ -156,6 +177,14 @@ namespace SecureShell void doRead() { stream.perform([state = shared_from_this()]() { + if (!state->stream.file_) + { + state->promise.set_value(std::unexpected(SftpError{ + .message = "File is null", + .wrapperError = WrapperErrors::FileNull, + })); + return; + } state->onRead(sftp_read(state->stream.file_.get(), state->buffer.data(), state->buffer.size())); }); } @@ -178,6 +207,9 @@ namespace SecureShell std::function&&)> onWriteComplete) { perform([this, toWrite, onWriteComplete = std::move(onWriteComplete)]() { + if (!file_) + return; + const auto written = sftp_write(file_.get(), toWrite.data(), std::min(toWrite.size(), writeLengthLimit())); if (written < 0) @@ -203,6 +235,7 @@ namespace SecureShell if (data.size() <= writeLengthLimit()) { return performPromise([this, data]() -> std::expected { + VERIFY_FILE_STREAM(); const auto written = sftp_write(file_.get(), data.data(), data.size()); if (written < 0) return std::unexpected(lastError()); diff --git a/ssh/source/ssh/session.cpp b/ssh/source/ssh/session.cpp index b7f29863..843df2ac 100644 --- a/ssh/source/ssh/session.cpp +++ b/ssh/source/ssh/session.cpp @@ -7,6 +7,25 @@ namespace SecureShell { + namespace + { + void removeFromContainer(auto& container, auto* ptr, bool isBackElement) + { + if (isBackElement && container.back().get() == ptr) + { + container.pop_back(); + } + else + { + auto it = std::find_if(container.begin(), container.end(), [ptr](const auto& c) { + return c.get() == ptr; + }); + if (it != container.end()) + container.erase(it); + } + } + } + Session::Session() : processingThread_{} , session_{} @@ -15,7 +34,7 @@ namespace SecureShell void Session::start() { - processingThread_.start(std::chrono::milliseconds{100}, std::chrono::milliseconds{1}); + processingThread_.start(std::chrono::milliseconds{1}); } void Session::stop() @@ -32,10 +51,8 @@ namespace SecureShell { removeAllChannels(); removeAllSftpSessions(); - processingThread_.pushTask([this]() { - session_.disconnect(); - }); processingThread_.stop(); + session_.disconnect(); } Session::~Session() @@ -45,46 +62,34 @@ namespace SecureShell void Session::removeAllChannels() { - processingThread_.pushTask([this]() { - for (auto& channel : channels_) - channel->close(); - }); + while (!channels_.empty()) + { + channels_.back()->close(true); + } } void Session::removeAllSftpSessions() { - processingThread_.pushTask([this]() { - for (auto& sftp : sftpSessions_) - sftp->close(); - }); + while (!sftpSessions_.empty()) + { + sftpSessions_.back()->close(false); + } } - void Session::channelRemoveItself(Channel* channel) + void Session::channelRemoveItself(Channel* channel, bool isBackElement) { - processingThread_.pushTask([this, channel]() { - auto it = std::find_if(channels_.begin(), channels_.end(), [channel](const auto& c) { - return c.get() == channel; - }); - if (it != channels_.end()) - channels_.erase(it); - }); + removeFromContainer(channels_, channel, isBackElement); } - void Session::sftpSessionRemoveItself(SftpSession* sftpSession) + void Session::sftpSessionRemoveItself(SftpSession* sftpSession, bool isBackElement) { - processingThread_.pushTask([this, sftpSession]() { - auto it = std::find_if(sftpSessions_.begin(), sftpSessions_.end(), [sftpSession](const auto& s) { - return s.get() == sftpSession; - }); - if (it != sftpSessions_.end()) - sftpSessions_.erase(it); - }); + removeFromContainer(sftpSessions_, sftpSession, isBackElement); } std::future, int>> Session::createPtyChannel(PtyCreationOptions options) { auto promise = std::make_shared, int>>>(); - auto fut = promise->get_future(); - processingThread_.pushTask([this, options = std::move(options), promise = std::move(promise)]() mutable { + + processingThread_.pushTask([this, options = std::move(options), promise]() mutable { auto ptyChannel = std::make_unique(session_); auto& channel = *ptyChannel; auto result = Detail::sequential( @@ -122,7 +127,8 @@ namespace SecureShell channels_.push_back(sharedChannel); return promise->set_value(sharedChannel); }); - return fut; + + return promise->get_future(); } std::future, SftpError>> Session::createSftpSession() diff --git a/ssh/source/ssh/sftp_session.cpp b/ssh/source/ssh/sftp_session.cpp index a32b6103..8be7b803 100644 --- a/ssh/source/ssh/sftp_session.cpp +++ b/ssh/source/ssh/sftp_session.cpp @@ -3,8 +3,6 @@ #include -#include - namespace SecureShell { SftpSession::SftpSession(Session* owner, std::unique_ptr strand, sftp_session session) @@ -18,16 +16,27 @@ namespace SecureShell /* close makes no sense, since this lives in a shared_ptr and will only ever end here when it was already * removed */ } - void SftpSession::close() + bool SftpSession::close(bool isBackElement) { - strand_->doFinalSync([this]() { - removeAllFileStreams(); - owner_->sftpSessionRemoveItself(this); - }); + if (strand_->isFinalized()) + return false; + + return strand_ + ->pushFinalPromiseTask([this, isBackElement]() { + removeAllFileStreams(); + owner_->sftpSessionRemoveItself(this, isBackElement); + return true; + }) + .get(); } - void SftpSession::fileStreamRemoveItself(FileStream* stream) + void SftpSession::fileStreamRemoveItself(FileStream* stream, bool isBackElement) { - perform([this, stream]() { + if (isBackElement && fileStreams_.back().get() == stream) + { + fileStreams_.pop_back(); + } + else + { fileStreams_.erase( std::remove_if( fileStreams_.begin(), @@ -36,14 +45,14 @@ namespace SecureShell return item.get() == stream; }), fileStreams_.end()); - }); + } } void SftpSession::removeAllFileStreams() { - perform([this]() { - for (auto& stream : fileStreams_) - stream->close(); - }); + while (!fileStreams_.empty()) + { + fileStreams_.back()->close(true); + } } SftpSession::DirectoryEntry SftpSession::DirectoryEntry::fromSftpAttributes(sftp_attributes attributes) { @@ -216,18 +225,13 @@ namespace SecureShell { return performPromise( [this, source = std::move(source), destination = std::move(destination)]() -> std::expected { - std::cout << "X" << std::endl; auto s = source.generic_string(); auto d = destination.generic_string(); - std::cout << "Supported: " << sftp_extension_supported(session_, "posix-rename@openssh.com", "1") - << std::endl; - auto result = sftp_rename(session_, s.c_str(), d.c_str()); if (result != SSH_OK) { const auto le = lastError(); - std::cout << "Error: " << le.message << " - " << le.sftpError << " - " << le.sshError << std::endl; return std::unexpected(le); } return {}; diff --git a/ssh/test/ssh/test_processing_thread.hpp b/ssh/test/ssh/test_processing_thread.hpp index a6d5706d..99faefcc 100644 --- a/ssh/test/ssh/test_processing_thread.hpp +++ b/ssh/test/ssh/test_processing_thread.hpp @@ -74,7 +74,7 @@ namespace SecureShell::Test Awaiter awaiter{}; { ProcessingThread processingThread; - processingThread.start(std::chrono::milliseconds{1}, std::chrono::milliseconds{100}); + processingThread.start(std::chrono::milliseconds{1}); for (unsigned int i = 0; i < ProcessingThread::maximumTasksProcessableAtOnce * 3; ++i) { EXPECT_TRUE(processingThread.pushTask([&counter, &awaiter] { diff --git a/ssh/test/ssh/test_sftp.hpp b/ssh/test/ssh/test_sftp.hpp index 1a487978..1c2f26ab 100644 --- a/ssh/test/ssh/test_sftp.hpp +++ b/ssh/test/ssh/test_sftp.hpp @@ -434,4 +434,36 @@ namespace SecureShell::Test EXPECT_EQ(tellResult.value(), 2); } + + TEST_F(SftpTests, CanLetSftpSessionMoveOutOfScope) + { + CREATE_SERVER_AND_JOINER(Sftp); + + { + auto [_, sftp] = createSftpSession(serverStartResult->port); + } + } + + TEST_F(SftpTests, CanLetSftpSessionMoveOutOfScopeWithOpenFile) + { + CREATE_SERVER_AND_JOINER(Sftp); + + { + auto [_, sftp] = createSftpSession(serverStartResult->port); + auto fut = + sftp->openFile("/home/test/file1.txt", SftpSession::OpenType::Read, std::filesystem::perms::owner_read); + ASSERT_EQ(fut.wait_for(1s), std::future_status::ready); + auto result = fut.get(); + ASSERT_TRUE(result.has_value()); + } + } + + TEST_F(SftpTests, CannotCloseSftpSessionTwice) + { + CREATE_SERVER_AND_JOINER(Sftp); + + auto [_, sftp] = createSftpSession(serverStartResult->port); + ASSERT_TRUE(sftp->close()); + EXPECT_FALSE(sftp->close()); + } } diff --git a/ssh/test/ssh/test_ssh_session.hpp b/ssh/test/ssh/test_ssh_session.hpp index 04f86e3c..6cef2699 100644 --- a/ssh/test/ssh/test_ssh_session.hpp +++ b/ssh/test/ssh/test_ssh_session.hpp @@ -97,6 +97,27 @@ namespace SecureShell::Test processThread.join(); }}; + auto sessionScope = [this, &result]() { + auto expectedSession = makePasswordTestSession(result->port); + + ASSERT_TRUE(expectedSession.has_value()); + auto session = std::move(expectedSession).value(); + session->start(); + }; + + ASSERT_NO_FATAL_FAILURE(sessionScope()); + } + + TEST_F(SshSessionTests, CanConnectToSshServerCreateChannelAndDestructWithoutCrash) + { + auto [result, processThread] = createSshServer(); + ASSERT_TRUE(result); + auto joiner = Nui::ScopeExit{[&]() noexcept { + result->command("exit"); + if (processThread.joinable()) + processThread.join(); + }}; + auto sessionScope = [this, &result]() { auto expectedSession = makePasswordTestSession(result->port); @@ -394,4 +415,106 @@ namespace SecureShell::Test ASSERT_NO_FATAL_FAILURE(sessionScope()); } + + TEST_F(SshSessionTests, CannotStartReadingOnClosedChannel) + { + auto [result, processThread] = createSshServer(); + ASSERT_TRUE(result); + auto joiner = Nui::ScopeExit{[&]() noexcept { + result->command("exit"); + if (processThread.joinable()) + processThread.join(); + }}; + + auto expectedSession = makePasswordTestSession(result->port); + + ASSERT_TRUE(expectedSession.has_value()); + auto session = std::move(expectedSession).value(); + session->start(); + + auto expectedChannel = session->createPtyChannel({}).get(); + ASSERT_TRUE(expectedChannel.has_value()); + auto channel = expectedChannel.value().lock(); + + channel->close(); + + std::promise awaiter{}; + EXPECT_THROW(channelStartReading(channel, awaiter), std::runtime_error); + } + + TEST_F(SshSessionTests, CannotWriteAfterChannelClose) + { + auto [result, processThread] = createSshServer(); + ASSERT_TRUE(result); + auto joiner = Nui::ScopeExit{[&]() noexcept { + result->command("exit"); + if (processThread.joinable()) + processThread.join(); + }}; + + auto expectedSession = makePasswordTestSession(result->port); + + ASSERT_TRUE(expectedSession.has_value()); + auto session = std::move(expectedSession).value(); + session->start(); + + auto expectedChannel = session->createPtyChannel({}).get(); + ASSERT_TRUE(expectedChannel.has_value()); + auto channel = expectedChannel.value().lock(); + + channel->close(); + + EXPECT_EQ(channel->write("ls -lah"), false); + } + + TEST_F(SshSessionTests, CannotResizePtyAfterChannelClose) + { + auto [result, processThread] = createSshServer(); + ASSERT_TRUE(result); + auto joiner = Nui::ScopeExit{[&]() noexcept { + result->command("exit"); + if (processThread.joinable()) + processThread.join(); + }}; + + auto expectedSession = makePasswordTestSession(result->port); + + ASSERT_TRUE(expectedSession.has_value()); + auto session = std::move(expectedSession).value(); + session->start(); + + auto expectedChannel = session->createPtyChannel({}).get(); + ASSERT_TRUE(expectedChannel.has_value()); + auto channel = expectedChannel.value().lock(); + + channel->close(); + + auto resizeFut = channel->resizePty(80, 24); + ASSERT_EQ(resizeFut.wait_for(5s), std::future_status::ready); + EXPECT_EQ(resizeFut.get(), SSH_ERROR); + } + + TEST_F(SshSessionTests, CannotCloseChannelTwice) + { + auto [result, processThread] = createSshServer(); + ASSERT_TRUE(result); + auto joiner = Nui::ScopeExit{[&]() noexcept { + result->command("exit"); + if (processThread.joinable()) + processThread.join(); + }}; + + auto expectedSession = makePasswordTestSession(result->port); + + ASSERT_TRUE(expectedSession.has_value()); + auto session = std::move(expectedSession).value(); + session->start(); + + auto expectedChannel = session->createPtyChannel({}).get(); + ASSERT_TRUE(expectedChannel.has_value()); + auto channel = expectedChannel.value().lock(); + + channel->close(); + EXPECT_FALSE(channel->close()); + } } From d79f027133062773aaeaa8a9cc5c346d079c7541 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Sat, 8 Mar 2025 17:34:20 +0100 Subject: [PATCH 08/42] Made file explorer button border more visible. --- nui-file-explorer/styles/dropdown_menu.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nui-file-explorer/styles/dropdown_menu.css b/nui-file-explorer/styles/dropdown_menu.css index 79f848df..2452a208 100644 --- a/nui-file-explorer/styles/dropdown_menu.css +++ b/nui-file-explorer/styles/dropdown_menu.css @@ -35,7 +35,7 @@ .nui-file-grid-head-dropdown-button { transition: filter 0.3s ease; - border-left: 1px solid color-mix(in srgb, var(--nui-file-grid-border-color), white 20%); + border-left: 1px solid color-mix(in srgb, var(--nui-file-grid-border-color), white 35%); background-color: var(--nui-file-grid-dropdown-background); padding-left: 8px; padding-right: 8px; From 8c6446099a9eddc95a390dba856b6b1265b235cf Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Sun, 9 Mar 2025 18:08:42 +0100 Subject: [PATCH 09/42] Started implementation of download operation class. --- CMakeLists.txt | 1 + .../backend/sftp/download_operation.hpp | 83 ++++ backend/include/backend/sftp/operation.hpp | 77 +++ .../include/backend/sftp/operation_queue.hpp | 16 + .../include/backend/ssh_session_manager.hpp | 13 +- backend/source/backend/CMakeLists.txt | 37 +- backend/source/backend/pty/windows/conpty.cpp | 1 + .../backend/sftp/download_operation.cpp | 350 +++++++++++++ .../source/backend/sftp/operation_queue.cpp | 0 .../source/backend/ssh_session_manager.cpp | 9 +- backend/test/backend/CMakeLists.txt | 41 ++ backend/test/backend/main.cpp | 19 + .../test/backend/test_download_operation.hpp | 464 ++++++++++++++++++ .../session_components/operation_queue.hpp | 27 + frontend/source/frontend/CMakeLists.txt | 1 + frontend/source/frontend/session.cpp | 27 +- .../session_components/operation_queue.cpp | 55 +++ ids/include/ids/ids.hpp | 4 +- log/include/log/logger_backend.hpp | 6 +- .../include/shared_data/directory_entry.hpp | 34 +- ssh/include/ssh/async/processing_strand.hpp | 2 +- ssh/include/ssh/async/processing_thread.hpp | 1 - ssh/include/ssh/file_information.hpp | 13 + ssh/include/ssh/file_stream.hpp | 38 +- ssh/include/ssh/file_stream_interface.hpp | 119 +++++ ssh/include/ssh/mocks/file_stream_mock.hpp | 41 ++ ssh/include/ssh/sftp_session.hpp | 18 +- ssh/source/ssh/file_information.cpp | 31 ++ ssh/source/ssh/file_stream.cpp | 24 +- ssh/source/ssh/sftp_session.cpp | 42 +- ssh/test/ssh/CMakeLists.txt | 1 + ssh/test/ssh/common_fixture.hpp | 2 +- ssh/test/ssh/test_processing_thread.hpp | 2 +- ssh/test/ssh/test_sftp.hpp | 28 +- ssh/test/ssh/utility/node.hpp | 2 +- ssh/test/ssh/utility/temporary_directory.hpp | 80 --- .../include}/utility/awaiter.hpp | 0 .../include/utility/temporary_directory.hpp | 69 ++- .../source/utility/temporary_directory.cpp | 75 +-- 39 files changed, 1574 insertions(+), 279 deletions(-) create mode 100644 backend/include/backend/sftp/download_operation.hpp create mode 100644 backend/include/backend/sftp/operation.hpp create mode 100644 backend/include/backend/sftp/operation_queue.hpp create mode 100644 backend/source/backend/sftp/download_operation.cpp create mode 100644 backend/source/backend/sftp/operation_queue.cpp create mode 100644 backend/test/backend/CMakeLists.txt create mode 100644 backend/test/backend/main.cpp create mode 100644 backend/test/backend/test_download_operation.hpp create mode 100644 frontend/include/frontend/session_components/operation_queue.hpp create mode 100644 frontend/source/frontend/session_components/operation_queue.cpp create mode 100644 ssh/include/ssh/file_information.hpp create mode 100644 ssh/include/ssh/file_stream_interface.hpp create mode 100644 ssh/include/ssh/mocks/file_stream_mock.hpp create mode 100644 ssh/source/ssh/file_information.cpp delete mode 100644 ssh/test/ssh/utility/temporary_directory.hpp rename {ssh/test/ssh => utility/include}/utility/awaiter.hpp (100%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6775509b..4e7f68e4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -69,5 +69,6 @@ else() include(./_cmake/dependencies/gtest.cmake) enable_testing() add_subdirectory(ssh/test/ssh) + add_subdirectory(backend/test/backend) endif() endif() \ No newline at end of file diff --git a/backend/include/backend/sftp/download_operation.hpp b/backend/include/backend/sftp/download_operation.hpp new file mode 100644 index 00000000..46e2aa21 --- /dev/null +++ b/backend/include/backend/sftp/download_operation.hpp @@ -0,0 +1,83 @@ +#pragma once + +#include +#include +#include + +#include +#include +#include + +class DownloadOperation : public Operation +{ + public: + struct DownloadOperationOptions + { + std::function progressCallback = + [](auto, auto, auto) {}; + std::function onCompletionCallback = [](auto) {}; + std::filesystem::path remotePath{}; + std::filesystem::path localPath{}; + std::string tempFileSuffix{".filepart"}; + bool mayOverwrite{false}; + bool reserveSpace{false}; + bool tryContinue{false}; + bool inheritPermissions{false}; + bool doCleanup{true}; + std::optional permissions{std::nullopt}; + std::chrono::seconds futureTimeout{5}; + }; + + DownloadOperation(std::weak_ptr fileStream, DownloadOperationOptions options); + ~DownloadOperation() override; + DownloadOperation(DownloadOperation const&) = delete; + DownloadOperation(DownloadOperation&&) = delete; + DownloadOperation& operator=(DownloadOperation const&) = delete; + DownloadOperation& operator=(DownloadOperation&&) = delete; + + std::expected prepare() override; + std::expected start() override; + void cancel() override; + std::expected pause() override; + std::expected finalize() override; + + private: + std::expected openOrAdoptFile(SecureShell::IFileStream& stream); + + template + bool perform(FunctionT&& func) + { + auto stream = fileStream_.lock(); + if (!stream) + return false; + + auto* strand = stream->strand(); + if (!strand) + return false; + + return strand->pushTask(std::forward(func)); + } + + void cleanup(); + + private: + std::recursive_mutex mutex_; + std::weak_ptr fileStream_; + std::filesystem::path remotePath_; + std::filesystem::path localPath_; + std::string tempFileSuffix_; + std::function progressCallback_; + std::function onCompletionCallback_; + bool mayOverwrite_; + bool reserveSpace_; + bool tryContinue_; + bool inheritPermissions_; + bool doCleanup_; + bool preparationDone_; + std::atomic_bool interuptRead_; + std::optional permissions_; + std::ofstream localFile_; + std::uint64_t fileSize_; + std::future> readFuture_; + std::chrono::seconds futureTimeout_; +}; \ No newline at end of file diff --git a/backend/include/backend/sftp/operation.hpp b/backend/include/backend/sftp/operation.hpp new file mode 100644 index 00000000..6515067f --- /dev/null +++ b/backend/include/backend/sftp/operation.hpp @@ -0,0 +1,77 @@ +#pragma once + +#include +#include + +#include + +#include +#include + +BOOST_DEFINE_ENUM_CLASS( + OperationErrorType, + FileExists, + FileNotFound, + OpenFailure, + FileStreamExpired, + FileStatFailed, + SftpError, + InvalidPath, + RenameFailure, + CannotSetFilePermissions, + FutureTimeout, + OperationNotPrepared); + +class Operation +{ + public: + Operation() + : id_{Ids::generateOperationId()} + {} + Operation(Operation const&) = delete; + Operation& operator=(Operation const&) = delete; + Operation(Operation&&) = default; + Operation& operator=(Operation&&) = default; + + using ErrorType = OperationErrorType; + + struct Error + { + ErrorType type; + std::optional sftpError = std::nullopt; + }; + + Ids::OperationId id() const + { + return id_; + } + + virtual ~Operation() = default; + + virtual std::expected prepare() = 0; + + /** + * @brief Starts the operation, can also be used to resume cancelled operations. + * + * @return std::expected + */ + virtual std::expected start() = 0; + + /** + * @brief Cancels the operation and possibly does some cleanup. + */ + virtual void cancel() = 0; + + /** + * @brief Pauses the operation. + */ + virtual std::expected pause() = 0; + + /** + * @brief Finalizes the operation. This should be called after the operation is done (progress == max). + */ + virtual std::expected finalize() = 0; + + private: + Ids::OperationId id_; +}; \ No newline at end of file diff --git a/backend/include/backend/sftp/operation_queue.hpp b/backend/include/backend/sftp/operation_queue.hpp new file mode 100644 index 00000000..72aec0c2 --- /dev/null +++ b/backend/include/backend/sftp/operation_queue.hpp @@ -0,0 +1,16 @@ +#pragma once + +#include +#include + +class FileOperationQueue +{ + public: + FileOperationQueue(); + + void pause(); + void resume(); + void cancelAll(bool clean); + + private: +}; \ No newline at end of file diff --git a/backend/include/backend/ssh_session_manager.hpp b/backend/include/backend/ssh_session_manager.hpp index 58a0624d..f23d51ea 100644 --- a/backend/include/backend/ssh_session_manager.hpp +++ b/backend/include/backend/ssh_session_manager.hpp @@ -54,12 +54,23 @@ class SshSessionManager void registerRpcSftpCreateDirectory(Nui::Window&, Nui::RpcHub& hub); void registerRpcSftpCreateFile(Nui::Window&, Nui::RpcHub& hub); + // Sftp Files: + private: std::mutex passwordProvidersMutex_{}; std::mutex addSessionMutex_{}; std::unordered_map, Ids::IdHash> sessions_{}; std::unordered_map, Ids::IdHash> channels_{}; - std::unordered_map, Ids::IdHash> sftpChannels_{}; + struct SftpSession + { + std::weak_ptr sftp{}; + std::unordered_map fileStreams{}; + + SftpSession(std::weak_ptr sftp) + : sftp{std::move(sftp)} + {} + }; + std::unordered_map sftpChannels_{}; std::map passwordProviders_{}; std::unique_ptr addSessionThread_{}; std::vector pwCache_{}; diff --git a/backend/source/backend/CMakeLists.txt b/backend/source/backend/CMakeLists.txt index b81aa3f8..8cf06463 100644 --- a/backend/source/backend/CMakeLists.txt +++ b/backend/source/backend/CMakeLists.txt @@ -1,40 +1,47 @@ -target_sources( - ${PROJECT_NAME} - PRIVATE +add_library( + backend + STATIC password/password_prompter.cpp process/process.cpp process/process_store.cpp process/environment.cpp ssh_session_manager.cpp - main.cpp + sftp/operation_queue.cpp + sftp/download_operation.cpp ) if (WIN32) target_sources( - ${PROJECT_NAME} + backend PRIVATE pty/windows/conpty.cpp pty/windows/pipe.cpp ) else() target_sources( - ${PROJECT_NAME} + backend PRIVATE pty/linux/pty.cpp ) endif() +target_sources( + ${PROJECT_NAME} + PRIVATE + main.cpp +) + find_package(Boost CONFIG 1.86.0 REQUIRED COMPONENTS system filesystem asio) -target_include_directories(${PROJECT_NAME} PRIVATE "${CMAKE_SOURCE_DIR}/backend/include") +target_include_directories(backend PUBLIC "${CMAKE_SOURCE_DIR}/backend/include") -target_compile_definitions(${PROJECT_NAME} PRIVATE -DSSH_NO_CPP_EXCEPTIONS) +target_compile_definitions(backend PUBLIC -DSSH_NO_CPP_EXCEPTIONS) include("${CMAKE_CURRENT_LIST_DIR}/../../../_cmake/dependencies/libssh.cmake") # Link backend of nui outside of emscripten target_link_libraries( - ${PROJECT_NAME} + backend PRIVATE nui-backend efsw-static @@ -51,8 +58,16 @@ target_link_libraries( events ) +target_link_libraries( + ${PROJECT_NAME} + PRIVATE + backend +) + nui_set_target_output_directories(${PROJECT_NAME}) +add_dependencies(backend scp-resource-copy) + # Creates a target that is compiled through emscripten. This target becomes the frontend part. nui_add_emscripten_target( TARGET @@ -65,6 +80,4 @@ nui_add_emscripten_target( CMAKE_OPTIONS # I recommend to work with a release build by default because debug builds get big fast. -DCMAKE_BUILD_TYPE=Release -) - -add_dependencies(${PROJECT_NAME} scp-resource-copy) \ No newline at end of file +) \ No newline at end of file diff --git a/backend/source/backend/pty/windows/conpty.cpp b/backend/source/backend/pty/windows/conpty.cpp index 14b9efff..8c546b77 100644 --- a/backend/source/backend/pty/windows/conpty.cpp +++ b/backend/source/backend/pty/windows/conpty.cpp @@ -47,6 +47,7 @@ namespace ConPTY #pragma clang diagnostic push // This is winapi for you. #pragma clang diagnostic ignored "-Wcast-function-type-strict" +#pragma clang diagnostic ignored "-Wcast-function-type-mismatch" , createPseudoConsole{reinterpret_cast( GetProcAddress(kernel32, "CreatePseudoConsole"))} , resizePseudoConsole{reinterpret_cast( diff --git a/backend/source/backend/sftp/download_operation.cpp b/backend/source/backend/sftp/download_operation.cpp new file mode 100644 index 00000000..94e03976 --- /dev/null +++ b/backend/source/backend/sftp/download_operation.cpp @@ -0,0 +1,350 @@ +#include + +#include + +DownloadOperation::DownloadOperation( + std::weak_ptr fileStream, + DownloadOperationOptions options) + : Operation{} + , mutex_{} + , fileStream_{std::move(fileStream)} + , remotePath_{std::move(options.remotePath)} + , localPath_{std::move(options.localPath)} + , tempFileSuffix_{std::move(options.tempFileSuffix)} + , progressCallback_{std::move(options.progressCallback)} + , onCompletionCallback_{[this, cb = std::move(options.onCompletionCallback), once = false](bool success) mutable { + if (!cb) + throw std::invalid_argument("onCompletionCallback must be set."); + if (once) + return; + once = true; + progressCallback_(0, fileSize_, fileSize_); + cb(success); + }} + , mayOverwrite_{options.mayOverwrite} + , reserveSpace_{options.reserveSpace} + , tryContinue_{options.tryContinue} + , inheritPermissions_{options.inheritPermissions} + , doCleanup_{options.doCleanup} + , preparationDone_{false} + , interuptRead_{false} + , permissions_{options.permissions} + , localFile_{} + , fileSize_{0} + , readFuture_{} + , futureTimeout_{options.futureTimeout} +{ + if (tempFileSuffix_.empty()) + tempFileSuffix_ = ".filepart"; + if (tempFileSuffix_.find('/') != 0) + tempFileSuffix_ = ".filepart"; +} + +DownloadOperation::~DownloadOperation() +{ + cleanup(); + + if (auto stream = fileStream_.lock(); stream) + { + // wait for all tasks of the operation to finish + stream->strand() + ->pushPromiseTask([]() { + return true; + }) + .get(); + } +} + +std::expected DownloadOperation::openOrAdoptFile(SecureShell::IFileStream& stream) +{ + std::scoped_lock lock{mutex_}; + + const auto tempPath = localPath_.generic_string() + tempFileSuffix_; + + if (tryContinue_ && std::filesystem::exists(tempPath)) + { + localFile_.open(tempPath, std::ios::binary | std::ios::app); + if (!localFile_.is_open()) + { + Log::error("DownloadOperation: Failed to open file for appending: {}", tempPath); + return std::unexpected(Error{.type = ErrorType::OpenFailure}); + } + + // File complete but not renamed? just rename it in the finalize() step + if (static_cast(localFile_.tellp()) == fileSize_) + { + Log::info("DownloadOperation: File '{}' already complete, will be renamed in finalize() step.", tempPath); + localFile_.close(); + return {}; + } + // File is larger than expected? discard it and start over. + else if (static_cast(localFile_.tellp()) > fileSize_) + { + Log::info("DownloadOperation: File '{}' is larger than expected, discarding and starting over.", tempPath); + localFile_.close(); + // Reset the file + localFile_.open(tempPath, std::ios::binary | std::ios::trunc); + } + else + { + Log::info("DownloadOperation: File '{}' is incomplete, continuing download.", tempPath); + // Seek stream to position: + auto seekResult = stream.seek(localFile_.tellp()).get(); + if (!seekResult.has_value()) + return std::unexpected(Error{.type = ErrorType::FileStatFailed}); + } + } + else + { + Log::info("DownloadOperation: Starting new download to '{}'.", tempPath); + localFile_.open(tempPath, std::ios::binary | std::ios::trunc); + } + + if (!localFile_.is_open()) + { + Log::error("DownloadOperation: Failed to open file: {}", tempPath); + return std::unexpected(Error{.type = ErrorType::OpenFailure}); + } + + return {}; +} + +std::expected DownloadOperation::prepare() +{ + std::scoped_lock lock{mutex_}; + + if (localPath_.empty()) + { + Log::error("DownloadOperation: Invalid local path."); + return std::unexpected(Error{.type = ErrorType::InvalidPath}); + } + + // Initial check. Check again later before rename + if (std::filesystem::exists(localPath_)) + { + if (!mayOverwrite_) + { + Log::error( + "DownloadOperation: File '{}' already exists and may not be overwritten.", localPath_.generic_string()); + return std::unexpected(Error{.type = ErrorType::FileExists}); + } + } + + auto stream = fileStream_.lock(); + if (!stream) + { + Log::error("DownloadOperation: File stream expired."); + return std::unexpected(Error{.type = ErrorType::FileStreamExpired}); + } + + const auto fileInfo = stream->stat().get(); + if (!fileInfo.has_value()) + { + Log::error("DownloadOperation: Failed to stat file."); + return std::unexpected(Error{.type = ErrorType::FileStatFailed, .sftpError = fileInfo.error()}); + } + + fileSize_ = fileInfo->size; + + auto openResult = openOrAdoptFile(*stream); + if (!openResult.has_value()) + { + Log::error("DownloadOperation: Failed to open file."); + return std::unexpected(std::move(openResult).error()); + } + + if (reserveSpace_ && fileSize_ != 0) + { + // Reserve space + Log::info("DownloadOperation: Reserving space for file."); + const auto pos = localFile_.tellp(); + localFile_.seekp(fileSize_ - 1); + localFile_.put('\0'); + if (localFile_.fail()) + return std::unexpected(Error{.type = ErrorType::OpenFailure}); + localFile_.seekp(pos); + } + + Log::info( + "DownloadOperation: Prepared download of '{}' to '{}'.", + remotePath_.generic_string(), + localPath_.generic_string()); + + preparationDone_ = true; + return {}; +} +std::expected DownloadOperation::start() +{ + std::scoped_lock lock{mutex_}; + interuptRead_ = false; + + if (!preparationDone_) + { + Log::error("DownloadOperation: Operation not prepared."); + return std::unexpected(Error{.type = ErrorType::OperationNotPrepared}); + } + + if (!localFile_.is_open()) + { + Log::error("DownloadOperation: File is not open."); + return std::unexpected(Error{.type = ErrorType::OpenFailure}); + } + + if (fileSize_ == 0) + { + Log::info("DownloadOperation: Remote file is empty, nothing to do."); + return {}; + } + + auto stream = fileStream_.lock(); + if (!stream) + { + Log::error("DownloadOperation: File stream expired."); + return std::unexpected(Error{.type = ErrorType::FileStreamExpired}); + } + + readFuture_ = stream->read([this](std::string_view data) { + if (data.size() == 0) + { + Log::info("DownloadOperation: Remote file read complete or error."); + { + std::scoped_lock lock{mutex_}; + localFile_.close(); + } + // Defer completion by pushing it onto the strand. + perform([this]() { + std::scoped_lock lock{mutex_}; + onCompletionCallback_(true); + }); + return false; + } + + std::scoped_lock lock{mutex_}; + localFile_.write(data.data(), data.size()); + progressCallback_(0, fileSize_, localFile_.tellp()); + const auto doContinue = localFile_.good() || interuptRead_; + if (!doContinue) + { + if (!localFile_.good()) + { + Log::error("DownloadOperation read cycle stopped: localFile_.good() == false"); + perform([this]() { + std::scoped_lock lock{mutex_}; + onCompletionCallback_(false); + }); + } + } + return doContinue; + }); + + return {}; +} +void DownloadOperation::cancel() +{ + std::scoped_lock lock{mutex_}; + + Log::info( + "DownloadOperation: Download of '{}' to '{}' canceled.", + remotePath_.generic_string(), + localPath_.generic_string()); + + perform([this]() { + std::scoped_lock lock{mutex_}; + onCompletionCallback_(false); + }); + + cleanup(); +} +void DownloadOperation::cleanup() +{ + std::scoped_lock lock{mutex_}; + localFile_.close(); + + if (doCleanup_ && std::filesystem::exists(localPath_.generic_string() + tempFileSuffix_)) + std::filesystem::remove(localPath_.generic_string() + tempFileSuffix_); + + if (auto stream = fileStream_.lock(); stream) + stream->close(false); +} +std::expected DownloadOperation::pause() +{ + std::scoped_lock lock{mutex_}; + interuptRead_ = true; + + if (readFuture_.wait_for(futureTimeout_) != std::future_status::ready) + { + Log::error("DownloadOperation: Failed to pause download."); + return std::unexpected(Error{.type = ErrorType::FutureTimeout}); + } + return {}; +} +std::expected DownloadOperation::finalize() +{ + std::scoped_lock lock{mutex_}; + + if (readFuture_.wait_for(futureTimeout_) != std::future_status::ready) + { + Log::error("DownloadOperation: Failed to wait for download completion."); + return std::unexpected(Error{.type = ErrorType::FutureTimeout}); + } + + localFile_.close(); + + if (std::filesystem::exists(localPath_) && !mayOverwrite_) + { + Log::error( + "DownloadOperation: File '{}' already exists and may not be overwritten.", localPath_.generic_string()); + return std::unexpected(Error{.type = ErrorType::FileExists}); + } + + std::error_code ec{}; + std::filesystem::rename(localPath_.generic_string() + tempFileSuffix_, localPath_, ec); + if (ec) + { + Log::error("DownloadOperation: Failed to rename file: {}", ec.message()); + return std::unexpected(Error{.type = ErrorType::RenameFailure}); + } + + if (inheritPermissions_) + { + Log::info("DownloadOperation: Inheriting permissions from remote file."); + auto stream = fileStream_.lock(); + if (!stream) + { + Log::error("DownloadOperation: File stream expired."); + return std::unexpected(Error{.type = ErrorType::FileStreamExpired}); + } + + const auto fileInfo = stream->stat().get(); + if (!fileInfo.has_value()) + { + Log::error("DownloadOperation: Failed to stat file."); + return std::unexpected(Error{.type = ErrorType::FileStatFailed, .sftpError = fileInfo.error()}); + } + + std::error_code permissionsError{}; + std::filesystem::permissions(localPath_, fileInfo->permissions, permissionsError); + if (permissionsError) + { + Log::error("DownloadOperation: Failed to set permissions: {}", permissionsError.message()); + return std::unexpected(Error{.type = ErrorType::CannotSetFilePermissions}); + } + } + else if (permissions_) + { + Log::info("DownloadOperation: Setting permissions."); + std::error_code permissionsError{}; + std::filesystem::permissions(localPath_, *permissions_, permissionsError); + if (permissionsError) + { + Log::error("DownloadOperation: Failed to set permissions: {}", permissionsError.message()); + return std::unexpected(Error{.type = ErrorType::CannotSetFilePermissions}); + } + } + + Log::info( + "DownloadOperation: Finalized download of '{}' to '{}'.", + remotePath_.generic_string(), + localPath_.generic_string()); + return {}; +} \ No newline at end of file diff --git a/backend/source/backend/sftp/operation_queue.cpp b/backend/source/backend/sftp/operation_queue.cpp new file mode 100644 index 00000000..e69de29b diff --git a/backend/source/backend/ssh_session_manager.cpp b/backend/source/backend/ssh_session_manager.cpp index bc2a7d49..0777a378 100644 --- a/backend/source/backend/ssh_session_manager.cpp +++ b/backend/source/backend/ssh_session_manager.cpp @@ -311,8 +311,9 @@ void SshSessionManager::registerRpcChannelClose(Nui::Window&, Nui::RpcHub& hub) } else if (auto iter = sftpChannels_.find(channelId); iter != sftpChannels_.end()) { - if (auto channel = iter->second.lock(); channel) + if (auto channel = iter->second.sftp.lock(); channel) { + iter->second.fileStreams.clear(); channel->close(); } sftpChannels_.erase(iter); @@ -487,7 +488,7 @@ void SshSessionManager::registerRpcSftpListDirectory(Nui::Window&, Nui::RpcHub& return; } - auto locked = channel->second.lock(); + auto locked = channel->second.sftp.lock(); if (!locked) { Log::error("Failed to lock sftp channel with id: {}", channelId.value()); @@ -551,7 +552,7 @@ void SshSessionManager::registerRpcSftpCreateDirectory(Nui::Window&, Nui::RpcHub return; } - auto locked = channel->second.lock(); + auto locked = channel->second.sftp.lock(); if (!locked) { Log::error("Failed to lock sftp channel with id: {}", channelId.value()); @@ -615,7 +616,7 @@ void SshSessionManager::registerRpcSftpCreateFile(Nui::Window&, Nui::RpcHub& hub return; } - auto locked = channel->second.lock(); + auto locked = channel->second.sftp.lock(); if (!locked) { Log::error("Failed to lock sftp channel with id: {}", channelId.value()); diff --git a/backend/test/backend/CMakeLists.txt b/backend/test/backend/CMakeLists.txt new file mode 100644 index 00000000..d320906a --- /dev/null +++ b/backend/test/backend/CMakeLists.txt @@ -0,0 +1,41 @@ +include(GoogleTest) + +add_executable( + nui-scp-backend-test + main.cpp +) + +find_package(GTest CONFIG REQUIRED) + +find_package(Boost CONFIG REQUIRED COMPONENTS filesystem asio system process) + +target_link_libraries( + nui-scp-backend-test + PUBLIC + secure-shell + backend + GTest::GTest + GTest::gmock_main + GTest::Main + Boost::filesystem + Boost::asio + Boost::system + Boost::process +) + +if (WIN32) + target_link_libraries(nui-scp-backend-test PUBLIC ws2_32) +endif() + +set_target_properties(nui-scp-backend-test PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/test") + +gtest_discover_tests(nui-scp-backend-test) + +# If msys2, copy dynamic libraries to executable directory, visual studio does this automatically. +# And there is no need on linux. +if (DEFINED ENV{MSYSTEM}) + add_custom_command(TARGET nui-scp-backend-test POST_BUILD + COMMAND bash -c "ldd $" | "grep" "clang" | awk "NF == 4 { system(\"${CMAKE_COMMAND} -E copy \" \$3 \" $\") }" + VERBATIM + ) +endif() \ No newline at end of file diff --git a/backend/test/backend/main.cpp b/backend/test/backend/main.cpp new file mode 100644 index 00000000..cfe644c1 --- /dev/null +++ b/backend/test/backend/main.cpp @@ -0,0 +1,19 @@ +#include "test_download_operation.hpp" + +#include + +#include + +#include + +std::filesystem::path programDirectory; + +int main(int argc, char** argv) +{ + Log::setLevel(Log::Level::Off); + + programDirectory = std::filesystem::path{argv[0]}.parent_path(); + + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} \ No newline at end of file diff --git a/backend/test/backend/test_download_operation.hpp b/backend/test/backend/test_download_operation.hpp new file mode 100644 index 00000000..66fc1523 --- /dev/null +++ b/backend/test/backend/test_download_operation.hpp @@ -0,0 +1,464 @@ +#pragma once + +#include +#include +#include + +#include + +#include + +using namespace std::chrono_literals; +using namespace std::string_literals; + +extern std::filesystem::path programDirectory; + +namespace Test +{ + class DownloadOperationTests : public ::testing::Test + { + protected: + void SetUp() override + { + processingThread_.start(5ms); + } + + void TearDown() override + { + processingThread_.stop(); + } + + std::shared_ptr<::testing::NiceMock> makeFileStreamMock() + { + auto mock = std::make_shared<::testing::NiceMock>(); + + ON_CALL(*mock, strand()).WillByDefault([this]() -> SecureShell::ProcessingStrand* { + return strand_.get(); + }); + + return mock; + } + + void giveMockDefaultStat( + std::shared_ptr<::testing::NiceMock> const& mock, + std::optional size = std::nullopt) + { + if (!size) + size = fakeFileContent_.size(); + + ON_CALL(*mock, stat()) + .WillByDefault( + [size = size.value()]() + -> std::future> { + std::promise> promise; + promise.set_value(SecureShell::FileInformation{{ + .size = size, + .permissions = std::filesystem::perms::owner_all, + }}); + return promise.get_future(); + }); + } + + void giveMockExpectedRead(std::shared_ptr<::testing::NiceMock> const& mock) + { + EXPECT_CALL(*mock, read(testing::_)) + .WillRepeatedly( + [this](std::function cb) + -> std::future> { + onRead_ = std::move(cb); + readPromise_ = {}; + return readPromise_.get_future(); + }); + } + + void fakeReadCycle(std::optional chunkSizeOpt) + { + auto chunkSize = fakeFileContent_.size(); + if (chunkSizeOpt) + chunkSize = chunkSizeOpt.value(); + + if (readOffset_ + chunkSize > fakeFileContent_.size()) + chunkSize = fakeFileContent_.size() - readOffset_; + + // EOF: + if (chunkSize == 0) + { + readPromise_.set_value(readOffset_); + onRead_({}); + return; + } + + if (onRead_) + { + onRead_(std::string_view{fakeFileContent_}.substr(readOffset_, chunkSize)); + readOffset_ += chunkSize; + } + else + { + throw std::runtime_error("No read callback set."); + } + } + + protected: + std::string fakeFileContent_{"Hello, World!"}; + Utility::TemporaryDirectory isolateDirectory_{programDirectory / "temp", true}; + SecureShell::ProcessingThread processingThread_{}; + std::unique_ptr strand_{processingThread_.createStrand()}; + std::promise> readPromise_{}; + std::function onRead_{}; + std::size_t readOffset_{0}; + }; + + TEST_F(DownloadOperationTests, CanCreateDownloadOperation) + { + auto fileStream = makeFileStreamMock(); + auto options = DownloadOperation::DownloadOperationOptions{}; + DownloadOperation operation{fileStream, options}; + } + + TEST_F(DownloadOperationTests, DownloadPreparationFailsWithLocalPathEmpty) + { + auto fileStream = makeFileStreamMock(); + auto options = DownloadOperation::DownloadOperationOptions{}; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error().type, DownloadOperation::ErrorType::InvalidPath); + } + + TEST_F(DownloadOperationTests, DownloadPreparationFailsWhenFileExistsAndMayNotBeOverwritten) + { + auto fileStream = makeFileStreamMock(); + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .mayOverwrite = false, + }; + std::ofstream file{options.localPath}; + file.close(); + + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error().type, DownloadOperation::ErrorType::FileExists); + } + + TEST_F(DownloadOperationTests, DownloadPreparationFailsWhenStreamIsExpired) + { + std::weak_ptr fileWeak; + { + auto fileStream = makeFileStreamMock(); + fileWeak = fileStream; + } + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileWeak, options}; + + auto result = operation.prepare(); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error().type, DownloadOperation::ErrorType::FileStreamExpired); + } + + TEST_F(DownloadOperationTests, DownloadPreparationFailsWhenStattingTheFileFails) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + + EXPECT_CALL(*fileStream, stat()).WillOnce([]() -> std::future> { + std::promise> promise; + promise.set_value(std::unexpected(SftpError{ + .message = "Stat failed", + })); + return promise.get_future(); + }); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error().type, DownloadOperation::ErrorType::FileStatFailed); + } + + // TODO: Test continuation download + + TEST_F(DownloadOperationTests, DownloadPreparationSucceedsWithEmptyRemoteFile) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + + EXPECT_CALL(*fileStream, stat()) + .WillOnce([]() -> std::future> { + std::promise> promise; + promise.set_value(FileInformation{{ + .size = 0, + }}); + return promise.get_future(); + }); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + } + + TEST_F(DownloadOperationTests, DownloadPreparationSucceedsWithNonEmptyRemoteFile) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + + EXPECT_CALL(*fileStream, stat()) + .WillOnce([]() -> std::future> { + std::promise> promise; + promise.set_value(FileInformation{{ + .size = 42, + }}); + return promise.get_future(); + }); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + } + + TEST_F(DownloadOperationTests, DownloadPreparationCreatesPartFile) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + + EXPECT_CALL(*fileStream, stat()) + .WillOnce([]() -> std::future> { + std::promise> promise; + promise.set_value(FileInformation{{ + .size = 42, + }}); + return promise.get_future(); + }); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + } + + TEST_F(DownloadOperationTests, DestructorCleansUpIfOptionIsTrue) + { + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = true, + }; + + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + + EXPECT_CALL(*fileStream, stat()) + .WillOnce([]() -> std::future> { + std::promise> promise; + promise.set_value(FileInformation{{ + .size = 42, + }}); + return promise.get_future(); + }); + + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + } + + EXPECT_FALSE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + } + + TEST_F(DownloadOperationTests, DestructorDoesNotCleanUpIfOptionIsFalse) + { + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + + EXPECT_CALL(*fileStream, stat()) + .WillOnce([]() -> std::future> { + std::promise> promise; + promise.set_value(FileInformation{{ + .size = 42, + }}); + return promise.get_future(); + }); + + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + } + + EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + } + + TEST_F(DownloadOperationTests, CancelDoesNotRemoveTheFileIfCleanIsFalse) + { + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream); + + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + + operation.cancel(); + + EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + } + + TEST_F(DownloadOperationTests, CancelDoesRemoveTheFileIfCleanIsTrue) + { + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = true, + }; + + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + + EXPECT_CALL(*fileStream, stat()) + .WillOnce([]() -> std::future> { + std::promise> promise; + promise.set_value(FileInformation{{ + .size = 42, + }}); + return promise.get_future(); + }); + + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + + operation.cancel(); + + EXPECT_FALSE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + } + + TEST_F(DownloadOperationTests, CancelCallsTheCompletionHandler) + { + using namespace SecureShell; + + std::promise completionPromise; + + auto options = DownloadOperation::DownloadOperationOptions{ + .onCompletionCallback = + [&completionPromise](bool success) { + completionPromise.set_value(success); + }, + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = true, + }; + + const auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream); + + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + operation.cancel(); + + auto future = completionPromise.get_future(); + ASSERT_EQ(future.wait_for(1s), std::future_status::ready); + // Cancel is a failure + EXPECT_FALSE(future.get()); + } + + TEST_F(DownloadOperationTests, StartWithEmptyRemoteFileSucceeds) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, 0); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + } + + TEST_F(DownloadOperationTests, StartWithoutPrepareFails) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.start(); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error().type, DownloadOperation::ErrorType::OperationNotPrepared); + } + + TEST_F(DownloadOperationTests, StartFailsWithExpiredFileStream) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()) << boost::describe::enum_to_string(result.error().type, "INVALID_ENUM_VALUE"); + + fileStream.reset(); + + result = operation.start(); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error().type, DownloadOperation::ErrorType::FileStreamExpired); + } +} \ No newline at end of file diff --git a/frontend/include/frontend/session_components/operation_queue.hpp b/frontend/include/frontend/session_components/operation_queue.hpp new file mode 100644 index 00000000..baa791f8 --- /dev/null +++ b/frontend/include/frontend/session_components/operation_queue.hpp @@ -0,0 +1,27 @@ +#pragma once + +#include +#include +#include + +#include +#include + +class OperationQueue +{ + public: + OperationQueue( + Persistence::StateHolder* stateHolder, + FrontendEvents* events, + std::string persistenceSessionName, + std::string id, + ConfirmDialog* confirmDialog); + + ROAR_PIMPL_SPECIAL_FUNCTIONS(OperationQueue); + + Nui::ElementRenderer operator()(); + + private: + struct Implementation; + std::unique_ptr impl_; +}; \ No newline at end of file diff --git a/frontend/source/frontend/CMakeLists.txt b/frontend/source/frontend/CMakeLists.txt index 1c58adbd..d2c6a3d6 100644 --- a/frontend/source/frontend/CMakeLists.txt +++ b/frontend/source/frontend/CMakeLists.txt @@ -13,6 +13,7 @@ target_sources( dialog/password_prompter.cpp dialog/confirm_dialog.cpp session_components/session_options.cpp + session_components/operation_queue.cpp terminal/terminal.cpp terminal/executing_engine.cpp terminal/sftp_file_engine.cpp diff --git a/frontend/source/frontend/session.cpp b/frontend/source/frontend/session.cpp index c5908221..dd94bd2f 100644 --- a/frontend/source/frontend/session.cpp +++ b/frontend/source/frontend/session.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -53,7 +54,8 @@ struct Session::Implementation ConfirmDialog* confirmDialog; // Operation Queue for File Explorer - std::shared_ptr operationQueue; + OperationQueue operationQueue; + std::shared_ptr operationQueueElement; // File Explorer Things: NuiFileExplorer::FileGrid fileGrid; @@ -101,6 +103,8 @@ struct Session::Implementation , options{this->engine.terminalOptions.value()} , newItemAskDialog{newItemAskDialog} , confirmDialog{confirmDialog} + , operationQueue{this->stateHolder, this->events, this->initialName, this->id, this->confirmDialog} + , operationQueueElement{} , fileGrid{{ .pathBarOnTop = uiOptions.fileGridPathBarOnTop, }} @@ -602,16 +606,7 @@ void Session::visible(bool value) auto Session::makeOperationQueueElement() -> Nui::ElementRenderer { - using Nui::Elements::div; // because of the global div. - using namespace Nui::Attributes; - - // clang-format off - return div{ - style = "width: 100%; height: auto; display: block", - }( - "OPERATION QUEUE HERE" - ); - // clang-format on + return impl_->operationQueue(); } void Session::onChannelClosedByUser(Ids::ChannelId const& channelId) @@ -733,21 +728,21 @@ void Session::initializeLayout() return Nui::val::undefined(); }), Nui::bind([this]() -> Nui::val { - if (impl_->operationQueue) + if (impl_->operationQueueElement) { Log::warn("There is already an operation queue, cannot open another one"); return Nui::val::undefined(); } - impl_->operationQueue = Nui::Dom::makeStandaloneElement(makeOperationQueueElement()); - return impl_->operationQueue->val(); + impl_->operationQueueElement = Nui::Dom::makeStandaloneElement(makeOperationQueueElement()); + return impl_->operationQueueElement->val(); }), Nui::bind([this]() -> Nui::val { - if (!impl_->operationQueue) + if (!impl_->operationQueueElement) { Log::warn("There is no operation queue to remove"); return Nui::val::undefined(); } - impl_->operationQueue.reset(); + impl_->operationQueueElement.reset(); return Nui::val::undefined(); }), Nui::bind([this]() -> Nui::val { diff --git a/frontend/source/frontend/session_components/operation_queue.cpp b/frontend/source/frontend/session_components/operation_queue.cpp new file mode 100644 index 00000000..88943447 --- /dev/null +++ b/frontend/source/frontend/session_components/operation_queue.cpp @@ -0,0 +1,55 @@ +#include + +#include +#include +#include + +struct OperationQueue::Implementation +{ + Persistence::StateHolder* stateHolder; + FrontendEvents* events; + std::string persistenceSessionName; + std::string id; + ConfirmDialog* confirmDialog; + + Implementation( + Persistence::StateHolder* stateHolder, + FrontendEvents* events, + std::string persistenceSessionName, + std::string id, + ConfirmDialog* confirmDialog) + : stateHolder{stateHolder} + , events{events} + , persistenceSessionName{std::move(persistenceSessionName)} + , id{std::move(id)} + , confirmDialog{confirmDialog} + {} +}; + +OperationQueue::OperationQueue( + Persistence::StateHolder* stateHolder, + FrontendEvents* events, + std::string persistenceSessionName, + std::string id, + ConfirmDialog* confirmDialog) + : impl_{ + std::make_unique( + stateHolder, + events, + std::move(persistenceSessionName), + std::move(id), + confirmDialog), + } +{} +ROAR_PIMPL_SPECIAL_FUNCTIONS_IMPL(OperationQueue); + +Nui::ElementRenderer OperationQueue::operator()() +{ + using namespace Nui::Elements; + using namespace Nui::Attributes; + using Nui::Elements::div; + + return div{ + style = "width: 100%; height: auto; display: block", + }("OPERATION QUEUE HERE NEW"); +} \ No newline at end of file diff --git a/ids/include/ids/ids.hpp b/ids/include/ids/ids.hpp index 61866ed7..7b1e18ee 100644 --- a/ids/include/ids/ids.hpp +++ b/ids/include/ids/ids.hpp @@ -5,4 +5,6 @@ DEFINE_ID_TYPE(SessionId) DEFINE_ID_TYPE(SessionFrontendId) DEFINE_ID_TYPE(ChannelId) -DEFINE_ID_TYPE(UiSessionId) \ No newline at end of file +DEFINE_ID_TYPE(UiSessionId) +DEFINE_ID_TYPE(FileId) +DEFINE_ID_TYPE(OperationId) \ No newline at end of file diff --git a/log/include/log/logger_backend.hpp b/log/include/log/logger_backend.hpp index 0ef0595f..02c1fe90 100644 --- a/log/include/log/logger_backend.hpp +++ b/log/include/log/logger_backend.hpp @@ -91,7 +91,11 @@ namespace Log else { std::scoped_lock lock{guard_}; - stash_.emplace_back(level, msg); + // Do not accumulate logs in tests: + if (level != Level::Off) + { + stash_.emplace_back(level, msg); + } } spdlog::log(toSpdlogLevel(level), msg); diff --git a/shared_data/include/shared_data/directory_entry.hpp b/shared_data/include/shared_data/directory_entry.hpp index da7a76f6..63d68bac 100644 --- a/shared_data/include/shared_data/directory_entry.hpp +++ b/shared_data/include/shared_data/directory_entry.hpp @@ -9,9 +9,9 @@ namespace SharedData { struct DirectoryEntry { - std::filesystem::path path; - std::filesystem::path longName; - std::uint32_t flags; + std::filesystem::path path = ""; + std::filesystem::path longName = ""; + std::uint32_t flags = 0; /* SSH_FILEXFER_TYPE_REGULAR 1 SSH_FILEXFER_TYPE_DIRECTORY 2 @@ -23,20 +23,20 @@ namespace SharedData SSH_FILEXFER_TYPE_BLOCK_DEVICE 8 SSH_FILEXFER_TYPE_FIFO 9 */ - std::uint8_t type; - std::uint64_t size; - std::uint32_t uid; - std::uint32_t gid; - std::string owner; - std::string group; - std::filesystem::perms permissions; - std::uint64_t atime; - std::uint32_t atimeNsec; - std::uint64_t createTime; - std::uint32_t createTimeNsec; - std::uint64_t mtime; - std::uint32_t mtimeNsec; - std::string acl; + std::uint8_t type = 0; + std::uint64_t size = 0; + std::uint32_t uid = 0; + std::uint32_t gid = 0; + std::string owner = ""; + std::string group = ""; + std::filesystem::perms permissions = std::filesystem::perms::unknown; + std::uint64_t atime = 0; + std::uint32_t atimeNsec = 0; + std::uint64_t createTime = 0; + std::uint32_t createTimeNsec = 0; + std::uint64_t mtime = 0; + std::uint32_t mtimeNsec = 0; + std::string acl = ""; }; void to_json(nlohmann::json& j, DirectoryEntry const& entry); diff --git a/ssh/include/ssh/async/processing_strand.hpp b/ssh/include/ssh/async/processing_strand.hpp index bb2e38c1..7a74ff6a 100644 --- a/ssh/include/ssh/async/processing_strand.hpp +++ b/ssh/include/ssh/async/processing_strand.hpp @@ -2,10 +2,10 @@ #include -#include #include #include #include +#include namespace SecureShell { diff --git a/ssh/include/ssh/async/processing_thread.hpp b/ssh/include/ssh/async/processing_thread.hpp index 1dd3b60f..1f6ec767 100644 --- a/ssh/include/ssh/async/processing_thread.hpp +++ b/ssh/include/ssh/async/processing_thread.hpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include diff --git a/ssh/include/ssh/file_information.hpp b/ssh/include/ssh/file_information.hpp new file mode 100644 index 00000000..600bc9c6 --- /dev/null +++ b/ssh/include/ssh/file_information.hpp @@ -0,0 +1,13 @@ +#pragma once + +#include + +#include + +namespace SecureShell +{ + struct FileInformation : public SharedData::DirectoryEntry + { + static FileInformation fromSftpAttributes(sftp_attributes attributes); + }; +} \ No newline at end of file diff --git a/ssh/include/ssh/file_stream.hpp b/ssh/include/ssh/file_stream.hpp index deb89aaa..dd70aef7 100644 --- a/ssh/include/ssh/file_stream.hpp +++ b/ssh/include/ssh/file_stream.hpp @@ -1,6 +1,8 @@ #pragma once +#include #include +#include #include @@ -14,11 +16,13 @@ namespace SecureShell class Session; class SftpSession; - class FileStream : public std::enable_shared_from_this + class FileStream + : public IFileStream + , public std::enable_shared_from_this { public: FileStream(std::shared_ptr sftp, sftp_file file, sftp_limits_struct limits); - ~FileStream(); + ~FileStream() override; FileStream(FileStream const&) = delete; FileStream& operator=(FileStream const&) = delete; FileStream(FileStream&&); @@ -29,19 +33,26 @@ namespace SecureShell * * @param pos Where to move the file pointer to. */ - std::future> seek(std::size_t pos); + std::future> seek(std::size_t pos) override; /** * @brief Tells the current position in the file. * * @return std::future> The current position or an error. */ - std::future> tell(); + std::future> tell() override; + + /** + * @brief Retrieves information about the file. + * + * @return std::future> + */ + std::future> stat() override; /** * @brief Rewind the file to the beginning. */ - std::future> rewind(); + std::future> rewind() override; /** * @brief Reads some bytes from the file. Not necessarily fills the buffer. bufferSize MUST be less than or @@ -51,7 +62,7 @@ namespace SecureShell * @param bufferSize * @return std::future> */ - std::future> read(std::byte* buffer, std::size_t bufferSize); + std::future> read(std::byte* buffer, std::size_t bufferSize) override; /** * @brief Reads all bytes from the file. @@ -59,7 +70,8 @@ namespace SecureShell * @param onRead Let this function return false to stop reading. * @return std::future> The number of bytes read or an error. */ - std::future> read(std::function onRead); + std::future> + read(std::function onRead) override; /** * @brief Writes some bytes to the file. @@ -70,7 +82,7 @@ namespace SecureShell * @param bufferSize * @return std::future> */ - std::future> write(std::string_view data); + std::future> write(std::string_view data) override; /** * @brief Returns the maximum number of bytes that can be written in a single pure write operation. @@ -78,14 +90,14 @@ namespace SecureShell * * @return std::size_t */ - std::size_t writeLengthLimit() const; + std::size_t writeLengthLimit() const override; /** * @brief Returns the maximum number of bytes that can be read in a single pure read operation. * * @return std::size_t */ - std::size_t readLengthLimit() const; + std::size_t readLengthLimit() const override; /** * @brief Brings this class into an invalid state and returns the sftp_file. The ownership of the file is @@ -93,12 +105,14 @@ namespace SecureShell * * @return sftp_file */ - sftp_file release(); + sftp_file release() override; /** * @brief Closes the file and removes itself from the sftp session. */ - void close(bool isBackElement = false); + void close(bool isBackElement = false) override; + + ProcessingStrand* strand() const override; private: std::function makeFileDeleter(); diff --git a/ssh/include/ssh/file_stream_interface.hpp b/ssh/include/ssh/file_stream_interface.hpp new file mode 100644 index 00000000..4b47df28 --- /dev/null +++ b/ssh/include/ssh/file_stream_interface.hpp @@ -0,0 +1,119 @@ +#pragma once + +#include +#include +#include + +#include + +#include +#include +#include + +namespace SecureShell +{ + class Session; + class SftpSession; + + class IFileStream + { + public: + IFileStream() = default; + virtual ~IFileStream() = default; + IFileStream(IFileStream const&) = default; + IFileStream& operator=(IFileStream const&) = default; + IFileStream(IFileStream&&) = default; + IFileStream& operator=(IFileStream&&) = default; + + /** + * @brief Moves the file pointer to the specified position. + * + * @param pos Where to move the file pointer to. + */ + virtual std::future> seek(std::size_t pos) = 0; + + /** + * @brief Tells the current position in the file. + * + * @return std::future> The current position or an error. + */ + virtual std::future> tell() = 0; + + /** + * @brief Retrieves information about the file. + * + * @return std::future> + */ + virtual std::future> stat() = 0; + + /** + * @brief Rewind the file to the beginning. + */ + virtual std::future> rewind() = 0; + + /** + * @brief Reads some bytes from the file. Not necessarily fills the buffer. bufferSize MUST be less than or + * equal to the read limit. + * + * @param buffer + * @param bufferSize + * @return std::future> + */ + virtual std::future> read(std::byte* buffer, std::size_t bufferSize) = 0; + + /** + * @brief Reads all bytes from the file. + * + * @param onRead Let this function return false to stop reading. + * @return std::future> The number of bytes read or an error. + */ + virtual std::future> + read(std::function onRead) = 0; + + /** + * @brief Writes some bytes to the file. + * Makes sure that all data is written even if the data is larger than the write limit by breaking it into + * smaller parts. + * + * @param buffer + * @param bufferSize + * @return std::future> + */ + virtual std::future> write(std::string_view data) = 0; + + /** + * @brief Returns the maximum number of bytes that can be written in a single pure write operation. + * This limit is not necessary to uphold for the write function of this class. + * + * @return std::size_t + */ + virtual std::size_t writeLengthLimit() const = 0; + + /** + * @brief Returns the maximum number of bytes that can be read in a single pure read operation. + * + * @return std::size_t + */ + virtual std::size_t readLengthLimit() const = 0; + + /** + * @brief Brings this class into an invalid state and returns the sftp_file. The ownership of the file is + * transferred to the caller. + * + * @return sftp_file + */ + virtual sftp_file release() = 0; + + /** + * @brief Closes the file and removes itself from the sftp session. + */ + virtual void close(bool isBackElement = false) = 0; + + /** + * @brief Returns the processing strand of the file stream. + * + * @return void* + */ + virtual ProcessingStrand* strand() const = 0; + }; +} \ No newline at end of file diff --git a/ssh/include/ssh/mocks/file_stream_mock.hpp b/ssh/include/ssh/mocks/file_stream_mock.hpp new file mode 100644 index 00000000..1e8ba0f5 --- /dev/null +++ b/ssh/include/ssh/mocks/file_stream_mock.hpp @@ -0,0 +1,41 @@ +#pragma once + +#include + +#include + +#include +#include +#include +#include +#include +#include + +namespace SecureShell::Test +{ + class FileStreamMock : public SecureShell::IFileStream + { + public: + MOCK_METHOD((std::future>), seek, (std::size_t), (override)); + MOCK_METHOD((std::future>), tell, (), (override)); + MOCK_METHOD((std::future>), stat, (), (override)); + MOCK_METHOD((std::future>), rewind, (), (override)); + MOCK_METHOD( + (std::future>), + read, + (std::byte * buffer, std::size_t bufferSize), + (override)); + MOCK_METHOD( + (std::future>), + read, + (std::function onRead), + (override)); + MOCK_METHOD((std::future>), write, (std::string_view data), (override)); + MOCK_METHOD(std::size_t, writeLengthLimit, (), (const, override)); + MOCK_METHOD(std::size_t, readLengthLimit, (), (const, override)); + MOCK_METHOD(sftp_file, release, (), (override)); + MOCK_METHOD(void, close, (bool isBackElement), (override)); + MOCK_METHOD(ProcessingStrand*, strand, (), (const, override)); + }; + ; +} \ No newline at end of file diff --git a/ssh/include/ssh/sftp_session.hpp b/ssh/include/ssh/sftp_session.hpp index 3f3b654e..3a5a2343 100644 --- a/ssh/include/ssh/sftp_session.hpp +++ b/ssh/include/ssh/sftp_session.hpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include #include @@ -42,11 +42,6 @@ namespace SecureShell bool close(bool isBackElement = false); - struct DirectoryEntry : public SharedData::DirectoryEntry - { - static DirectoryEntry fromSftpAttributes(sftp_attributes attributes); - }; - template void perform(FunctionT&& func) { @@ -68,9 +63,10 @@ namespace SecureShell * @brief Lists the contents of a directory. * * @param path - * @return std::future, Error>> + * @return std::future, Error>> */ - std::future, Error>> listDirectory(std::filesystem::path const& path); + std::future, Error>> + listDirectory(std::filesystem::path const& path); /** * @brief Create a directory. @@ -115,16 +111,16 @@ namespace SecureShell * @brief Gets the attributes of a file or directory. * * @param path - * @return std::future> + * @return std::future> */ - std::future> stat(std::filesystem::path const& path); + std::future> stat(std::filesystem::path const& path); /** * @brief Sets the attributes of a file or directory. * * @param path * @param attributes - * @return std::future> + * @return std::future> */ std::future> stat(std::filesystem::path const& path, sftp_attributes attributes); diff --git a/ssh/source/ssh/file_information.cpp b/ssh/source/ssh/file_information.cpp new file mode 100644 index 00000000..1a3e1980 --- /dev/null +++ b/ssh/source/ssh/file_information.cpp @@ -0,0 +1,31 @@ +#include + +namespace SecureShell +{ + FileInformation FileInformation::fromSftpAttributes(sftp_attributes attributes) + { + return FileInformation{ + SharedData::DirectoryEntry{ + .path = attributes->name ? std::string{attributes->name} : std::string{}, + .longName = attributes->longname ? std::string{attributes->longname} : std::string{}, + .flags = attributes->flags, + .type = attributes->type, + .size = attributes->size, + .uid = attributes->uid, + .gid = attributes->gid, + .owner = attributes->owner ? std::string{attributes->owner} : std::string{}, + .group = attributes->group ? std::string{attributes->group} : std::string{}, + .permissions = static_cast(attributes->permissions), + .atime = attributes->atime, + .atimeNsec = attributes->atime_nseconds, + .createTime = attributes->createtime, + .createTimeNsec = attributes->createtime_nseconds, + .mtime = attributes->mtime, + .mtimeNsec = attributes->mtime_nseconds, + .acl = attributes->acl + ? std::string{ssh_string_get_char(attributes->acl), ssh_string_len(attributes->acl)} + : std::string{}, + }, + }; + } +} \ No newline at end of file diff --git a/ssh/source/ssh/file_stream.cpp b/ssh/source/ssh/file_stream.cpp index d8195d2a..3478e7eb 100644 --- a/ssh/source/ssh/file_stream.cpp +++ b/ssh/source/ssh/file_stream.cpp @@ -92,6 +92,17 @@ namespace SecureShell return {}; }); } + std::future> FileStream::stat() + { + return performPromise([this]() -> std::expected { + VERIFY_FILE_STREAM(); + std::unique_ptr attributes{ + sftp_fstat(file_.get()), sftp_attributes_free}; + if (attributes == nullptr) + return std::unexpected(lastError()); + return FileInformation::fromSftpAttributes(attributes.get()); + }); + } std::future> FileStream::tell() { return performPromise([this]() -> std::expected { @@ -141,12 +152,14 @@ namespace SecureShell std::string buffer; std::function callback; std::promise> promise; + std::size_t totalRead; ReadState(FileStream& stream, std::size_t limit, std::function callback) : stream{stream} , buffer(std::min(4096ull, limit), '\0') , callback{std::move(callback)} , promise{} + , totalRead{0} {} void onRead(std::make_signed_t amount) @@ -160,14 +173,16 @@ namespace SecureShell if (amount == 0) { - promise.set_value(static_cast(buffer.size())); + promise.set_value(totalRead); callback({}); return; } + totalRead += amount; + if (!callback({buffer.data(), static_cast(amount)})) { - promise.set_value(static_cast(buffer.size())); + promise.set_value(totalRead); return; } @@ -229,6 +244,11 @@ namespace SecureShell writePart(toWrite.substr(written), std::move(onWriteComplete)); }); } + ProcessingStrand* FileStream::strand() const + { + auto sftp = sftp_.lock(); + return sftp ? sftp->strand_.get() : nullptr; + } std::future> FileStream::write(std::string_view data) { // Short easy path: diff --git a/ssh/source/ssh/sftp_session.cpp b/ssh/source/ssh/sftp_session.cpp index 8be7b803..e822fa64 100644 --- a/ssh/source/ssh/sftp_session.cpp +++ b/ssh/source/ssh/sftp_session.cpp @@ -54,41 +54,14 @@ namespace SecureShell fileStreams_.back()->close(true); } } - SftpSession::DirectoryEntry SftpSession::DirectoryEntry::fromSftpAttributes(sftp_attributes attributes) - { - return DirectoryEntry{ - SharedData::DirectoryEntry{ - .path = attributes->name ? std::string{attributes->name} : std::string{}, - .longName = attributes->longname ? std::string{attributes->longname} : std::string{}, - .flags = attributes->flags, - .type = attributes->type, - .size = attributes->size, - .uid = attributes->uid, - .gid = attributes->gid, - .owner = attributes->owner ? std::string{attributes->owner} : std::string{}, - .group = attributes->group ? std::string{attributes->group} : std::string{}, - .permissions = static_cast(attributes->permissions), - .atime = attributes->atime, - .atimeNsec = attributes->atime_nseconds, - .createTime = attributes->createtime, - .createTimeNsec = attributes->createtime_nseconds, - .mtime = attributes->mtime, - .mtimeNsec = attributes->mtime_nseconds, - .acl = attributes->acl - ? std::string{ssh_string_get_char(attributes->acl), ssh_string_len(attributes->acl)} - : std::string{}, - }, - }; - } - std::future, SftpSession::Error>> + std::future, SftpSession::Error>> SftpSession::listDirectory(std::filesystem::path const& path) { return performPromise( - [this, - path = std::move(path)]() -> std::expected, SftpSession::Error> { + [this, path = std::move(path)]() -> std::expected, SftpSession::Error> { int closeResult = 0; - std::vector entries{}; + std::vector entries{}; { std::unique_ptr> dir{ @@ -113,7 +86,7 @@ namespace SecureShell for (; entry != nullptr; entry.reset(sftp_readdir(session_, dir.get()))) { - entries.push_back(DirectoryEntry::fromSftpAttributes(entry.get())); + entries.push_back(FileInformation::fromSftpAttributes(entry.get())); } } @@ -196,16 +169,15 @@ namespace SecureShell }); } - std::future> - SftpSession::stat(std::filesystem::path const& path) + std::future> SftpSession::stat(std::filesystem::path const& path) { - return performPromise([this, path = std::move(path)]() -> std::expected { + return performPromise([this, path = std::move(path)]() -> std::expected { std::unique_ptr attributes{ sftp_stat(session_, path.generic_string().c_str()), sftp_attributes_free}; if (attributes == nullptr) return std::unexpected(lastError()); - return DirectoryEntry::fromSftpAttributes(attributes.get()); + return FileInformation::fromSftpAttributes(attributes.get()); }); } diff --git a/ssh/test/ssh/CMakeLists.txt b/ssh/test/ssh/CMakeLists.txt index 492606cf..1e0e820e 100644 --- a/ssh/test/ssh/CMakeLists.txt +++ b/ssh/test/ssh/CMakeLists.txt @@ -11,6 +11,7 @@ find_package(Boost CONFIG REQUIRED COMPONENTS filesystem asio system process) target_link_libraries( nui-scp-ssh-test PUBLIC + utility secure-shell GTest::GTest GTest::Main diff --git a/ssh/test/ssh/common_fixture.hpp b/ssh/test/ssh/common_fixture.hpp index 643cb629..560d663f 100644 --- a/ssh/test/ssh/common_fixture.hpp +++ b/ssh/test/ssh/common_fixture.hpp @@ -154,7 +154,7 @@ namespace SecureShell::Test }); } - TemporaryDirectory isolateDirectory_{programDirectory / "temp", false}; + Utility::TemporaryDirectory isolateDirectory_{programDirectory / "temp", false}; boost::asio::thread_pool pool_{1}; nlohmann::json defaultPackageJson_ = nlohmann::json({ {"name", "test"}, diff --git a/ssh/test/ssh/test_processing_thread.hpp b/ssh/test/ssh/test_processing_thread.hpp index 99faefcc..641af68c 100644 --- a/ssh/test/ssh/test_processing_thread.hpp +++ b/ssh/test/ssh/test_processing_thread.hpp @@ -1,6 +1,6 @@ #pragma once -#include "utility/awaiter.hpp" +#include #include #include diff --git a/ssh/test/ssh/test_sftp.hpp b/ssh/test/ssh/test_sftp.hpp index 1c2f26ab..fe756128 100644 --- a/ssh/test/ssh/test_sftp.hpp +++ b/ssh/test/ssh/test_sftp.hpp @@ -21,9 +21,12 @@ namespace SecureShell::Test { if (std::filesystem::exists(programDirectory / "temp" / "log.txt")) { + if (!std::filesystem::exists(programDirectory / "logs")) + std::filesystem::create_directory(programDirectory / "logs"); + std::filesystem::rename( programDirectory / "temp" / "log.txt", - programDirectory / "temp" / + programDirectory / "logs" / ("log_"s + ::testing::UnitTest::GetInstance()->current_test_info()->test_case_name() + "_"s + ::testing::UnitTest::GetInstance()->current_test_info()->name() + ".txt")); } @@ -466,4 +469,27 @@ namespace SecureShell::Test ASSERT_TRUE(sftp->close()); EXPECT_FALSE(sftp->close()); } + + TEST_F(SftpTests, CanStatFileUsingFileStream) + { + CREATE_SERVER_AND_JOINER(Sftp); + auto [_, sftp] = createSftpSession(serverStartResult->port); + + auto fut = + sftp->openFile("/home/test/file1.txt", SftpSession::OpenType::Read, std::filesystem::perms::owner_read); + ASSERT_EQ(fut.wait_for(1s), std::future_status::ready); + auto result = fut.get(); + ASSERT_TRUE(result.has_value()); + + auto fileWeak = std::move(result).value(); + auto file = fileWeak.lock(); + ASSERT_TRUE(file); + + auto statFut = file->stat(); + ASSERT_EQ(statFut.wait_for(1s), std::future_status::ready); + auto statResult = statFut.get(); + ASSERT_TRUE(statResult.has_value()); + + EXPECT_GT(statResult.value().size, 0); + } } diff --git a/ssh/test/ssh/utility/node.hpp b/ssh/test/ssh/utility/node.hpp index 8e3e670f..c94bac29 100644 --- a/ssh/test/ssh/utility/node.hpp +++ b/ssh/test/ssh/utility/node.hpp @@ -1,6 +1,6 @@ #pragma once -#include "./temporary_directory.hpp" +#include #include diff --git a/ssh/test/ssh/utility/temporary_directory.hpp b/ssh/test/ssh/utility/temporary_directory.hpp deleted file mode 100644 index 477b3d6b..00000000 --- a/ssh/test/ssh/utility/temporary_directory.hpp +++ /dev/null @@ -1,80 +0,0 @@ -#pragma once - -#include - -#include -#include -#include -#include - -#include - -namespace SecureShell::Test -{ - class TemporaryDirectory - { - public: - TemporaryDirectory(std::filesystem::path path, bool randomSubFolder = true) - : path_{std::move(path)} - { - using namespace std::string_literals; - - std::error_code error; - bool valid = true; - if (!std::filesystem::exists(path_, error)) - { - valid = std::filesystem::create_directories(path_, error) && !error; - } - else if (!std::filesystem::is_directory(path_)) - { - throw std::runtime_error(std::string{"Given path exists and is not a directory: "} + path_.string()); - } - if (valid && randomSubFolder) - { - auto dir = path_ / ("temp_"s + boost::uuids::to_string(gen_())); - std::filesystem::create_directory(dir); - valid = std::filesystem::is_directory(dir); - path_ = dir; - } - if (!valid) - { - throw std::runtime_error(std::string{"Could not setup temporary directory: "} + path_.string()); - } - } - ~TemporaryDirectory() - { - std::error_code error; - - // if (!path_.empty()) - // std::filesystem::remove_all(path_, error); - } - - TemporaryDirectory(TemporaryDirectory const&) = delete; - - TemporaryDirectory(TemporaryDirectory&& t) - : path_{std::move(t.path_)} - {} - - TemporaryDirectory& operator=(TemporaryDirectory const&) = delete; - - TemporaryDirectory& operator=(TemporaryDirectory&& t) - { - path_ = std::exchange(t.path_, {}); - return *this; - } - - std::filesystem::path path() const noexcept - { - return path_; - } - - operator std::filesystem::path() const noexcept - { - return path_; - } - - private: - std::filesystem::path path_; - boost::uuids::random_generator gen_{}; - }; -} \ No newline at end of file diff --git a/ssh/test/ssh/utility/awaiter.hpp b/utility/include/utility/awaiter.hpp similarity index 100% rename from ssh/test/ssh/utility/awaiter.hpp rename to utility/include/utility/awaiter.hpp diff --git a/utility/include/utility/temporary_directory.hpp b/utility/include/utility/temporary_directory.hpp index a59dfeba..95db6021 100644 --- a/utility/include/utility/temporary_directory.hpp +++ b/utility/include/utility/temporary_directory.hpp @@ -2,24 +2,79 @@ #include +#include +#include +#include +#include + +#include + namespace Utility { class TemporaryDirectory { public: - TemporaryDirectory(); - ~TemporaryDirectory(); + TemporaryDirectory(std::filesystem::path path, bool randomSubFolder = true) + : path_{std::move(path)} + { + using namespace std::string_literals; + + std::error_code error; + bool valid = true; + if (!std::filesystem::exists(path_, error)) + { + valid = std::filesystem::create_directories(path_, error) && !error; + } + else if (!std::filesystem::is_directory(path_)) + { + throw std::runtime_error(std::string{"Given path exists and is not a directory: "} + path_.string()); + } + if (valid && randomSubFolder) + { + auto dir = path_ / ("temp_"s + boost::uuids::to_string(gen_())); + std::filesystem::create_directory(dir); + valid = std::filesystem::is_directory(dir); + path_ = dir; + } + if (!valid) + { + throw std::runtime_error(std::string{"Could not setup temporary directory: "} + path_.string()); + } + } + ~TemporaryDirectory() + { + std::error_code error; + + if (!path_.empty()) + std::filesystem::remove_all(path_, error); + } TemporaryDirectory(TemporaryDirectory const&) = delete; + + TemporaryDirectory(TemporaryDirectory&& t) + : path_{std::move(t.path_)} + {} + TemporaryDirectory& operator=(TemporaryDirectory const&) = delete; - TemporaryDirectory(TemporaryDirectory&&) = delete; - TemporaryDirectory& operator=(TemporaryDirectory&&) = delete; + TemporaryDirectory& operator=(TemporaryDirectory&& t) + { + path_ = std::exchange(t.path_, {}); + return *this; + } + + std::filesystem::path path() const noexcept + { + return path_; + } - std::filesystem::path const& path() const; + operator std::filesystem::path() const noexcept + { + return path_; + } private: - std::filesystem::path m_basePath; - std::filesystem::path m_path; + std::filesystem::path path_; + boost::uuids::random_generator gen_{}; }; } \ No newline at end of file diff --git a/utility/source/utility/temporary_directory.cpp b/utility/source/utility/temporary_directory.cpp index d0a5a33f..d3e50b4a 100644 --- a/utility/source/utility/temporary_directory.cpp +++ b/utility/source/utility/temporary_directory.cpp @@ -1,74 +1 @@ -#include - -#include -#include -#include - -using namespace std::string_literals; - -namespace Utility -{ - namespace - { - [[maybe_unused]] std::string generateRandomString(int length) - { - std::string characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; - std::string randomString; - - std::mt19937 rng(std::time(nullptr)); // Seed the random generator with time - std::uniform_int_distribution distribution(0, characters.length() - 1); - - for (int i = 0; i < length; ++i) - { - randomString += characters[distribution(rng)]; - } - - return randomString; - } - } - - TemporaryDirectory::TemporaryDirectory() - : m_basePath{[]() { - const auto temporaryDirectory = std::filesystem::temp_directory_path(); - return temporaryDirectory / "sscv2_client_tmpdir"; - }()} - , m_path{} - { - if (!std::filesystem::exists(m_basePath)) - std::filesystem::create_directories(m_basePath); - -#if __linux__ - std::string dirNameAsString{(m_basePath / "dirXXXXXX").string()}; - bool valid = mkdtemp(&dirNameAsString[0]) && std::filesystem::is_directory(dirNameAsString); - if (valid) - m_path = dirNameAsString; -#else - int i = 0; - for (; i != 1000; ++i) - { - const auto directoryName = "dir"s + generateRandomString(10); - const auto path = m_basePath / directoryName; - if (!std::filesystem::exists(path)) - { - std::filesystem::create_directory(path); - m_path = path; - break; - } - } - bool valid = i != 1000; -#endif - if (!valid) - throw std::runtime_error(std::string{"Could not setup temporary directory: "} + m_path.string()); - } - TemporaryDirectory::~TemporaryDirectory() - { - std::error_code error; - std::filesystem::remove_all(m_path, error); - std::filesystem::remove(m_basePath, error); - } - - std::filesystem::path const& TemporaryDirectory::path() const - { - return m_path; - } -} \ No newline at end of file +// TODO: Remove \ No newline at end of file From 98143ab4cc2845af678868d588216b4954ffc715 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Sun, 9 Mar 2025 18:49:22 +0100 Subject: [PATCH 10/42] Simplified operation task flush. --- backend/source/backend/sftp/download_operation.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/backend/source/backend/sftp/download_operation.cpp b/backend/source/backend/sftp/download_operation.cpp index 94e03976..a6b28d3a 100644 --- a/backend/source/backend/sftp/download_operation.cpp +++ b/backend/source/backend/sftp/download_operation.cpp @@ -47,11 +47,7 @@ DownloadOperation::~DownloadOperation() if (auto stream = fileStream_.lock(); stream) { // wait for all tasks of the operation to finish - stream->strand() - ->pushPromiseTask([]() { - return true; - }) - .get(); + stream->strand()->pushPromiseTask([]() {}).get(); } } From 0cfca0f045b543dcf7d9c7b089c1a36024c294cb Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Sun, 9 Mar 2025 23:25:30 +0100 Subject: [PATCH 11/42] Added tests for download operation. --- .../backend/sftp/download_operation.hpp | 3 +- backend/include/backend/sftp/operation.hpp | 5 +- .../backend/sftp/download_operation.cpp | 75 ++- .../test/backend/test_download_operation.hpp | 466 +++++++++++++++++- 4 files changed, 523 insertions(+), 26 deletions(-) diff --git a/backend/include/backend/sftp/download_operation.hpp b/backend/include/backend/sftp/download_operation.hpp index 46e2aa21..0e58d039 100644 --- a/backend/include/backend/sftp/download_operation.hpp +++ b/backend/include/backend/sftp/download_operation.hpp @@ -37,7 +37,7 @@ class DownloadOperation : public Operation std::expected prepare() override; std::expected start() override; - void cancel() override; + std::expected cancel() override; std::expected pause() override; std::expected finalize() override; @@ -74,6 +74,7 @@ class DownloadOperation : public Operation bool inheritPermissions_; bool doCleanup_; bool preparationDone_; + bool isReading_; std::atomic_bool interuptRead_; std::optional permissions_; std::ofstream localFile_; diff --git a/backend/include/backend/sftp/operation.hpp b/backend/include/backend/sftp/operation.hpp index 6515067f..b1eb2d37 100644 --- a/backend/include/backend/sftp/operation.hpp +++ b/backend/include/backend/sftp/operation.hpp @@ -20,7 +20,8 @@ BOOST_DEFINE_ENUM_CLASS( RenameFailure, CannotSetFilePermissions, FutureTimeout, - OperationNotPrepared); + OperationNotPrepared, + CannotFinalizeDuringRead); class Operation { @@ -60,7 +61,7 @@ class Operation /** * @brief Cancels the operation and possibly does some cleanup. */ - virtual void cancel() = 0; + virtual std::expected cancel() = 0; /** * @brief Pauses the operation. diff --git a/backend/source/backend/sftp/download_operation.cpp b/backend/source/backend/sftp/download_operation.cpp index a6b28d3a..5149776e 100644 --- a/backend/source/backend/sftp/download_operation.cpp +++ b/backend/source/backend/sftp/download_operation.cpp @@ -18,6 +18,9 @@ DownloadOperation::DownloadOperation( if (once) return; once = true; + + std::scoped_lock lock{mutex_}; + isReading_ = false; progressCallback_(0, fileSize_, fileSize_); cb(success); }} @@ -27,6 +30,7 @@ DownloadOperation::DownloadOperation( , inheritPermissions_{options.inheritPermissions} , doCleanup_{options.doCleanup} , preparationDone_{false} + , isReading_{false} , interuptRead_{false} , permissions_{options.permissions} , localFile_{} @@ -199,29 +203,40 @@ std::expected DownloadOperation::start() return std::unexpected(Error{.type = ErrorType::FileStreamExpired}); } + isReading_ = true; readFuture_ = stream->read([this](std::string_view data) { if (data.size() == 0) { - Log::info("DownloadOperation: Remote file read complete or error."); { - std::scoped_lock lock{mutex_}; - localFile_.close(); + std::lock_guard lock{mutex_}; + isReading_ = false; } + Log::info("DownloadOperation: Remote file read complete or error."); // Defer completion by pushing it onto the strand. perform([this]() { std::scoped_lock lock{mutex_}; onCompletionCallback_(true); }); - return false; + return true; } - std::scoped_lock lock{mutex_}; - localFile_.write(data.data(), data.size()); - progressCallback_(0, fileSize_, localFile_.tellp()); - const auto doContinue = localFile_.good() || interuptRead_; + std::uint64_t fileSize = 0; + std::uint64_t tellp = 0; + bool doContinue = true; + bool good = true; + { + std::scoped_lock lock{mutex_}; + localFile_.write(data.data(), data.size()); + fileSize = fileSize_; + tellp = static_cast(localFile_.tellp()); + good = localFile_.good(); + doContinue = good && !interuptRead_; + isReading_ = doContinue; + } + progressCallback_(0, fileSize, tellp); if (!doContinue) { - if (!localFile_.good()) + if (!good) { Log::error("DownloadOperation read cycle stopped: localFile_.good() == false"); perform([this]() { @@ -235,14 +250,26 @@ std::expected DownloadOperation::start() return {}; } -void DownloadOperation::cancel() +std::expected DownloadOperation::cancel() { - std::scoped_lock lock{mutex_}; + { + std::scoped_lock lock{mutex_}; - Log::info( - "DownloadOperation: Download of '{}' to '{}' canceled.", - remotePath_.generic_string(), - localPath_.generic_string()); + if (isReading_) + { + const auto res = pause(); + if (!res.has_value()) + { + Log::error("DownloadOperation: Failed to pause download."); + return res; + } + } + + Log::info( + "DownloadOperation: Download of '{}' to '{}' canceled.", + remotePath_.generic_string(), + localPath_.generic_string()); + } perform([this]() { std::scoped_lock lock{mutex_}; @@ -250,6 +277,8 @@ void DownloadOperation::cancel() }); cleanup(); + + return {}; } void DownloadOperation::cleanup() { @@ -264,20 +293,32 @@ void DownloadOperation::cleanup() } std::expected DownloadOperation::pause() { - std::scoped_lock lock{mutex_}; - interuptRead_ = true; + { + std::scoped_lock lock{mutex_}; + interuptRead_ = true; + } if (readFuture_.wait_for(futureTimeout_) != std::future_status::ready) { Log::error("DownloadOperation: Failed to pause download."); return std::unexpected(Error{.type = ErrorType::FutureTimeout}); } + { + std::scoped_lock lock{mutex_}; + isReading_ = false; + } return {}; } std::expected DownloadOperation::finalize() { std::scoped_lock lock{mutex_}; + if (isReading_) + { + Log::error("DownloadOperation: Cannot finalize while reading."); + return std::unexpected(Error{.type = ErrorType::CannotFinalizeDuringRead}); + } + if (readFuture_.wait_for(futureTimeout_) != std::future_status::ready) { Log::error("DownloadOperation: Failed to wait for download completion."); diff --git a/backend/test/backend/test_download_operation.hpp b/backend/test/backend/test_download_operation.hpp index 66fc1523..aac4cacd 100644 --- a/backend/test/backend/test_download_operation.hpp +++ b/backend/test/backend/test_download_operation.hpp @@ -8,6 +8,12 @@ #include +#include +#include +#include +#include +#include + using namespace std::chrono_literals; using namespace std::string_literals; @@ -20,6 +26,9 @@ namespace Test protected: void SetUp() override { + for (int i = 0; i != 10; ++i) + fakeFileContent_ += "This is a test file content.\n"; + processingThread_.start(5ms); } @@ -84,14 +93,19 @@ namespace Test if (chunkSize == 0) { readPromise_.set_value(readOffset_); - onRead_({}); + onReadResult_ = onRead_({}); return; } if (onRead_) { - onRead_(std::string_view{fakeFileContent_}.substr(readOffset_, chunkSize)); + onReadResult_ = onRead_(std::string_view{fakeFileContent_}.substr(readOffset_, chunkSize)); readOffset_ += chunkSize; + if (!onReadResult_) + { + readPromise_.set_value(readOffset_); + return; + } } else { @@ -99,14 +113,21 @@ namespace Test } } + std::string readFile(std::filesystem::path const& path) + { + std::ifstream file{path}; + return std::string{std::istreambuf_iterator{file}, std::istreambuf_iterator{}}; + } + protected: - std::string fakeFileContent_{"Hello, World!"}; + std::string fakeFileContent_{}; Utility::TemporaryDirectory isolateDirectory_{programDirectory / "temp", true}; SecureShell::ProcessingThread processingThread_{}; std::unique_ptr strand_{processingThread_.createStrand()}; std::promise> readPromise_{}; std::function onRead_{}; std::size_t readOffset_{0}; + bool onReadResult_{true}; }; TEST_F(DownloadOperationTests, CanCreateDownloadOperation) @@ -340,7 +361,7 @@ namespace Test EXPECT_TRUE(result.has_value()); EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); - operation.cancel(); + EXPECT_TRUE(operation.cancel().has_value()); EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); } @@ -371,7 +392,7 @@ namespace Test EXPECT_TRUE(result.has_value()); EXPECT_TRUE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); - operation.cancel(); + EXPECT_TRUE(operation.cancel().has_value()); EXPECT_FALSE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); } @@ -398,7 +419,7 @@ namespace Test auto result = operation.prepare(); EXPECT_TRUE(result.has_value()); - operation.cancel(); + EXPECT_TRUE(operation.cancel().has_value()); auto future = completionPromise.get_future(); ASSERT_EQ(future.wait_for(1s), std::future_status::ready); @@ -461,4 +482,437 @@ namespace Test EXPECT_FALSE(result.has_value()); EXPECT_EQ(result.error().type, DownloadOperation::ErrorType::FileStreamExpired); } + + TEST_F(DownloadOperationTests, StartCallsReadOnFileStream) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, 42); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + } + + TEST_F(DownloadOperationTests, PrepareReservesSpaceForFile) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .reserveSpace = true, + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + EXPECT_TRUE(operation.cancel().has_value()); + + EXPECT_EQ( + std::filesystem::file_size(options.localPath.generic_string() + ".filepart"), fakeFileContent_.size()); + } + + TEST_F(DownloadOperationTests, ReadCycleWritesDataToFile) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + fakeReadCycle(5); + fakeReadCycle(0); // EOF Cycle + + EXPECT_TRUE(operation.cancel().has_value()); + + EXPECT_EQ(std::filesystem::file_size(options.localPath.generic_string() + ".filepart"), 5); + EXPECT_EQ(readFile(options.localPath.generic_string() + ".filepart"), fakeFileContent_.substr(0, 5)); + } + + TEST_F(DownloadOperationTests, ReadWritesFileThroughMultipleCycles) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + for (std::size_t i = 0; i < fakeFileContent_.size(); ++i) + fakeReadCycle(1); + fakeReadCycle(0); // EOF Cycle + + EXPECT_TRUE(operation.cancel().has_value()); + + EXPECT_EQ( + std::filesystem::file_size(options.localPath.generic_string() + ".filepart"), fakeFileContent_.size()); + EXPECT_EQ(readFile(options.localPath.generic_string() + ".filepart"), fakeFileContent_); + } + + TEST_F(DownloadOperationTests, ReadWritesFileThroughSingleCycle) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + fakeReadCycle(fakeFileContent_.size()); + fakeReadCycle(0); // EOF Cycle + + EXPECT_TRUE(operation.cancel().has_value()); + + EXPECT_EQ( + std::filesystem::file_size(options.localPath.generic_string() + ".filepart"), fakeFileContent_.size()); + EXPECT_EQ(readFile(options.localPath.generic_string() + ".filepart"), fakeFileContent_); + } + + TEST_F(DownloadOperationTests, CanPauseRead) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + fakeReadCycle(5); + + std::expected pauseResult; + std::promise threadStarted; + std::thread asyncPause{[&operation, &pauseResult, &threadStarted]() { + threadStarted.set_value(); + pauseResult = operation.pause(); + }}; + threadStarted.get_future().wait(); + // dont know actually how to wait for the pause, except for burdening the wait on the user. + std::this_thread::sleep_for(200ms); + + // This one still goes through but now signals a stop to the read. + fakeReadCycle(5); + + asyncPause.join(); + + EXPECT_TRUE(operation.cancel().has_value()); + + EXPECT_EQ(onReadResult_, false); + EXPECT_EQ(std::filesystem::file_size(options.localPath.generic_string() + ".filepart"), 10); + EXPECT_EQ(readFile(options.localPath.generic_string() + ".filepart"), fakeFileContent_.substr(0, 10)); + } + + TEST_F(DownloadOperationTests, CanContinueReadingAfterPause) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + fakeReadCycle(5); + + std::expected pauseResult; + std::promise threadStarted; + std::thread asyncPause{[&operation, &pauseResult, &threadStarted]() { + threadStarted.set_value(); + pauseResult = operation.pause(); + }}; + threadStarted.get_future().wait(); + // dont know actually how to wait for the pause, except for burdening the wait on the user. + std::this_thread::sleep_for(200ms); + + // This one still goes through but now signals a stop to the read. + fakeReadCycle(5); + + asyncPause.join(); + + EXPECT_EQ(onReadResult_, false); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + // This one should go through and continue the read. + fakeReadCycle(fakeFileContent_.size() - 10); + fakeReadCycle(0); // EOF Cycle + + result = operation.cancel(); + EXPECT_TRUE(result.has_value()); + + EXPECT_EQ(onReadResult_, true); + EXPECT_EQ( + std::filesystem::file_size(options.localPath.generic_string() + ".filepart"), fakeFileContent_.size()); + EXPECT_EQ(readFile(options.localPath.generic_string() + ".filepart"), fakeFileContent_); + } + + TEST_F(DownloadOperationTests, CannotFinalizeWhileReading) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + fakeReadCycle(5); + + auto fin = operation.finalize(); + EXPECT_FALSE(fin.has_value()); + EXPECT_EQ(fin.error().type, DownloadOperation::ErrorType::CannotFinalizeDuringRead); + } + + TEST_F(DownloadOperationTests, FinalizeFailsIfFileExistsButOverwriteIsForbidden) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .mayOverwrite = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + fakeReadCycle(fakeFileContent_.size()); + fakeReadCycle(0); // EOF Cycle + + { + std::ofstream file{options.localPath}; + } + + auto fin = operation.finalize(); + EXPECT_FALSE(fin.has_value()); + EXPECT_EQ(fin.error().type, DownloadOperation::ErrorType::FileExists); + } + + TEST_F(DownloadOperationTests, CanFinalizeOperation) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .mayOverwrite = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + fakeReadCycle(fakeFileContent_.size()); + fakeReadCycle(0); // EOF Cycle + + auto fin = operation.finalize(); + EXPECT_TRUE(fin.has_value()); + + EXPECT_EQ(readFile(options.localPath), fakeFileContent_); + EXPECT_FALSE(std::filesystem::exists(options.localPath.generic_string() + ".filepart")); + } + + TEST_F(DownloadOperationTests, FinalizeSucceedsIfFileExistsButOverwriteIsAllowed) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .localPath = isolateDirectory_.path() / "file.txt", + .mayOverwrite = true, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + fakeReadCycle(fakeFileContent_.size()); + fakeReadCycle(0); // EOF Cycle + + { + std::ofstream file{options.localPath}; + } + + auto fin = operation.finalize(); + EXPECT_TRUE(fin.has_value()); + } + + TEST_F(DownloadOperationTests, ProgressCallbackIsCalledDuringRead) + { + using namespace SecureShell; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + std::vector> progressCalls; + + auto options = DownloadOperation::DownloadOperationOptions{ + .progressCallback = + [&progressCalls](std::int64_t min, std::int64_t max, std::int64_t current) { + progressCalls.emplace_back(min, max, current); + }, + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + for (std::size_t i = 0; i < fakeFileContent_.size(); ++i) + fakeReadCycle(1); + fakeReadCycle(0); // EOF Cycle + + EXPECT_TRUE(operation.cancel().has_value()); + + EXPECT_EQ(progressCalls.size(), fakeFileContent_.size()); + for (std::size_t i = 0; i < fakeFileContent_.size(); ++i) + { + EXPECT_EQ(std::get<0>(progressCalls[i]), 0); + EXPECT_EQ(std::get<1>(progressCalls[i]), fakeFileContent_.size()); + EXPECT_EQ(std::get<2>(progressCalls[i]), i + 1); + } + + EXPECT_EQ(std::get<0>(progressCalls.back()), 0); + EXPECT_EQ(std::get<1>(progressCalls.back()), fakeFileContent_.size()); + EXPECT_EQ(std::get<2>(progressCalls.back()), fakeFileContent_.size()); + } + + TEST_F(DownloadOperationTests, CompletionCallbackIsCalled) + { + using namespace SecureShell; + + std::promise completionPromise; + + auto fileStream = makeFileStreamMock(); + giveMockDefaultStat(fileStream, fakeFileContent_.size()); + giveMockExpectedRead(fileStream); + + auto options = DownloadOperation::DownloadOperationOptions{ + .onCompletionCallback = + [&completionPromise](bool success) { + completionPromise.set_value(success); + }, + .localPath = isolateDirectory_.path() / "file.txt", + .doCleanup = false, + }; + DownloadOperation operation{fileStream, options}; + + auto result = operation.prepare(); + EXPECT_TRUE(result.has_value()); + + result = operation.start(); + EXPECT_TRUE(result.has_value()); + + for (std::size_t i = 0; i < fakeFileContent_.size(); ++i) + fakeReadCycle(1); + fakeReadCycle(0); // EOF Cycle + + auto future = completionPromise.get_future(); + ASSERT_EQ(future.wait_for(1s), std::future_status::ready); + EXPECT_TRUE(future.get()); + } } \ No newline at end of file From cc301cef59b91d5731a2f39e870ed7a0fec4bbce Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Mon, 10 Mar 2025 02:39:13 +0100 Subject: [PATCH 12/42] Added download operation add to queue. --- .gitignore | 3 +- backend/include/backend/sftp/operation.hpp | 11 ++- .../include/backend/sftp/operation_queue.hpp | 32 ++++++-- .../include/backend/ssh_session_manager.hpp | 20 ++--- backend/source/backend/CMakeLists.txt | 3 +- backend/source/backend/main.cpp | 2 +- .../source/backend/sftp/operation_queue.cpp | 68 ++++++++++++++++ .../source/backend/ssh_session_manager.cpp | 72 +++++++++++++++-- .../persistence/state/sftp_options.hpp | 35 +++++++++ .../persistence/state/ssh_session_options.hpp | 2 + .../include/persistence/state/state.hpp | 1 + persistence/source/persistence/CMakeLists.txt | 1 + .../source/persistence/state/sftp_options.cpp | 78 +++++++++++++++++++ .../persistence/state/ssh_session_options.cpp | 4 + .../source/persistence/state/state.cpp | 4 + ssh/source/ssh/CMakeLists.txt | 1 + ssh/test/ssh/utility/node.cpp | 2 +- ssh/test/ssh/utility/node.hpp | 2 +- 18 files changed, 308 insertions(+), 33 deletions(-) create mode 100644 persistence/include/persistence/state/sftp_options.hpp create mode 100644 persistence/source/persistence/state/sftp_options.cpp diff --git a/.gitignore b/.gitignore index e3d5bf0d..82c65bbe 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,5 @@ dependencies/* node_modules/* themes/node_modules/* themes/node_modules -todo.txt \ No newline at end of file +todo.txt +.clangd \ No newline at end of file diff --git a/backend/include/backend/sftp/operation.hpp b/backend/include/backend/sftp/operation.hpp index b1eb2d37..353afeb4 100644 --- a/backend/include/backend/sftp/operation.hpp +++ b/backend/include/backend/sftp/operation.hpp @@ -21,7 +21,8 @@ BOOST_DEFINE_ENUM_CLASS( CannotSetFilePermissions, FutureTimeout, OperationNotPrepared, - CannotFinalizeDuringRead); + CannotFinalizeDuringRead, + InvalidOptionsKey); class Operation { @@ -40,6 +41,14 @@ class Operation { ErrorType type; std::optional sftpError = std::nullopt; + + std::string toString() const + { + const auto enumString = boost::describe::enum_to_string(type, "INVALID_ENUM_VALUE"); + if (sftpError.has_value()) + return fmt::format("{}: {}", enumString, sftpError->toString()); + return enumString; + } }; Ids::OperationId id() const diff --git a/backend/include/backend/sftp/operation_queue.hpp b/backend/include/backend/sftp/operation_queue.hpp index 72aec0c2..85aee22e 100644 --- a/backend/include/backend/sftp/operation_queue.hpp +++ b/backend/include/backend/sftp/operation_queue.hpp @@ -1,16 +1,36 @@ #pragma once +#include +#include +#include +#include #include -#include -class FileOperationQueue +#include +#include +#include +#include + +class OperationQueue { public: - FileOperationQueue(); + OperationQueue(); + + using Error = OperationErrorType; + void start(); void pause(); - void resume(); - void cancelAll(bool clean); + void cancelAll(); + void cancel(Ids::OperationId id); + + std::expected addDownloadOperation( + Persistence::State const& state, + std::string const& sshSessionOptionsKey, + SecureShell::SftpSession& sftp, + Ids::OperationId id, + std::filesystem::path const& localPath, + std::filesystem::path const& remotePath); private: -}; \ No newline at end of file + std::vector>> operations_; +}; diff --git a/backend/include/backend/ssh_session_manager.hpp b/backend/include/backend/ssh_session_manager.hpp index f23d51ea..7db152b1 100644 --- a/backend/include/backend/ssh_session_manager.hpp +++ b/backend/include/backend/ssh_session_manager.hpp @@ -1,9 +1,9 @@ #pragma once #include +#include #include -// #include -// #include +#include #include #include #include @@ -23,7 +23,7 @@ class SshSessionManager public: constexpr static auto futureTimeout = std::chrono::seconds{10}; - SshSessionManager(); + SshSessionManager(Persistence::StateHolder* stateHolder); ~SshSessionManager(); SshSessionManager(SshSessionManager const&) = delete; SshSessionManager& operator=(SshSessionManager const&) = delete; @@ -55,22 +55,16 @@ class SshSessionManager void registerRpcSftpCreateFile(Nui::Window&, Nui::RpcHub& hub); // Sftp Files: + void registerRpcSftpAddDownloadOperation(Nui::Window&, Nui::RpcHub& hub); private: std::mutex passwordProvidersMutex_{}; std::mutex addSessionMutex_{}; + Persistence::StateHolder* stateHolder_{}; std::unordered_map, Ids::IdHash> sessions_{}; std::unordered_map, Ids::IdHash> channels_{}; - struct SftpSession - { - std::weak_ptr sftp{}; - std::unordered_map fileStreams{}; - - SftpSession(std::weak_ptr sftp) - : sftp{std::move(sftp)} - {} - }; - std::unordered_map sftpChannels_{}; + std::unordered_map, Ids::IdHash> sftpChannels_{}; + OperationQueue operationQueue_{}; std::map passwordProviders_{}; std::unique_ptr addSessionThread_{}; std::vector pwCache_{}; diff --git a/backend/source/backend/CMakeLists.txt b/backend/source/backend/CMakeLists.txt index 8cf06463..e0dad62a 100644 --- a/backend/source/backend/CMakeLists.txt +++ b/backend/source/backend/CMakeLists.txt @@ -42,13 +42,14 @@ include("${CMAKE_CURRENT_LIST_DIR}/../../../_cmake/dependencies/libssh.cmake") # Link backend of nui outside of emscripten target_link_libraries( backend - PRIVATE + PUBLIC nui-backend efsw-static Boost::filesystem Boost::asio Boost::system Boost::process + roar-include-only shared_data utility log diff --git a/backend/source/backend/main.cpp b/backend/source/backend/main.cpp index 152e87dc..c353d9c5 100644 --- a/backend/source/backend/main.cpp +++ b/backend/source/backend/main.cpp @@ -158,7 +158,7 @@ Main::Main(int const, char const* const* argv) , hub_{window_} , processes_{window_.getExecutor(), window_, hub_} , prompter_{hub_} - , sshSessionManager_{} + , sshSessionManager_{&stateHolder_} , shuttingDown_{false} , childSignalTimer_{window_.getExecutor()} { diff --git a/backend/source/backend/sftp/operation_queue.cpp b/backend/source/backend/sftp/operation_queue.cpp index e69de29b..7638890d 100644 --- a/backend/source/backend/sftp/operation_queue.cpp +++ b/backend/source/backend/sftp/operation_queue.cpp @@ -0,0 +1,68 @@ +#include + +#include + +OperationQueue::OperationQueue() + : operations_{} +{} +void OperationQueue::start() +{} +void OperationQueue::pause() +{} +void OperationQueue::cancelAll() +{} +void OperationQueue::cancel(Ids::OperationId id) +{} +std::expected OperationQueue::addDownloadOperation( + Persistence::State const& state, + std::string const& sshSessionOptionsKey, + SecureShell::SftpSession& sftp, + Ids::OperationId id, + std::filesystem::path const& localPath, + std::filesystem::path const& remotePath) +{ + auto fut = sftp.openFile(remotePath, SecureShell::SftpSession::OpenType::Read, std::filesystem::perms::unknown); + if (fut.wait_for(std::chrono::seconds{5}) != std::future_status::ready) + { + Log::error("Failed to open remote sftp file: timeout"); + return std::unexpected(Operation::Error{.type = Operation::ErrorType::OpenFailure}); + } + + const auto result = fut.get(); + if (!result.has_value()) + { + Log::error("Failed to open remote sftp file: {}", result.error().message); + return std::unexpected(Operation::Error{.type = Operation::ErrorType::SftpError, .sftpError = result.error()}); + } + + auto sessionOptsIter = state.sshSessionOptions.find(sshSessionOptionsKey); + if (sessionOptsIter == state.sshSessionOptions.end()) + { + Log::error("No ssh session options found with key: {}", sshSessionOptionsKey); + return std::unexpected(Operation::Error{.type = Operation::ErrorType::InvalidOptionsKey}); + } + + auto sessionOpts = sessionOptsIter->second; + sessionOpts.sftpOptions.resolveWith(state.sftpOptions); + const auto& sftpOpts = sessionOpts.sftpOptions; + const auto transferOptions = sftpOpts->downloadOptions.value_or(Persistence::TransferOptions{}); + const auto defaultOptions = DownloadOperation::DownloadOperationOptions{}; + + auto operation = std::make_unique( + std::move(result).value(), + DownloadOperation::DownloadOperationOptions{ + .remotePath = remotePath, + .localPath = localPath, + .tempFileSuffix = transferOptions.tempFileSuffix.value_or(defaultOptions.tempFileSuffix), + .mayOverwrite = transferOptions.mayOverwrite.value_or(defaultOptions.mayOverwrite), + .reserveSpace = transferOptions.reserveSpace.value_or(defaultOptions.reserveSpace), + .tryContinue = transferOptions.tryContinue.value_or(defaultOptions.tryContinue), + .inheritPermissions = transferOptions.inheritPermissions.value_or(defaultOptions.inheritPermissions), + .doCleanup = transferOptions.doCleanup.value_or(defaultOptions.doCleanup), + .permissions = + transferOptions.customPermissions ? transferOptions.customPermissions : defaultOptions.permissions, + }); + + operations_.emplace_back(id, std::move(operation)); + return {}; +} \ No newline at end of file diff --git a/backend/source/backend/ssh_session_manager.cpp b/backend/source/backend/ssh_session_manager.cpp index 0777a378..61be0711 100644 --- a/backend/source/backend/ssh_session_manager.cpp +++ b/backend/source/backend/ssh_session_manager.cpp @@ -59,7 +59,8 @@ int askPassDefault(char const* prompt, char* buf, std::size_t length, int, int, return -1; } -SshSessionManager::SshSessionManager() +SshSessionManager::SshSessionManager(Persistence::StateHolder* stateHolder) + : stateHolder_(stateHolder) {} SshSessionManager::~SshSessionManager() @@ -311,9 +312,8 @@ void SshSessionManager::registerRpcChannelClose(Nui::Window&, Nui::RpcHub& hub) } else if (auto iter = sftpChannels_.find(channelId); iter != sftpChannels_.end()) { - if (auto channel = iter->second.sftp.lock(); channel) + if (auto channel = iter->second.lock(); channel) { - iter->second.fileStreams.clear(); channel->close(); } sftpChannels_.erase(iter); @@ -424,8 +424,8 @@ void SshSessionManager::registerRpcChannelPtyResize(Nui::Window&, Nui::RpcHub& h // if (fut.wait_for(std::chrono::milliseconds{500}) != std::future_status::ready) // { // Log::error("Failed to resize pty: timeout"); - // hub->callRemote(responseId, nlohmann::json{{"error", "Failed to resize pty: timeout"}}); - // return; + // hub->callRemote(responseId, nlohmann::json{{"error", "Failed to resize pty: + // timeout"}}); return; // } // const auto result = fut.get(); // if (result != 0) @@ -488,7 +488,7 @@ void SshSessionManager::registerRpcSftpListDirectory(Nui::Window&, Nui::RpcHub& return; } - auto locked = channel->second.sftp.lock(); + auto locked = channel->second.lock(); if (!locked) { Log::error("Failed to lock sftp channel with id: {}", channelId.value()); @@ -552,7 +552,7 @@ void SshSessionManager::registerRpcSftpCreateDirectory(Nui::Window&, Nui::RpcHub return; } - auto locked = channel->second.sftp.lock(); + auto locked = channel->second.lock(); if (!locked) { Log::error("Failed to lock sftp channel with id: {}", channelId.value()); @@ -616,7 +616,7 @@ void SshSessionManager::registerRpcSftpCreateFile(Nui::Window&, Nui::RpcHub& hub return; } - auto locked = channel->second.sftp.lock(); + auto locked = channel->second.lock(); if (!locked) { Log::error("Failed to lock sftp channel with id: {}", channelId.value()); @@ -651,6 +651,60 @@ void SshSessionManager::registerRpcSftpCreateFile(Nui::Window&, Nui::RpcHub& hub }); } +void SshSessionManager::registerRpcSftpAddDownloadOperation(Nui::Window&, Nui::RpcHub& hub) +{ + hub.registerFunction( + "SshSessionManager::sftp::addDownload", + [this, hub = &hub]( + std::string const& responseId, + std::string const& sessionIdString, + std::string const& channelIdString, + std::string const& newOperationIdString, + std::string const& sshSessionOptionsKey, + std::string const& localPath, + std::string const& remotePath) { + auto const& state = stateHolder_->stateCache(); + + const auto sessionId = Ids::makeSessionId(sessionIdString); + const auto channelId = Ids::makeChannelId(channelIdString); + + if (sessions_.find(sessionId) == sessions_.end()) + { + Log::error("No session found with id: {}", sessionId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "No session found with id"}}); + return; + } + + auto channel = sftpChannels_.find(channelId); + if (channel == sftpChannels_.end()) + { + Log::error("No sftp channel found with id: {}", channelId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "No sftp channel found with id"}}); + return; + } + + auto sftp = channel->second.lock(); + if (!sftp) + { + Log::error("Failed to lock sftp channel with id: {}", channelId.value()); + hub->callRemote(responseId, nlohmann::json{{"error", "Failed to lock sftp channel"}}); + return; + } + + const auto result = operationQueue_.addDownloadOperation( + state, sshSessionOptionsKey, *sftp, Ids::makeOperationId(newOperationIdString), localPath, remotePath); + + if (!result.has_value()) + { + Log::error("Failed to add download operation: {}", result.error().toString()); + hub->callRemote(responseId, nlohmann::json{{"error", result.error().toString()}}); + return; + } + + hub->callRemote(responseId, nlohmann::json{{"success", true}}); + }); +} + void SshSessionManager::registerRpc(Nui::Window& wnd, Nui::RpcHub& hub) { registerRpcConnect(wnd, hub); @@ -664,6 +718,8 @@ void SshSessionManager::registerRpc(Nui::Window& wnd, Nui::RpcHub& hub) registerRpcSftpListDirectory(wnd, hub); registerRpcSftpCreateDirectory(wnd, hub); registerRpcSftpCreateFile(wnd, hub); + + registerRpcSftpAddDownloadOperation(wnd, hub); } void SshSessionManager::addPasswordProvider(int priority, PasswordProvider* provider) diff --git a/persistence/include/persistence/state/sftp_options.hpp b/persistence/include/persistence/state/sftp_options.hpp new file mode 100644 index 00000000..e9775c87 --- /dev/null +++ b/persistence/include/persistence/state/sftp_options.hpp @@ -0,0 +1,35 @@ +#pragma once + +#include + +#include +#include +#include + +namespace Persistence +{ + struct TransferOptions + { + std::optional tempFileSuffix{std::nullopt}; + std::optional mayOverwrite{std::nullopt}; + std::optional reserveSpace{std::nullopt}; + std::optional tryContinue{std::nullopt}; + std::optional inheritPermissions{std::nullopt}; + std::optional doCleanup{std::nullopt}; + std::optional customPermissions{std::nullopt}; + + void useDefaultsFrom(TransferOptions const& other); + }; + void to_json(nlohmann::json& j, TransferOptions const& options); + void from_json(nlohmann::json const& j, TransferOptions& options); + + struct SftpOptions + { + std::optional downloadOptions{}; + std::optional uploadOptions{}; + + void useDefaultsFrom(SftpOptions const& other); + }; + void to_json(nlohmann::json& j, SftpOptions const& options); + void from_json(nlohmann::json const& j, SftpOptions& options); +} \ No newline at end of file diff --git a/persistence/include/persistence/state/ssh_session_options.hpp b/persistence/include/persistence/state/ssh_session_options.hpp index e972ffaf..0deecd41 100644 --- a/persistence/include/persistence/state/ssh_session_options.hpp +++ b/persistence/include/persistence/state/ssh_session_options.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -21,6 +22,7 @@ namespace Persistence std::optional defaultDirectory{std::nullopt}; Referenceable sshOptions{}; + Referenceable sftpOptions{}; void useDefaultsFrom(SshSessionOptions const& other); }; void to_json(nlohmann::json& j, SshSessionOptions const& options); diff --git a/persistence/include/persistence/state/state.hpp b/persistence/include/persistence/state/state.hpp index add9ca7e..7743ed91 100644 --- a/persistence/include/persistence/state/state.hpp +++ b/persistence/include/persistence/state/state.hpp @@ -16,6 +16,7 @@ namespace Persistence std::unordered_map terminalOptions{}; std::unordered_map termios{}; std::unordered_map sshOptions{}; + std::unordered_map sftpOptions{}; std::unordered_map sessions{}; std::unordered_map sshSessionOptions{}; UiOptions uiOptions{}; diff --git a/persistence/source/persistence/CMakeLists.txt b/persistence/source/persistence/CMakeLists.txt index 0e06eeb9..d4c1c7b1 100644 --- a/persistence/source/persistence/CMakeLists.txt +++ b/persistence/source/persistence/CMakeLists.txt @@ -2,6 +2,7 @@ add_library( persistence STATIC state/ssh_options.cpp + state/sftp_options.cpp state/ssh_session_options.cpp state/state.cpp state/terminal_engine.cpp diff --git a/persistence/source/persistence/state/sftp_options.cpp b/persistence/source/persistence/state/sftp_options.cpp new file mode 100644 index 00000000..30f42004 --- /dev/null +++ b/persistence/source/persistence/state/sftp_options.cpp @@ -0,0 +1,78 @@ +#include + +namespace Persistence +{ + void TransferOptions::useDefaultsFrom(TransferOptions const& other) + { + if (!tempFileSuffix) + tempFileSuffix = other.tempFileSuffix; + if (!mayOverwrite) + mayOverwrite = other.mayOverwrite; + if (!reserveSpace) + reserveSpace = other.reserveSpace; + if (!tryContinue) + tryContinue = other.tryContinue; + if (!inheritPermissions) + inheritPermissions = other.inheritPermissions; + if (!doCleanup) + doCleanup = other.doCleanup; + if (!customPermissions) + customPermissions = other.customPermissions; + } + void to_json(nlohmann::json& j, TransferOptions const& options) + { + if (options.tempFileSuffix) + j["tempFileSuffix"] = *options.tempFileSuffix; + if (options.mayOverwrite) + j["mayOverwrite"] = *options.mayOverwrite; + if (options.reserveSpace) + j["reserveSpace"] = *options.reserveSpace; + if (options.tryContinue) + j["tryContinue"] = *options.tryContinue; + if (options.inheritPermissions) + j["inheritPermissions"] = *options.inheritPermissions; + if (options.doCleanup) + j["doCleanup"] = *options.doCleanup; + if (options.customPermissions) + j["customPermissions"] = static_cast(*options.customPermissions); + } + void from_json(nlohmann::json const& j, TransferOptions& options) + { + if (j.contains("tempFileSuffix")) + options.tempFileSuffix = j["tempFileSuffix"].template get(); + if (j.contains("mayOverwrite")) + options.mayOverwrite = j["mayOverwrite"].get(); + if (j.contains("reserveSpace")) + options.reserveSpace = j["reserveSpace"].get(); + if (j.contains("tryContinue")) + options.tryContinue = j["tryContinue"].get(); + if (j.contains("inheritPermissions")) + options.inheritPermissions = j["inheritPermissions"].get(); + if (j.contains("doCleanup")) + options.doCleanup = j["doCleanup"].get(); + if (j.contains("customPermissions")) + options.customPermissions = std::filesystem::perms{j["customPermissions"].template get()}; + } + + void to_json(nlohmann::json& j, SftpOptions const& options) + { + if (options.downloadOptions) + j["downloadOptions"] = *options.downloadOptions; + if (options.uploadOptions) + j["uploadOptions"] = *options.uploadOptions; + } + void from_json(nlohmann::json const& j, SftpOptions& options) + { + if (j.contains("downloadOptions")) + options.downloadOptions = j["downloadOptions"].get(); + if (j.contains("uploadOptions")) + options.uploadOptions = j["uploadOptions"].get(); + } + void SftpOptions::useDefaultsFrom(SftpOptions const& other) + { + if (!downloadOptions) + downloadOptions = other.downloadOptions; + if (!uploadOptions) + uploadOptions = other.uploadOptions; + } +} \ No newline at end of file diff --git a/persistence/source/persistence/state/ssh_session_options.cpp b/persistence/source/persistence/state/ssh_session_options.cpp index e6db018a..d408ac8b 100644 --- a/persistence/source/persistence/state/ssh_session_options.cpp +++ b/persistence/source/persistence/state/ssh_session_options.cpp @@ -17,6 +17,7 @@ namespace Persistence if (!defaultDirectory.has_value()) defaultDirectory = other.defaultDirectory; + sftpOptions.useDefaultsFrom(other.sftpOptions.value()); sshOptions.useDefaultsFrom(other.sshOptions.value()); } @@ -31,6 +32,7 @@ namespace Persistence TO_JSON_OPTIONAL(j, options, defaultDirectory); j["openSftpByDefault"] = options.openSftpByDefault; j["sshOptions"] = options.sshOptions; + j["sftpOptions"] = options.sftpOptions; } void from_json(nlohmann::json const& j, SshSessionOptions& options) { @@ -47,5 +49,7 @@ namespace Persistence j.at("sshOptions").get_to(options.sshOptions); if (j.contains("openSftpByDefault")) j.at("openSftpByDefault").get_to(options.openSftpByDefault); + if (j.contains("sftpOptions")) + j.at("sftpOptions").get_to(options.sftpOptions); } } \ No newline at end of file diff --git a/persistence/source/persistence/state/state.cpp b/persistence/source/persistence/state/state.cpp index c6119d5e..0e23156e 100644 --- a/persistence/source/persistence/state/state.cpp +++ b/persistence/source/persistence/state/state.cpp @@ -13,6 +13,7 @@ namespace Persistence j["sessions"] = state.sessions; j["termios"] = state.termios; j["sshOptions"] = state.sshOptions; + j["sftpOptions"] = state.sftpOptions; j["sshSessionOptions"] = state.sshSessionOptions; j["uiOptions"] = state.uiOptions; j["logLevel"] = [&]() { @@ -33,6 +34,9 @@ namespace Persistence if (j.contains("sshOptions")) j.at("sshOptions").get_to(state.sshOptions); + if (j.contains("sftpOptions")) + j.at("sftpOptions").get_to(state.sftpOptions); + if (j.contains("sshSessionOptions")) j.at("sshSessionOptions").get_to(state.sshSessionOptions); diff --git a/ssh/source/ssh/CMakeLists.txt b/ssh/source/ssh/CMakeLists.txt index a0e36b57..1292dd21 100644 --- a/ssh/source/ssh/CMakeLists.txt +++ b/ssh/source/ssh/CMakeLists.txt @@ -6,6 +6,7 @@ add_library( channel.cpp sftp_session.cpp file_stream.cpp + file_information.cpp ) target_include_directories( diff --git a/ssh/test/ssh/utility/node.cpp b/ssh/test/ssh/utility/node.cpp index 5383945f..c1492a8b 100644 --- a/ssh/test/ssh/utility/node.cpp +++ b/ssh/test/ssh/utility/node.cpp @@ -89,7 +89,7 @@ namespace SecureShell::Test std::shared_ptr nodeProcess( boost::asio::any_io_executor executor, - TemporaryDirectory const& isolateDirectory, + Utility::TemporaryDirectory const& isolateDirectory, std::string const& program) { using namespace std::string_literals; diff --git a/ssh/test/ssh/utility/node.hpp b/ssh/test/ssh/utility/node.hpp index c94bac29..dda85759 100644 --- a/ssh/test/ssh/utility/node.hpp +++ b/ssh/test/ssh/utility/node.hpp @@ -42,6 +42,6 @@ namespace SecureShell::Test std::shared_ptr nodeProcess( boost::asio::any_io_executor executor, - TemporaryDirectory const& isolateDirectory, + Utility::TemporaryDirectory const& isolateDirectory, std::string const& program); } \ No newline at end of file From 7db12eac16c8fa4e21280566565e98997db82212 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Tue, 11 Mar 2025 19:41:31 +0100 Subject: [PATCH 13/42] Removed unused code. --- persistence/source/persistence/state/termios.cpp | 2 ++ ssh/test/ssh/utility/node.cpp | 16 ---------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/persistence/source/persistence/state/termios.cpp b/persistence/source/persistence/state/termios.cpp index 0aeda704..7bcdd839 100644 --- a/persistence/source/persistence/state/termios.cpp +++ b/persistence/source/persistence/state/termios.cpp @@ -9,10 +9,12 @@ namespace Persistence { namespace { +#ifndef _WIN32 bool unpackBoolOptional(std::optional const& value) { return value.has_value() ? value.value() : false; } +#endif } unsigned int Termios::InputFlags::assemble() const diff --git a/ssh/test/ssh/utility/node.cpp b/ssh/test/ssh/utility/node.cpp index c1492a8b..e4b136d9 100644 --- a/ssh/test/ssh/utility/node.cpp +++ b/ssh/test/ssh/utility/node.cpp @@ -108,22 +108,6 @@ namespace SecureShell::Test boost::asio::readable_pipe{executor}, nullptr); -#ifdef _WIN32 - const auto nodeShell = MSYS2_BASH; - const auto nodeCommandArgs = std::vector{ - "--login", - "-i", - "-c", - "\"cd "s + isolateDirectory.path().generic_string() + " && node ./main.mjs --log-file=./log.txt\"", - }; -#else - const auto nodeShell = "/bin/bash"; - const auto nodeCommandArgs = std::vector{ - "-c", - "cd "s + isolateDirectory.path().generic_string() + " && node ./main.mjs --log-file=./log.txt", - }; -#endif - result->mainModule = std::make_unique( executor, node, From d7872510cf7e7d979e9cdf3989591e9f78f18f60 Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Tue, 11 Mar 2025 19:44:23 +0100 Subject: [PATCH 14/42] Removed accidentally added clangd file. --- .clangd | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 .clangd diff --git a/.clangd b/.clangd deleted file mode 100644 index 9342164e..00000000 --- a/.clangd +++ /dev/null @@ -1,18 +0,0 @@ -If: - PathMatch: "(ssh\/.*)|(ids\/.*)|(backend\/.*)|(dependencies\/Nui\/.*)" - PathExclude: "(dependencies\/Nui\/nui\/include\/nui\/frontend.*)" -CompileFlags: - CompilationDatabase: "build/clang_debug" - Add: - - "-IE:/DevelopmentFast/scp/build/clang_debug/_deps/emscripten-src/upstream/emscripten/system/include" - - "-ID:/msys2/clang64/include" - - "-D__cplusplus=202302L" ---- -If: - PathMatch: "(frontend\/.*)|(nui-file-explorer\/.*)|(dependencies\/Nui\/nui\/include\/nui\/frontend.*)" -CompileFlags: - CompilationDatabase: "build/clang_debug/module_nui-scp" - Add: - - "-D__EMSCRIPTEN__" - - "-IE:/DevelopmentFast/scp/build/clang_debug/_deps/emscripten-src/upstream/emscripten/system/include" - - "-ID:/msys2/clang64/include" From 2c0b509a5e26043e374c8d5b1a3758e072409bdc Mon Sep 17 00:00:00 2001 From: Tim Ebbeke Date: Fri, 14 Mar 2025 19:56:24 +0100 Subject: [PATCH 15/42] Added handlers for file grid context menu. --- .../include/frontend/components/ui5-list.hpp | 24 ++ .../frontend/dialog/confirm_dialog.hpp | 30 ++- .../include/frontend/dialog/input_dialog.hpp | 16 +- .../source/frontend/dialog/confirm_dialog.cpp | 76 ++++-- .../source/frontend/dialog/input_dialog.cpp | 17 +- .../frontend/dialog/password_prompter.cpp | 11 +- frontend/source/frontend/session.cpp | 226 ++++++++++++++---- .../session_components/session_options.cpp | 44 +++- frontend/source/frontend/toolbar.cpp | 11 +- .../include/nui-file-explorer/file_grid.hpp | 63 +++++ .../source/nui-file-explorer/file_grid.cpp | 149 +++++++++++- nui-file-explorer/styles/file_grid.css | 16 ++ .../persistence/state/ssh_session_options.hpp | 2 + .../persistence/state/terminal_engine.hpp | 1 + .../persistence/state/ssh_session_options.cpp | 4 + .../persistence/state/terminal_engine.cpp | 2 + ssh/source/ssh/session.cpp | 12 + 17 files changed, 588 insertions(+), 116 deletions(-) create mode 100644 frontend/include/frontend/components/ui5-list.hpp diff --git a/frontend/include/frontend/components/ui5-list.hpp b/frontend/include/frontend/components/ui5-list.hpp new file mode 100644 index 00000000..efdbe1fd --- /dev/null +++ b/frontend/include/frontend/components/ui5-list.hpp @@ -0,0 +1,24 @@ +#pragma once + +#include + +// clang-format off + +#ifdef NUI_INLINE +// @inline(js, ui5-list) +js_import "@ui5/webcomponents/dist/List.js"; +js_import "@ui5/webcomponents/dist/ListItemStandard.js"; +js_import "@ui5/webcomponents/dist/ListItemCustom.js"; +js_import "@ui5/webcomponents/dist/ListItemGroup.js"; +// @endinline +#endif + +// clang-format on + +namespace ui5 +{ + NUI_MAKE_HTML_ELEMENT_RENAME(list, "ui5-list") + NUI_MAKE_HTML_ELEMENT_RENAME(li, "ui5-li") + NUI_MAKE_HTML_ELEMENT_RENAME(li_custom, "ui5-li-custom") + NUI_MAKE_HTML_ELEMENT_RENAME(li_group, "ui5-li-group") +} diff --git a/frontend/include/frontend/dialog/confirm_dialog.hpp b/frontend/include/frontend/dialog/confirm_dialog.hpp index fb1e1291..a72a3ae2 100644 --- a/frontend/include/frontend/dialog/confirm_dialog.hpp +++ b/frontend/include/frontend/dialog/confirm_dialog.hpp @@ -20,6 +20,11 @@ class ConfirmDialog No = 0b0000'1000, }; + friend auto operator|(Button lhs, Button rhs) + { + return static_cast