From 893b43472872cdc90b4d602670c8fa9750ca7407 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 24 Feb 2023 16:49:23 +0100 Subject: [PATCH] allow dest handler to handle folder destinations --- src/fsfw/cfdp/handler/DestHandler.cpp | 64 ++++++++++++++++++--------- src/fsfw/cfdp/handler/DestHandler.h | 2 + src/fsfw/filesystem/HasFileSystemIF.h | 6 +++ src/fsfw_hal/host/HostFilesystem.cpp | 15 +++++++ src/fsfw_hal/host/HostFilesystem.h | 3 ++ unittests/mocks/FilesystemMock.cpp | 7 +++ unittests/mocks/FilesystemMock.h | 4 ++ 7 files changed, 81 insertions(+), 20 deletions(-) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 1b1187b1..4ffa797f 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -281,12 +281,10 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met } ReturnValue_t result = OK; size_t sourceNameSize = 0; + const uint8_t* sourceNamePtr = info.getSourceFileName().getValue(&sourceNameSize); if (sourceNameSize + 1 > tp.sourceName.size()) { - fp.eventReporter->forwardEvent(events::FILENAME_TOO_LARGE_ERROR, 0, 0); -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "cfdp::DestHandler: source filename too large" << std::endl; -#endif + fileErrorHandler(events::FILENAME_TOO_LARGE_ERROR, 0, "source filename too large"); return FAILED; } std::memcpy(tp.sourceName.data(), sourceNamePtr, sourceNameSize); @@ -294,10 +292,7 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met size_t destNameSize = 0; const uint8_t* destNamePtr = info.getDestFileName().getValue(&destNameSize); if (destNameSize + 1 > tp.destName.size()) { - fp.eventReporter->forwardEvent(events::FILENAME_TOO_LARGE_ERROR, 0, 0); -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "cfdp::DestHandler: dest filename too large" << std::endl; -#endif + fileErrorHandler(events::FILENAME_TOO_LARGE_ERROR, 0, "dest filename too large"); return FAILED; } std::memcpy(tp.destName.data(), destNamePtr, destNameSize); @@ -307,27 +302,25 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met // so there is no need to create a file or truncate an existing file if (destNameSize > 0 and sourceNameSize > 0) { FilesystemParams fparams(tp.destName.data()); + // handling to allow only specifying target directory. Example: + // Source path /test/hello.txt, dest path /tmp -> dest path /tmp/hello.txt + if (dp.user.vfs.isDirectory(tp.destName.data())) { + result = tryBuildingAbsoluteDestName(destNameSize); + if (result != OK) { + return result; + } + } if (dp.user.vfs.fileExists(fparams)) { result = dp.user.vfs.truncateFile(fparams); if (result != returnvalue::OK) { - // TODO: Provide execution step as parameter - fp.eventReporter->forwardEvent(events::FILESTORE_ERROR, static_cast(fsmRes.step), - result); -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "cfdp::DestHandler: file truncation error with code " << result - << std::endl; -#endif + fileErrorHandler(events::FILESTORE_ERROR, result, "file truncation error"); return FAILED; // TODO: Relevant for filestore rejection error? } } else { result = dp.user.vfs.createFile(fparams); if (result != OK) { - fp.eventReporter->forwardEvent(events::FILESTORE_ERROR, static_cast(fsmRes.step), - result); -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "cfdp::DestHandler: file creation error with code " << result << std::endl; -#endif + fileErrorHandler(events::FILESTORE_ERROR, result, "file creation error"); return FAILED; // TODO: Relevant for filestore rejection error? } @@ -394,6 +387,37 @@ ReturnValue_t cfdp::DestHandler::handleTransferCompletion() { return OK; } +ReturnValue_t cfdp::DestHandler::tryBuildingAbsoluteDestName(size_t destNameSize) { + char baseNameBuf[tp.destName.size()]{}; + FilesystemParams fparamsSrc(tp.sourceName.data()); + size_t baseNameLen = 0; + ReturnValue_t result = + dp.user.vfs.getBaseFilename(fparamsSrc, baseNameBuf, sizeof(baseNameBuf), baseNameLen); + if (result != returnvalue::OK or baseNameLen == 0) { + fileErrorHandler(events::FILENAME_TOO_LARGE_ERROR, 0, "error retrieving source base name"); + return FAILED; + } + // Destination name + slash + base name + null termination + if (destNameSize + 1 + baseNameLen + 1 > tp.destName.size()) { + fileErrorHandler(events::FILENAME_TOO_LARGE_ERROR, 0, + "dest filename too large after adding source base name"); + return FAILED; + } + tp.destName[destNameSize++] = '/'; + std::memcpy(tp.destName.data() + destNameSize, baseNameBuf, baseNameLen); + destNameSize += baseNameLen; + tp.destName[destNameSize++] = '\0'; + return OK; +} + +void cfdp::DestHandler::fileErrorHandler(Event event, ReturnValue_t result, const char* info) { + fp.eventReporter->forwardEvent(events::FILENAME_TOO_LARGE_ERROR, + static_cast(fsmRes.step), result); +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "cfdp::DestHandler: " << info << std::endl; +#endif +} + void cfdp::DestHandler::finish() { tp.reset(); dp.packetListRef.clear(); diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h index 357493df..9057b3f5 100644 --- a/src/fsfw/cfdp/handler/DestHandler.h +++ b/src/fsfw/cfdp/handler/DestHandler.h @@ -191,9 +191,11 @@ class DestHandler { ReturnValue_t handleMetadataParseError(ReturnValue_t result, const uint8_t* rawData, size_t maxSize); ReturnValue_t handleTransferCompletion(); + ReturnValue_t tryBuildingAbsoluteDestName(size_t destNameSize); ReturnValue_t sendFinishedPdu(); ReturnValue_t noticeOfCompletion(); ReturnValue_t checksumVerification(); + void fileErrorHandler(Event event, ReturnValue_t result, const char* info); const FsmResult& updateFsmRes(uint8_t errors); void checkAndHandleError(ReturnValue_t result, uint8_t& errorIdx); void finish(); diff --git a/src/fsfw/filesystem/HasFileSystemIF.h b/src/fsfw/filesystem/HasFileSystemIF.h index 24400b1c..a507938e 100644 --- a/src/fsfw/filesystem/HasFileSystemIF.h +++ b/src/fsfw/filesystem/HasFileSystemIF.h @@ -74,6 +74,12 @@ class HasFileSystemIF { return MessageQueueIF::NO_QUEUE; } + // Get the base filename without the full directory path + virtual ReturnValue_t getBaseFilename(FilesystemParams params, char* nameBuf, size_t maxLen, + size_t& baseNameLen) = 0; + + virtual bool isDirectory(const char* path) = 0; + virtual bool fileExists(FilesystemParams params) = 0; /** diff --git a/src/fsfw_hal/host/HostFilesystem.cpp b/src/fsfw_hal/host/HostFilesystem.cpp index fe593f27..d430d0f0 100644 --- a/src/fsfw_hal/host/HostFilesystem.cpp +++ b/src/fsfw_hal/host/HostFilesystem.cpp @@ -160,3 +160,18 @@ ReturnValue_t HostFilesystem::truncateFile(FilesystemParams params) { ofstream of(path); return returnvalue::OK; } + +bool HostFilesystem::isDirectory(const char *path) { return filesystem::is_directory(path); } + +ReturnValue_t HostFilesystem::getBaseFilename(FilesystemParams params, char *nameBuf, size_t maxLen, + size_t &baseNameLen) { + std::string path(params.path); + std::string baseName = path.substr(path.find_last_of("/\\") + 1); + if (baseName.size() + 1 > maxLen) { + return returnvalue::FAILED; + } + std::memcpy(nameBuf, baseName.c_str(), baseName.size()); + nameBuf[baseName.size()] = '\0'; + baseNameLen = baseName.size(); + return returnvalue::OK; +} diff --git a/src/fsfw_hal/host/HostFilesystem.h b/src/fsfw_hal/host/HostFilesystem.h index 7b865e2d..da217aec 100644 --- a/src/fsfw_hal/host/HostFilesystem.h +++ b/src/fsfw_hal/host/HostFilesystem.h @@ -9,6 +9,9 @@ class HostFilesystem : public HasFileSystemIF { public: HostFilesystem(); + ReturnValue_t getBaseFilename(FilesystemParams params, char *nameBuf, size_t maxLen, + size_t &baseNameLen) override; + bool isDirectory(const char *path) override; bool fileExists(FilesystemParams params) override; ReturnValue_t truncateFile(FilesystemParams params) override; ReturnValue_t writeToFile(FileOpParams params, const uint8_t *data) override; diff --git a/unittests/mocks/FilesystemMock.cpp b/unittests/mocks/FilesystemMock.cpp index bf0c3bf6..24850227 100644 --- a/unittests/mocks/FilesystemMock.cpp +++ b/unittests/mocks/FilesystemMock.cpp @@ -138,3 +138,10 @@ ReturnValue_t FilesystemMock::truncateFile(FilesystemParams params) { truncateCalledOnFile = params.path; return returnvalue::OK; } + +ReturnValue_t FilesystemMock::getBaseFilename(FilesystemParams params, char *nameBuf, size_t maxLen, + size_t &baseNameLen) { + return returnvalue::OK; +} + +bool FilesystemMock::isDirectory(const char *path) { return false; } diff --git a/unittests/mocks/FilesystemMock.h b/unittests/mocks/FilesystemMock.h index 74221d70..2ddbefc3 100644 --- a/unittests/mocks/FilesystemMock.h +++ b/unittests/mocks/FilesystemMock.h @@ -56,6 +56,10 @@ class FilesystemMock : public HasFileSystemIF { std::string truncateCalledOnFile; ReturnValue_t feedFile(const std::string &filename, std::ifstream &file); + ReturnValue_t getBaseFilename(FilesystemParams params, char *nameBuf, size_t maxLen, + size_t &baseNameLen) override; + + bool isDirectory(const char *path) override; bool fileExists(FilesystemParams params) override; ReturnValue_t truncateFile(FilesystemParams params) override;