Os::File::open Bounded Inputs

Os/File.cpp

@@ -3,8 +3,10 @@
 // \brief common function implementation for Os::File
 // ======================================================================
 #include <Fw/Types/Assert.hpp>
+#include <Fw/Types/StringUtils.hpp>
 #include <Os/File.hpp>
 #include <algorithm>
+#include <config/FppConstantsAc.hpp>
 
 extern "C" {
 #include <Utils/Hash/libcrc/lib_crc.h>  // borrow CRC
@@ -48,8 +50,21 @@ File::Status File::open(const CHAR* filepath, File::Mode requested_mode) {
 }
 
 File::Status File::open(const CHAR* filepath, File::Mode requested_mode, File::OverwriteType overwrite) {
+    FW_ASSERT(nullptr != filepath);
+    return this->open(filepath, static_cast<FwSizeType>(FileNameStringSize + 1), requested_mode, overwrite);
+}
+
+File::Status File::open(const CHAR* filepath, FwSizeType length, File::Mode requested_mode) {
+    return this->open(filepath, length, requested_mode, OverwriteType::NO_OVERWRITE);
+}
+
+File::Status File::open(const CHAR* filepath,
+                        FwSizeType length,
+                        File::Mode requested_mode,
+                        File::OverwriteType overwrite) {
     FW_ASSERT(&this->m_delegate == reinterpret_cast<FileInterface*>(&this->m_handle_storage[0]));
     FW_ASSERT(nullptr != filepath);
+    FW_ASSERT(Fw::StringUtils::string_length(filepath, length) < length);
     FW_ASSERT(File::Mode::OPEN_NO_MODE < requested_mode && File::Mode::MAX_OPEN_MODE > requested_mode);
     FW_ASSERT((0 <= this->m_mode) && (this->m_mode < Mode::MAX_OPEN_MODE));
     FW_ASSERT((0 <= overwrite) && (overwrite < OverwriteType::MAX_OVERWRITE_TYPE));
@@ -68,6 +83,15 @@ File::Status File::open(const CHAR* filepath, File::Mode requested_mode, File::O
     return status;
 }
 
+File::Status File::open(const Fw::ConstStringBase& path, File::Mode requested_mode) {
+    return this->open(path.toChar(), static_cast<FwSizeType>(path.getCapacity()), requested_mode,
+                      OverwriteType::NO_OVERWRITE);
+}
+
+File::Status File::open(const Fw::ConstStringBase& path, File::Mode requested_mode, File::OverwriteType overwrite) {
+    return this->open(path.toChar(), static_cast<FwSizeType>(path.getCapacity()), requested_mode, overwrite);
+}
+
 void File::close() {
     FW_ASSERT(&this->m_delegate == reinterpret_cast<FileInterface*>(&this->m_handle_storage[0]));
     FW_ASSERT(this->m_mode < Mode::MAX_OPEN_MODE);

Os/File.hpp

@@ -6,6 +6,7 @@
 #define Os_File_hpp_
 
 #include <Fw/FPrimeBasicTypes.hpp>
+#include <Fw/Types/ConstStringBase.hpp>
 #include <Os/Os.hpp>
 
 // Forward declaration for UTs
@@ -264,6 +265,51 @@ class File final : public FileInterface {
     //!
     Os::FileInterface::Status open(const char* path, Mode mode);
 
+    //! \brief open file with supplied path, bounded length, and mode
+    //!
+    //! Open the file passed in with the given mode. The path length is bounded by `length`.
+    //! Opening files with `OPEN_CREATE` mode will not clobber existing files. Use the overload
+    //! accepting `OverwriteType` to set overwrite flag and clobber existing files.
+    //!
+    //! It is invalid to send `nullptr` as the path.
+    //! It is invalid to supply `mode` as a non-enumerated value.
+    //! It is invalid for the path to not be null-terminated within `length` characters.
+    //!
+    //! \param path: c-string of path to open
+    //! \param length: bound on the path buffer size
+    //! \param mode: file operation mode
+    //! \return: status of the open
+    //!
+    Os::FileInterface::Status open(const char* path, FwSizeType length, Mode mode);
+
+    //! \brief open file with supplied string path and mode
+    //!
+    //! Open the file passed in with the given mode. Opening files with `OPEN_CREATE` mode will not clobber existing
+    //! files. Use the overload accepting `OverwriteType` to set overwrite flag and clobber existing files.
+    //!
+    //! It is invalid to supply `mode` as a non-enumerated value.
+    //!
+    //! \param path: ConstStringBase reference of path to open
+    //! \param mode: file operation mode
+    //! \return: status of the open
+    //!
+    Os::FileInterface::Status open(const Fw::ConstStringBase& path, Mode mode);
+
+    //! \brief open file with supplied string path, mode, and overwrite type
+    //!
+    //! Open the file passed in with the given mode. If overwrite is set to OVERWRITE, then opening files in
+    //! OPEN_CREATE mode will clobber existing files. Set overwrite to NO_OVERWRITE to preserve existing files.
+    //!
+    //! It is invalid to supply `mode` as a non-enumerated value.
+    //! It is invalid to supply `overwrite` as a non-enumerated value.
+    //!
+    //! \param path: ConstStringBase reference of path to open
+    //! \param mode: file operation mode
+    //! \param overwrite: overwrite existing file on create
+    //! \return: status of the open
+    //!
+    Os::FileInterface::Status open(const Fw::ConstStringBase& path, Mode mode, OverwriteType overwrite);
+
     //! \brief read data from this file into supplied buffer bounded by size
     //!
     //! Read data from this file up to the `size` and store it in `buffer`.  This version will
@@ -321,6 +367,26 @@ class File final : public FileInterface {
     //!
     Os::FileInterface::Status open(const char* path, Mode mode, OverwriteType overwrite) override;
 
+    //! \brief open file with supplied path, bounded length, mode, and overwrite type
+    //!
+    //! Open the file passed in with the given mode. The path length is bounded by `length`.
+    //! If overwrite is set to OVERWRITE, then opening files in OPEN_CREATE mode will clobber
+    //! existing files. Set overwrite to NO_OVERWRITE to preserve existing files. This is the
+    //! core open implementation to which all other open overloads delegate.
+    //!
+    //! It is invalid to send `nullptr` as the path.
+    //! It is invalid to supply `mode` as a non-enumerated value.
+    //! It is invalid to supply `overwrite` as a non-enumerated value.
+    //! It is invalid for the path to not be null-terminated within `length` characters.
+    //!
+    //! \param path: c-string of path to open
+    //! \param length: bound on the path buffer size
+    //! \param mode: file operation mode
+    //! \param overwrite: overwrite existing file on create
+    //! \return: status of the open
+    //!
+    Os::FileInterface::Status open(const char* path, FwSizeType length, Mode mode, OverwriteType overwrite);
+
     //! \brief close the file, if not opened then do nothing
     //!
     //! Closes the file, if open. Otherwise this function does nothing. Delegates to the chosen implementation's

Os/test/ut/file/CommonTests.cpp

@@ -66,6 +66,24 @@ TEST_F(Functionality, CopyConstructor) {
     close_rule.apply(*tester);
 }
 
+// Ensure that open works via the bounded char* overload
+TEST_F(Functionality, OpenWithCreationBounded) {
+    Os::Test::FileTest::Tester::OpenFileCreateBounded rule(false);
+    rule.apply(*tester);
+}
+
+// Ensure that open works via the ConstStringBase overload
+TEST_F(Functionality, OpenWithCreationString) {
+    Os::Test::FileTest::Tester::OpenFileCreateString rule(false);
+    rule.apply(*tester);
+}
+
+// Ensure that open asserts on unterminated path within bounds
+TEST_F(InvalidArguments, OpenBoundedPathUnterminated) {
+    Os::Test::FileTest::Tester::OpenIllegalBoundedPath rule;
+    rule.apply(*tester);
+}
+
 // Ensure that open on existence works
 TEST_F(FunctionalIO, OpenWithCreationExists) {
     Os::Test::FileTest::Tester::OpenFileCreate open_rule(false);
@@ -310,6 +328,9 @@ TEST_F(Functionality, RandomizedInterfaceTesting) {
     Os::Test::FileTest::Tester::WriteIllegalBuffer write_illegal_buffer;
     Os::Test::FileTest::Tester::IncrementalCrcInvalidModes incremental_invalid_mode_rule;
     Os::Test::FileTest::Tester::FullCrcInvalidModes full_invalid_mode_rule;
+    Os::Test::FileTest::Tester::OpenFileCreateBounded open_file_create_bounded_rule(true);
+    Os::Test::FileTest::Tester::OpenFileCreateString open_file_create_string_rule(true);
+    Os::Test::FileTest::Tester::OpenIllegalBoundedPath open_illegal_bounded_path;
 
     // Place these rules into a list of rules
     STest::Rule<Os::Test::FileTest::Tester>* rules[] = {&open_file_create_overwrite_rule,
@@ -328,7 +349,10 @@ TEST_F(Functionality, RandomizedInterfaceTesting) {
                                                         &read_illegal_buffer,
                                                         &write_illegal_buffer,
                                                         &incremental_invalid_mode_rule,
-                                                        &full_invalid_mode_rule};
+                                                        &full_invalid_mode_rule,
+                                                        &open_file_create_bounded_rule,
+                                                        &open_file_create_string_rule,
+                                                        &open_illegal_bounded_path};
 
     // Take the rules and place them into a random scenario
     STest::RandomScenario<Os::Test::FileTest::Tester> random("Random Rules", rules, FW_NUM_ARRAY_ELEMENTS(rules));
@@ -368,6 +392,9 @@ TEST_F(FunctionalIO, RandomizedTesting) {
     Os::Test::FileTest::Tester::WriteInvalidModes write_invalid_modes_rule;
     Os::Test::FileTest::Tester::IncrementalCrcInvalidModes incremental_invalid_mode_rule;
     Os::Test::FileTest::Tester::FullCrcInvalidModes full_invalid_mode_rule;
+    Os::Test::FileTest::Tester::OpenFileCreateBounded open_file_create_bounded_rule(true);
+    Os::Test::FileTest::Tester::OpenFileCreateString open_file_create_string_rule(true);
+    Os::Test::FileTest::Tester::OpenIllegalBoundedPath open_illegal_bounded_path;
 
     // Place these rules into a list of rules
     STest::Rule<Os::Test::FileTest::Tester>* rules[] = {&open_file_create_rule,
@@ -393,7 +420,10 @@ TEST_F(FunctionalIO, RandomizedTesting) {
                                                         &read_invalid_modes_rule,
                                                         &write_invalid_modes_rule,
                                                         &incremental_invalid_mode_rule,
-                                                        &full_invalid_mode_rule};
+                                                        &full_invalid_mode_rule,
+                                                        &open_file_create_bounded_rule,
+                                                        &open_file_create_string_rule,
+                                                        &open_illegal_bounded_path};
 
     // Take the rules and place them into a random scenario
     STest::RandomScenario<Os::Test::FileTest::Tester> random("Random Rules", rules, FW_NUM_ARRAY_ELEMENTS(rules));

Os/test/ut/file/FileRules.cpp

@@ -2,6 +2,7 @@
 // \title Os/test/ut/file/MyRules.cpp
 // \brief rule implementations for common testing
 // ======================================================================
+#include <Fw/Types/String.hpp>
 #include <algorithm>
 #include <cstdio>
 #include "RulesHeaders.hpp"
@@ -383,6 +384,106 @@ Os::Test::FileTest::Tester::OpenForRead::OpenForRead(const bool randomize_filena
                                                static_cast<bool>(STest::Pick::lowerUpper(0, 1)),
                                                randomize_filename) {}
 
+// ------------------------------------------------------------------------------------------------------
+// Rule:  OpenFileCreateBounded
+//
+// ------------------------------------------------------------------------------------------------------
+
+Os::Test::FileTest::Tester::OpenFileCreateBounded::OpenFileCreateBounded(const bool randomize_filename)
+    : Os::Test::FileTest::Tester::OpenBaseRule("OpenFileCreateBounded",
+                                               Os::File::Mode::OPEN_CREATE,
+                                               false,
+                                               randomize_filename) {}
+
+void Os::Test::FileTest::Tester::OpenFileCreateBounded::action(Os::Test::FileTest::Tester& state  //!< The test state
+) {
+    printf("--> Rule: %s mode %d\n", this->getName(), this->m_mode);
+    // Initial variables used for this test
+    std::shared_ptr<const std::string> filename = state.get_filename(this->m_random);
+    // When randomly generating filenames, some seeds can result in duplicate filenames
+    // Continue generating until unique
+    constexpr U32 MAX_FILENAME_ATTEMPTS = 100000;
+    U32 attempts = 0;
+    if (this->m_random) {
+        while (state.exists(*filename)) {
+            filename = state.get_filename(this->m_random);
+            attempts++;
+            ASSERT_LT(attempts, MAX_FILENAME_ATTEMPTS)
+                << "Failed to generate unique filename after " << attempts << " attempts. "
+                << "Consider expanding the filename generation in get_filename().";
+        }
+    }
+
+    // Ensure initial and shadow states synchronized
+    state.assert_file_consistent();
+    state.assert_file_closed();
+
+    // Perform action using the bounded char* open overload
+    FwSizeType length = static_cast<FwSizeType>(filename->length() + 1);
+    Os::File::Status status = state.m_file.open(filename->c_str(), length, m_mode, this->m_overwrite);
+    Os::File::Status s2 = state.shadow_open(*filename, m_mode, this->m_overwrite);
+    ASSERT_EQ(status, s2);
+
+    // Extra check to ensure file is consistently open
+    if (Os::File::Status::OP_OK == status) {
+        state.assert_file_opened(*filename, m_mode);
+        FileState file_state = state.current_file_state();
+        ASSERT_EQ(file_state.position, 0);  // Open always zeros the position
+    }
+    // Assert the file state remains consistent.
+    state.assert_file_consistent();
+}
+
+// ------------------------------------------------------------------------------------------------------
+// Rule:  OpenFileCreateString
+//
+// ------------------------------------------------------------------------------------------------------
+
+Os::Test::FileTest::Tester::OpenFileCreateString::OpenFileCreateString(const bool randomize_filename)
+    : Os::Test::FileTest::Tester::OpenBaseRule("OpenFileCreateString",
+                                               Os::File::Mode::OPEN_CREATE,
+                                               false,
+                                               randomize_filename) {}
+
+void Os::Test::FileTest::Tester::OpenFileCreateString::action(Os::Test::FileTest::Tester& state  //!< The test state
+) {
+    printf("--> Rule: %s mode %d\n", this->getName(), this->m_mode);
+    // Initial variables used for this test
+    std::shared_ptr<const std::string> filename = state.get_filename(this->m_random);
+    // When randomly generating filenames, some seeds can result in duplicate filenames
+    // Continue generating until unique
+    constexpr U32 MAX_FILENAME_ATTEMPTS = 100000;
+    U32 attempts = 0;
+    if (this->m_random) {
+        while (state.exists(*filename)) {
+            filename = state.get_filename(this->m_random);
+            attempts++;
+            ASSERT_LT(attempts, MAX_FILENAME_ATTEMPTS)
+                << "Failed to generate unique filename after " << attempts << " attempts. "
+                << "Consider expanding the filename generation in get_filename().";
+        }
+    }
+
+    // Ensure initial and shadow states synchronized
+    state.assert_file_consistent();
+    state.assert_file_closed();
+
+    // Perform action using the ConstStringBase open overload
+    Fw::String path(filename->c_str());
+    Os::File::Status status = state.m_file.open(path, m_mode, this->m_overwrite);
+    Os::File::Status s2 = state.shadow_open(*filename, m_mode, this->m_overwrite);
+    ASSERT_EQ(status, s2);
+
+    // Extra check to ensure file is consistently open
+    if (Os::File::Status::OP_OK == status) {
+        state.assert_file_opened(*filename, m_mode);
+        FileState file_state = state.current_file_state();
+        ASSERT_EQ(file_state.position, 0);  // Open always zeros the position
+    }
+    // Assert the file state remains consistent.
+    state.assert_file_consistent();
+}
+
 // ------------------------------------------------------------------------------------------------------
 // Rule:  CloseFile
 //
@@ -831,6 +932,33 @@ void Os::Test::FileTest::Tester::OpenIllegalPath::action(Os::Test::FileTest::Tes
     state.assert_file_consistent();
 }
 
+// ------------------------------------------------------------------------------------------------------
+// Rule:  OpenIllegalBoundedPath
+//
+// ------------------------------------------------------------------------------------------------------
+
+Os::Test::FileTest::Tester::OpenIllegalBoundedPath::OpenIllegalBoundedPath()
+    : Os::Test::FileTest::Tester::AssertRule("OpenIllegalBoundedPath") {}
+
+void Os::Test::FileTest::Tester::OpenIllegalBoundedPath::action(Os::Test::FileTest::Tester& state  //!< The test state
+) {
+    printf("--> Rule: %s \n", this->getName());
+    state.assert_file_consistent();
+    // Create a buffer filled with non-null characters with no null terminator within bounds
+    constexpr FwSizeType BOUND = 10;
+    CHAR path[BOUND];
+    memset(path, 'A', BOUND);  // Fill entirely with 'A', no null terminator within BOUND
+
+    Os::File::Mode random_mode =
+        static_cast<Os::File::Mode>(STest::Pick::lowerUpper(Os::File::Mode::OPEN_READ, Os::File::Mode::OPEN_APPEND));
+    bool overwrite = static_cast<bool>(STest::Pick::lowerUpper(0, 1));
+    ASSERT_DEATH_IF_SUPPORTED(
+        state.m_file.open(path, BOUND, random_mode,
+                          overwrite ? Os::File::OverwriteType::OVERWRITE : Os::File::OverwriteType::NO_OVERWRITE),
+        ASSERT_IN_FILE_CPP);
+    state.assert_file_consistent();
+}
+
 // ------------------------------------------------------------------------------------------------------
 // Rule:  OpenIllegalMode
 //