From e5c5fb4a2e62492ab1972a10ebf166e3b7147727 Mon Sep 17 00:00:00 2001
From: Kody Stribrny <89810515+kstribrnAmzn@users.noreply.github.com>
Date: Tue, 21 Feb 2023 11:20:00 -0800
Subject: [PATCH] Fix stringBuilderUInt32Decimal to handle 0 value (#475)

* Fix stringBuilderUInt32Decimal to handle 0 value

Previously the stringBuilder method would return
and empty string when the integer value of '0' was
supplied. With this change, the string representation
"0" is returned.
---
 source/ota_mqtt.c          | 20 +++++++++----------
 test/unit-test/ota_utest.c | 40 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/source/ota_mqtt.c b/source/ota_mqtt.c
index 2c7276113..ad4e9f76e 100644
--- a/source/ota_mqtt.c
+++ b/source/ota_mqtt.c
@@ -282,6 +282,8 @@ static size_t stringBuilder( char * pBuffer,
         curLen += thisLength;
     }
 
+    pBuffer[ curLen ] = '\0';
+
     return curLen;
 }
 
@@ -295,17 +297,16 @@ static size_t stringBuilderUInt32Decimal( char * pBuffer,
     uint32_t valueCopy = value;
     size_t size = 0;
 
-    /* Assert if there is not enough buffer space. */
-
-    assert( bufferSizeBytes >= U32_MAX_LEN );
     ( void ) bufferSizeBytes;
+    /* Assert if there is not enough buffer space. */
+    assert( bufferSizeBytes >= U32_MAX_LEN + 1 );
 
-    while( valueCopy > 0U )
+    do
     {
         *pCur = asciiDigits[ ( valueCopy % 10U ) ];
         pCur++;
         valueCopy /= 10U;
-    }
+    } while( valueCopy > 0U );
 
     while( pCur > workBuf )
     {
@@ -315,7 +316,6 @@ static size_t stringBuilderUInt32Decimal( char * pBuffer,
     }
 
     pDest[ size ] = '\0';
-    size++;
     return size;
 }
 
@@ -330,10 +330,9 @@ static size_t stringBuilderUInt32Hex( char * pBuffer,
     uint32_t valueCopy = value;
     size_t i;
 
-    /* Assert if there is not enough buffer space. */
-
-    assert( bufferSizeBytes >= U32_MAX_LEN );
     ( void ) bufferSizeBytes;
+    /* Assert if there is not enough buffer space. */
+    assert( bufferSizeBytes >= U32_MAX_LEN + 1 );
 
     /* Render all 8 digits, including leading zeros. */
     for( i = 0U; i < 8U; i++ )
@@ -351,7 +350,6 @@ static size_t stringBuilderUInt32Hex( char * pBuffer,
     }
 
     pDest[ size ] = '\0';
-    size++;
     return size;
 }
 
@@ -870,6 +868,7 @@ OtaErr_t requestJob_Mqtt( const OtaAgentContext_t * pAgentCtx )
     uint32_t msgSize = 0;
     uint16_t topicLen = 0;
     uint32_t xThingNameLength = 0;
+    char reqCounterString[ U32_MAX_LEN + 1 ];
     uint32_t reqCounterStringLength = 0;
 
     /* NULL-terminated list of topic string parts. */
@@ -880,7 +879,6 @@ OtaErr_t requestJob_Mqtt( const OtaAgentContext_t * pAgentCtx )
         MQTT_API_JOBS_NEXT_GET,
         NULL
     };
-    char reqCounterString[ U32_MAX_LEN + 1 ];
 
     /* NULL-terminated list of payload parts */
 
diff --git a/test/unit-test/ota_utest.c b/test/unit-test/ota_utest.c
index 1a0df0dde..6d29907b3 100644
--- a/test/unit-test/ota_utest.c
+++ b/test/unit-test/ota_utest.c
@@ -503,6 +503,28 @@ static OtaMqttStatus_t stubMqttPublishOnlySuccedsIfTruncatedValue( const char *
     return OtaMqttSuccess;
 }
 
+static OtaMqttStatus_t stubMqttPublishZeroBlocksReceived( const char * const unused_1,
+                                                          uint16_t unused_2,
+                                                          const char * msg,
+                                                          uint32_t msgSize,
+                                                          uint8_t unused_3 )
+{
+    ( void ) unused_1;
+    ( void ) unused_2;
+    ( void ) unused_3;
+
+    /* Maximum message size is 77 characters */
+    TEST_ASSERT_LESS_OR_EQUAL( 77U, msgSize );
+
+    char actual[ 19 ] = { 0 };
+
+    memcpy( actual, msg, 18U );
+
+    TEST_ASSERT_EQUAL_STRING( "{\"status\":\"IN_PROGRESS\",\"statusDetails\":{\"receive\":\"0/2\"}}", actual );
+
+    return OtaMqttSuccess;
+}
+
 OtaErr_t mockControlInterfaceRequestJobAlwaysFail( const OtaAgentContext_t * unused )
 {
     ( void ) unused;
@@ -2646,6 +2668,24 @@ void test_OTA_ReceiveFileBlockCompleteMqttCountUpdateJobCalledTime()
     TEST_ASSERT_EQUAL( OTA_TEST_FILE_NUM_BLOCKS, otaReceivedFileBlockNumber );
 }
 
+void test_OTA_UpdateJobStatus()
+{
+    OtaErr_t err = OtaErrNone;
+
+    otaInitDefault();
+
+    /* Verify the conversion of the value 0U and 2U to a string */
+    otaInterfaces.mqtt.publish = stubMqttPublishZeroBlocksReceived;
+
+    /* Set the file size to an arbitrary value < block size to ensure 2 blocks for the OTA */
+    otaAgent.fileContext.fileSize = 100U;
+    /* With 2 blocks remaining, status message will say '0/2' blocks received */
+    otaAgent.fileContext.blocksRemaining = 2U;
+
+    err = updateJobStatus_Mqtt( &otaAgent, JobStatusInProgress, 0, 0 );
+    TEST_ASSERT_EQUAL( OtaErrNone, err );
+}
+
 void test_OTA_EventProcessingTask_ExitOnAbort()
 {
     OtaEventMsg_t otaEvent = { 0 };