Update error handling for processing data blocks

source/include/ota_private.h

@@ -164,20 +164,19 @@
  */
 typedef enum
 {
-    IngestResultFileComplete = -1,       /*!< The file transfer is complete and the signature check passed. */
-    IngestResultSigCheckFail = -2,       /*!< The file transfer is complete but the signature check failed. */
-    IngestResultFileCloseFail = -3,      /*!< There was a problem trying to close the receive file. */
-    IngestResultNullContext = -4,        /*!< The specified OTA context pointer is null. */
-    IngestResultBadFileHandle = -5,      /*!< The receive file pointer is invalid. */
-    IngestResultUnexpectedBlock = -6,    /*!< We were asked to ingest a block but were not expecting one. */
-    IngestResultBlockOutOfRange = -7,    /*!< The received block is out of the expected range. */
-    IngestResultBadData = -8,            /*!< The data block from the server was malformed. */
-    IngestResultWriteBlockFailed = -9,   /*!< The PAL layer failed to write the file block. */
-    IngestResultNullResultPointer = -10, /*!< The pointer to the close result pointer was null. */
-    IngestResultNoDecodeMemory = -11,    /*!< Memory could not be allocated for decoding . */
-    IngestResultUninitialized = -127,    /*!< Software BUG: We forgot to set the result code. */
-    IngestResultAccepted_Continue = 0,   /*!< The block was accepted and we're expecting more. */
-    IngestResultDuplicate_Continue = 1   /*!< The block was a duplicate but that's OK. Continue. */
+    IngestResultFileComplete = -1,     /*!< The file transfer is complete and the signature check passed. */
+    IngestResultSigCheckFail = -2,     /*!< The file transfer is complete but the signature check failed. */
+    IngestResultFileCloseFail = -3,    /*!< There was a problem trying to close the receive file. */
+    IngestResultNullInput = -4,        /*!< One of the input pointers is NULL. */
+    IngestResultBadFileHandle = -5,    /*!< The receive file pointer is invalid. */
+    IngestResultUnexpectedBlock = -6,  /*!< We were asked to ingest a block but were not expecting one. */
+    IngestResultBlockOutOfRange = -7,  /*!< The received block is out of the expected range. */
+    IngestResultBadData = -8,          /*!< The data block from the server was malformed. */
+    IngestResultWriteBlockFailed = -9, /*!< The PAL layer failed to write the file block. */
+    IngestResultNoDecodeMemory = -10,  /*!< Memory could not be allocated for decoding . */
+    IngestResultUninitialized = -127,  /*!< Software BUG: We forgot to set the result code. */
+    IngestResultAccepted_Continue = 0, /*!< The block was accepted and we're expecting more. */
+    IngestResultDuplicate_Continue = 1 /*!< The block was a duplicate but that's OK. Continue. */
 } IngestResult_t;
 
 /**

source/ota.c

@@ -1158,15 +1158,23 @@ static OtaErr_t processDataHandler( const OtaEventData_t * pEventData )
     OtaErr_t err = OtaErrNone;
     OtaPalStatus_t closeResult = OTA_PAL_COMBINE_ERR( OtaPalUninitialized, 0 );
     OtaEventMsg_t eventMsg = { 0 };
+    IngestResult_t result = IngestResultUninitialized;
 
     /* Get the file context. */
     OtaFileContext_t * pFileContext = &( otaAgent.fileContext );
 
     /* Ingest data blocks received. */
-    IngestResult_t result = ingestDataBlock( pFileContext,
-                                             pEventData->data,
-                                             pEventData->dataLength,
-                                             &closeResult );
+    if( pEventData != NULL )
+    {
+        result = ingestDataBlock( pFileContext,
+                                  pEventData->data,
+                                  pEventData->dataLength,
+                                  &closeResult );
+    }
+    else
+    {
+        result = IngestResultNullInput;
+    }
 
     if( result == IngestResultFileComplete )
     {
@@ -2658,27 +2666,15 @@ static IngestResult_t ingestDataBlock( OtaFileContext_t * pFileContext,
     uint32_t uBlockIndex = 0;
     uint8_t * pPayload = NULL;
 
-    /* Check if the file context is NULL. */
-    if( pFileContext == NULL )
-    {
-        eIngestResult = IngestResultNullContext;
-    }
-
-    /* Check if the result pointer is NULL. */
-    if( eIngestResult == IngestResultUninitialized )
-    {
-        if( pCloseResult == NULL )
-        {
-            eIngestResult = IngestResultNullResultPointer;
-        }
-    }
+    /* Assume the file context and result pointers are not NULL. This function
+     * is only intended to be called by processDataHandler, which always passes
+     * them in as pointers to static variables. */
+    assert( pFileContext != NULL );
+    assert( pCloseResult != NULL );
 
     /* Decode the received data block. */
-    if( eIngestResult == IngestResultUninitialized )
-    {
-        /* If we have a block bitmap available then process the message. */
-        eIngestResult = decodeAndStoreDataBlock( pFileContext, pRawMsg, messageSize, &pPayload, &uBlockSize, &uBlockIndex );
-    }
+    /* If we have a block bitmap available then process the message. */
+    eIngestResult = decodeAndStoreDataBlock( pFileContext, pRawMsg, messageSize, &pPayload, &uBlockSize, &uBlockIndex );
 
     /* Validate the data block and process it to store the information.*/
     if( eIngestResult == IngestResultUninitialized )

test/unit-test/ota_utest.c

@@ -143,6 +143,7 @@ extern void receiveAndProcessOtaEvent( void );
 extern OtaErr_t initFileHandler( const OtaEventData_t * pEventData );
 extern OtaErr_t requestDataHandler( const OtaEventData_t * pEventData );
 extern OtaErr_t requestJobHandler( const OtaEventData_t * pEventData );
+extern OtaErr_t processDataHandler( const OtaEventData_t * pEventData );
 
 /* ========================================================================== */
 /* ====================== Unit test helper functions ======================== */
@@ -2229,6 +2230,21 @@ void test_OTA_requestJobHandler_EventSendFails( void )
     TEST_ASSERT_EQUAL( OtaErrSignalEventFailed, requestJobHandler( otaEvent.pEventData ) );
 }
 
+/**
+ * @brief Test that processDataHandler safely handles receiving invalid events.
+ */
+void test_OTA_processDataHandler_InvalidEvent( void )
+{
+    /* Initialize the OTA interfaces so they are not NULL. */
+    otaGoToState( OtaAgentStateReady );
+
+    /* Test that passing NULL event data does not cause a segmentation fault.
+     * The expected return is OtaErrNone because the error return value of this
+     * handler represents the success of updating the job document when there
+     * is an issue processing the block. */
+    TEST_ASSERT_EQUAL( OtaErrNone, processDataHandler( NULL ) );
+}
+
 /* ========================================================================== */
 /* ======================== OTA Interface Unit Tests ======================== */
 /* ========================================================================== */

tools/lexicon.txt

@@ -212,6 +212,7 @@ ingestresultfilecomplete
 ingestresultfilecomplete
 ingestresultnodecodememory
 ingestresultnullcontext
+ingestresultnullinput
 ingestresultnullresultpointer
 ingestresultsigcheckfail
 ingestresultunexpectedblock