diff --git a/source/core_mqtt.c b/source/core_mqtt.c index 5d0ca7f1d..5fd5f1ff1 100644 --- a/source/core_mqtt.c +++ b/source/core_mqtt.c @@ -767,7 +767,7 @@ static int32_t sendMessageVector( MQTTContext_t * pContext, size_t ioVecCount ) { int32_t sendResult; - uint32_t timeoutMs; + uint32_t startTime; TransportOutVector_t * pIoVectIterator; size_t vectorsToBeSent = ioVecCount; size_t bytesToSend = 0U; @@ -788,8 +788,8 @@ static int32_t sendMessageVector( MQTTContext_t * pContext, /* Reset the iterator to point to the first entry in the array. */ pIoVectIterator = pIoVec; - /* Set the timeout. */ - timeoutMs = pContext->getTime() + MQTT_SEND_TIMEOUT_MS; + /* Note the start time. */ + startTime = pContext->getTime(); while( ( bytesSentOrError < ( int32_t ) bytesToSend ) && ( bytesSentOrError >= 0 ) ) { @@ -832,7 +832,7 @@ static int32_t sendMessageVector( MQTTContext_t * pContext, } /* Check for timeout. */ - if( pContext->getTime() >= timeoutMs ) + if( calculateElapsedTime( pContext->getTime(), startTime ) > MQTT_SEND_TIMEOUT_MS ) { LogError( ( "sendMessageVector: Unable to send packet: Timed out." ) ); break; @@ -1701,7 +1701,7 @@ static MQTTStatus_t receiveSingleIteration( MQTTContext_t * pContext, pContext->index += ( size_t ) recvBytes; status = MQTT_ProcessIncomingPacketTypeAndLength( pContext->networkBuffer.pBuffer, - &pContext->index, + &( pContext->index ), &incomingPacket ); totalMQTTPacketLength = incomingPacket.remainingLength + incomingPacket.headerLength; diff --git a/test/unit-test/core_mqtt_config.h b/test/unit-test/core_mqtt_config.h index 6c659fa64..ba1d9a02f 100644 --- a/test/unit-test/core_mqtt_config.h +++ b/test/unit-test/core_mqtt_config.h @@ -77,4 +77,6 @@ struct NetworkContext #define MQTT_SUB_UNSUB_MAX_VECTORS ( 6U ) +#define MQTT_SEND_TIMEOUT_MS ( 20U ) + #endif /* ifndef CORE_MQTT_CONFIG_H_ */ diff --git a/test/unit-test/core_mqtt_utest.c b/test/unit-test/core_mqtt_utest.c index dc8b0441b..762feefe7 100644 --- a/test/unit-test/core_mqtt_utest.c +++ b/test/unit-test/core_mqtt_utest.c @@ -320,6 +320,37 @@ static uint32_t getTime( void ) return globalEntryTime++; } +static int32_t getTimeMockCallLimit = -1; + +/** + * @brief A mocked timer query function that increments on every call. This + * guarantees that only a single iteration runs in the ProcessLoop for ease + * of testing. Additionally, this tests whether the number of calls to this + * function have exceeded than the set limit and asserts. + */ +static uint32_t getTimeMock( void ) +{ + TEST_ASSERT_GREATER_THAN_INT32( -1, getTimeMockCallLimit-- ); + return globalEntryTime++; +} + +static int32_t getTimeMockBigTimeStepCallLimit = -1; + +/** + * @brief A mocked timer query function that increments by MQTT_SEND_TIMEOUT_MS + * to simulate the time consumed by a long running high priority task on every + * call. Additionally, this tests whether the number of calls to this function + * have exceeded than the set limit and asserts. + */ +static uint32_t getTimeMockBigTimeStep( void ) +{ + TEST_ASSERT_GREATER_THAN_INT32( -1, getTimeMockBigTimeStepCallLimit-- ); + + globalEntryTime += MQTT_SEND_TIMEOUT_MS; + return globalEntryTime; +} + + /** * @brief A mocked timer function that could be used on a device with no system * time. @@ -4755,6 +4786,92 @@ void test_MQTT_Subscribe_error_paths2( void ) TEST_ASSERT_EQUAL( MQTTSendFailed, mqttStatus ); } +/** + * @brief This test case verifies that MQTT_Subscribe returns MQTTSendFailed + * if transport interface send fails and timer overflows. + */ +void test_MQTT_Subscribe_error_paths_timerOverflowCheck( void ) +{ + MQTTStatus_t mqttStatus; + MQTTContext_t context = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + MQTTSubscribeInfo_t subscribeInfo = { 0 }; + size_t remainingLength = MQTT_SAMPLE_REMAINING_LENGTH; + size_t packetSize = MQTT_SAMPLE_REMAINING_LENGTH; + + globalEntryTime = UINT32_MAX - 2U; + + /* The timer function can be called a maximum of these many times + * (which is way less than UINT32_MAX). This ensures that if overflow + * check is not correct, then the timer mock call will fail and assert. */ + getTimeMockCallLimit = MQTT_SEND_TIMEOUT_MS + 1; + + /* Verify that an error is propagated when transport interface returns an error. */ + setupNetworkBuffer( &networkBuffer ); + setupSubscriptionInfo( &subscribeInfo ); + subscribeInfo.qos = MQTTQoS0; + setupTransportInterface( &transport ); + transport.writev = NULL; + /* Case when there is timeout in sending data through transport send. */ + transport.send = transportSendNoBytes; /* Use the mock function that returns zero bytes sent. */ + + /* Initialize context. */ + mqttStatus = MQTT_Init( &context, &transport, getTimeMock, eventCallback, &networkBuffer ); + TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + MQTT_SerializeSubscribeHeader_Stub( MQTT_SerializeSubscribedHeader_cb ); + mqttStatus = MQTT_Subscribe( &context, &subscribeInfo, 1, MQTT_FIRST_VALID_PACKET_ID ); + TEST_ASSERT_EQUAL( MQTTSendFailed, mqttStatus ); + TEST_ASSERT_EQUAL( -1, getTimeMockCallLimit ); +} + +/** + * @brief This test case verifies that MQTT_Subscribe returns MQTTSendFailed + * if transport interface send fails and timer overflows. + */ +void test_MQTT_Subscribe_error_paths_timerOverflowCheck1( void ) +{ + MQTTStatus_t mqttStatus; + MQTTContext_t context = { 0 }; + TransportInterface_t transport = { 0 }; + MQTTFixedBuffer_t networkBuffer = { 0 }; + MQTTSubscribeInfo_t subscribeInfo = { 0 }; + size_t remainingLength = MQTT_SAMPLE_REMAINING_LENGTH; + size_t packetSize = MQTT_SAMPLE_REMAINING_LENGTH; + + globalEntryTime = UINT32_MAX - MQTT_SEND_TIMEOUT_MS + 1; + + /* The timer function can be called a exactly 2 times. First when setting + * the initial time, next time when checking for timeout. + */ + getTimeMockBigTimeStepCallLimit = 2; + + /* Verify that an error is propagated when transport interface returns an error. */ + setupNetworkBuffer( &networkBuffer ); + setupSubscriptionInfo( &subscribeInfo ); + subscribeInfo.qos = MQTTQoS0; + setupTransportInterface( &transport ); + transport.writev = NULL; + /* Case when there is timeout in sending data through transport send. */ + transport.send = transportSendNoBytes; /* Use the mock function that returns zero bytes sent. */ + + /* Initialize context. */ + mqttStatus = MQTT_Init( &context, &transport, getTimeMockBigTimeStep, eventCallback, &networkBuffer ); + TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus ); + + MQTT_GetSubscribePacketSize_ExpectAnyArgsAndReturn( MQTTSuccess ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pPacketSize( &packetSize ); + MQTT_GetSubscribePacketSize_ReturnThruPtr_pRemainingLength( &remainingLength ); + MQTT_SerializeSubscribeHeader_Stub( MQTT_SerializeSubscribedHeader_cb ); + mqttStatus = MQTT_Subscribe( &context, &subscribeInfo, 1, MQTT_FIRST_VALID_PACKET_ID ); + TEST_ASSERT_EQUAL( MQTTSendFailed, mqttStatus ); + TEST_ASSERT_EQUAL( -1, getTimeMockBigTimeStepCallLimit ); +} + /* ========================================================================== */ /**