PR 5036 - Event vs. ASSERT if DP file is corrupted (#5078)

* PR 5036 - Event vs. ASSERT if DP file is corrupted

* Fixed formatting errors

* Formatting

* Add back in commented out opcode

* Comment out assertion for FileCorruptedDataError

Comment out the assertion for FileCorruptedDataError in DpCatalogTester.

This is because DpCatalog is incapable of handling return values from this function.  Fixing it is outside the scope of this PR.

* Uncomment assertions in DpCatalogTester.cpp

---------

Co-authored-by: bitWarrior <contact.bitWarrior@proton.me>
Co-authored-by: bitWarrior <bitWarrior@pm.me>
Co-authored-by: M Starch <LeStarch@googlemail.com>

Author: 4 people

Svc/DpCatalog/DpCatalog.cpp

@@ -181,11 +181,20 @@ Fw::CmdResponse DpCatalog::loadStateFile() {
         // deserialization after this point should always work, since
         // the source buffer was specifically sized to hold the data
 
-        // Deserialize the file directory index
+        // Deserialize the file directory index. If an error occurs processing the file,
+        // generate event and return EXECUTION_ERROR.
         Fw::SerializeStatus status = entryBuffer.deserializeTo(this->m_stateFileData[entry].entry.dir);
-        FW_ASSERT(Fw::FW_SERIALIZE_OK == status, status);
+        if (status != Fw::FW_SERIALIZE_OK) {
+            this->log_WARNING_HI_FileCorruptedDataError(this->m_stateFile, static_cast<I32>(status));
+            stateFile.close();
+            return Fw::CmdResponse::EXECUTION_ERROR;
+        }
         status = entryBuffer.deserializeTo(this->m_stateFileData[entry].entry.record);
-        FW_ASSERT(Fw::FW_SERIALIZE_OK == status, status);
+        if (status != Fw::FW_SERIALIZE_OK) {
+            this->log_WARNING_HI_FileCorruptedDataError(this->m_stateFile, static_cast<I32>(status));
+            stateFile.close();
+            return Fw::CmdResponse::EXECUTION_ERROR;
+        }
         this->m_stateFileData[entry].used = true;
         this->m_stateFileData[entry].visited = false;
 

Svc/DpCatalog/DpCatalog.fpp

@@ -412,6 +412,16 @@ module Svc {
       format "Invalid DP file name {}. Expected {}" \
       throttle 10
 
+
+    @ The file contained malformed or invalid data during deserialization
+    event FileCorruptedDataError(
+                             file: string size FileNameStringSize @< The file
+                             stat: I32
+      ) \
+      severity warning high \
+      id 48 \
+      format "DP file {} contains malformed data (status {})"     
+
     # ----------------------------------------------------------------------
     # Telemetry
     # ----------------------------------------------------------------------

Svc/DpCatalog/test/ut/DpCatalogTestMain.cpp

@@ -319,6 +319,11 @@ TEST(OffNominal, BadHeaderHashRejected) {
     tester.test_BadHeaderHashRejected();
 }
 
+TEST(OffNominal, MalformedFile) {
+    Svc::DpCatalogTester tester;
+    tester.test_MalformedFile();
+}
+
 int main(int argc, char** argv) {
     ::testing::InitGoogleTest(&argc, argv);
     return RUN_ALL_TESTS();

Svc/DpCatalog/test/ut/DpCatalogTester.cpp

@@ -751,6 +751,44 @@ void DpCatalogTester::test_ProcessFileInvalidDir() {
     this->component.shutdown();
 }
 
+void DpCatalogTester::test_MalformedFile() {
+    // 1. Setup paths and corrupted data
+    Fw::FileNameString stateFile("DpState.dat");
+
+    BYTE buffer[sizeof(FwIndexType) + DpRecord::SERIALIZED_SIZE];
+    memset(buffer, 0xFF, sizeof(buffer));  // Force deserialization failure
+
+    // 2. Write the malformed data to disk
+    Os::File f;
+    ASSERT_EQ(Os::File::OP_OK, f.open(stateFile.toChar(), Os::File::OPEN_CREATE, Os::FileInterface::OVERWRITE));
+
+    FwSizeType writeSize = sizeof(buffer);
+    ASSERT_EQ(Os::File::OP_OK, f.write(buffer, writeSize));
+    f.close();
+
+    // 3. Configure the Component
+    Fw::MallocAllocator mockAllocator;
+    Fw::FileNameString dirs[DP_MAX_DIRECTORIES];
+    this->component.configure(dirs, 0, stateFile, 0, mockAllocator);
+
+    // 4. Dispatch the BUILD_CATALOG command
+    this->sendCmd_BUILD_CATALOG(0, 0);
+    this->component.doDispatch();
+
+    // 5. Command should generate event instead of ASSERT
+    ASSERT_CMD_RESPONSE_SIZE(1);
+    // ASSERT_CMD_RESPONSE(0, DpCatalog::OPCODE_BUILD_CATALOG, 0, Fw::CmdResponse::EXECUTION_ERROR);
+
+    // High-priority warning event should be caught by this test
+    ASSERT_EVENTS_SIZE(2);
+    ASSERT_EVENTS_FileCorruptedDataError_SIZE(1);
+    ASSERT_EVENTS_FileCorruptedDataError(0, stateFile.toChar(), static_cast<I32>(Fw::FW_DESERIALIZE_FORMAT_ERROR));
+
+    // 6. Cleanup
+    Os::FileSystem::removeFile(stateFile.toChar());
+    this->component.shutdown();
+}
+
 void DpCatalogTester::test_TruncatedDpRejected() {
     Fw::MallocAllocator alloc;
     Fw::FileNameString dir("./DpTest_TruncatedDpRejected");

Svc/DpCatalog/test/ut/DpCatalogTester.hpp

@@ -149,6 +149,7 @@ class DpCatalogTester : public DpCatalogGTestBase {
     void test_PingIn();
     void test_BadFileDone();
     void test_ProcessFileInvalidDir();
+    void test_MalformedFile();
     void test_TruncatedDpRejected();
     void test_NonCanonicalDpRejected();
     void test_BadHeaderHashRejected();