Increase coverage of iot_mqtt_network.c (aws#770)

libraries/standard/mqtt/src/iot_mqtt_network.c

@@ -903,3 +903,11 @@ IotMqttError_t IotMqtt_GetIncomingMQTTPacketTypeAndLength( IotMqttPacketInfo_t *
 }
 
 /*-----------------------------------------------------------*/
+
+/* Provide access to internal functions and variables if testing. */
+/* IOT_BUILD_TESTS is defined outside the code base, e.g. passed in by build command. */
+/* coverity[misra_c_2012_rule_20_9_violation] */
+/* coverity[caretline] */
+#if IOT_BUILD_TESTS == 1
+    #include "iot_test_access_mqtt_network.c"
+#endif

libraries/standard/mqtt/test/access/iot_test_access_mqtt.h

@@ -47,6 +47,16 @@ _mqttConnection_t * IotTestMqtt_createMqttConnection( bool awsIotMqttMode,
  */
 IotMqttError_t IotTestMqtt_scheduleKeepAlive( IotMqttConnection_t pMqttConnection );
 
+/*-------------------------- iot_mqtt_network.c -------------------------*/
+
+/**
+ * @brief Test access function for #_sendPuback.
+ *
+ * @see #_sendPuback.
+ */
+void IotTestMqtt_sendPuback( _mqttConnection_t * pMqttConnection,
+                             uint16_t packetIdentifier );
+
 /*------------------------- iot_mqtt_serialize.c ------------------------*/
 
 /*

libraries/standard/mqtt/test/access/iot_test_access_mqtt_network.c

@@ -0,0 +1,43 @@
+/*
+ * IoT MQTT V2.1.0
+ * Copyright (C) 2020 Amazon.com, Inc. or its affiliates.  All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to
+ * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
+ * the Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/**
+ * @file iot_test_access_mqtt_network.c
+ * @brief Provides access to the internal functions and variables of
+ * iot_mqtt_network.c
+ *
+ * This file should only be included at the bottom of iot_mqtt_network.c
+ * and never compiled by itself.
+ */
+
+void IotTestMqtt_sendPuback( _mqttConnection_t * pMqttConnection,
+                             uint16_t packetIdentifier );
+
+/*-----------------------------------------------------------*/
+
+void IotTestMqtt_sendPuback( _mqttConnection_t * pMqttConnection,
+                             uint16_t packetIdentifier )
+{
+    _sendPuback( pMqttConnection, packetIdentifier );
+}
+
+/*-----------------------------------------------------------*/

libraries/standard/mqtt/test/unit/iot_tests_mqtt_api.c

@@ -2339,15 +2339,18 @@ TEST( MQTT_Unit_API, GetIncomingMQTTPacketTypeAndLengthChecks )
     TEST_ASSERT_EQUAL_INT( 0x02, mqttPacket.remainingLength );
 
     /* Test with NULL network interface */
+    bufPtr = buffer;
     status = IotMqtt_GetIncomingMQTTPacketTypeAndLength( &mqttPacket, _getNextByte, NULL );
     TEST_ASSERT_EQUAL( IOT_MQTT_NETWORK_ERROR, status );
 
     /* Test with incorrect packet type. */
+    bufPtr = buffer;
     buffer[ 0 ] = 0x10; /* INVALID */
     status = IotMqtt_GetIncomingMQTTPacketTypeAndLength( &mqttPacket, _getNextByte, pNetworkInterface );
     TEST_ASSERT_EQUAL( IOT_MQTT_BAD_RESPONSE, status );
 
     /* Test with invalid remaining length. */
+    bufPtr = buffer;
     buffer[ 0 ] = 0x20; /* CONN ACK */
 
     /* To generate invalid remaining length response,

libraries/standard/mqtt/test/unit/iot_tests_mqtt_platform.c

@@ -176,6 +176,22 @@ static IotNetworkError_t _networkDestroy( IotNetworkConnection_t pConnection )
 
 /*-----------------------------------------------------------*/
 
+/**
+ * @brief Serializer override for PUBACK that always fails.
+ */
+static IotMqttError_t _serializePuback( uint16_t packetIdentifier,
+                                              uint8_t ** pPubackPacket,
+                                              size_t * pPacketSize )
+{
+    ( void ) packetIdentifier;
+    ( void ) pPubackPacket;
+    ( void ) pPacketSize;
+
+    return IOT_MQTT_NO_MEMORY;
+}
+
+/*-----------------------------------------------------------*/
+
 /**
  * @brief Test group for MQTT platform tests.
  */
@@ -231,8 +247,9 @@ TEST_GROUP_RUNNER( MQTT_Unit_Platform )
 {
     RUN_TEST_CASE( MQTT_Unit_Platform, ConnectNetworkFailure );
     RUN_TEST_CASE( MQTT_Unit_Platform, ConnectScheduleFailure );
-    RUN_TEST_CASE( MQTT_Unit_Platform, DisconnectSendFailure );
+    RUN_TEST_CASE( MQTT_Unit_Platform, DisconnectNetworkFailure );
     RUN_TEST_CASE( MQTT_Unit_Platform, PublishScheduleFailure );
+    RUN_TEST_CASE( MQTT_Unit_Platform, PubackScheduleSerializeFailure );
     RUN_TEST_CASE( MQTT_Unit_Platform, SubscriptionScheduleFailure );
 }
 
@@ -309,17 +326,22 @@ TEST( MQTT_Unit_Platform, ConnectScheduleFailure )
 /**
  * @brief Tests the behavior of @ref mqtt_function_disconnect when the network fails.
  */
-TEST( MQTT_Unit_Platform, DisconnectSendFailure )
+TEST( MQTT_Unit_Platform, DisconnectNetworkFailure )
 {
     _mqttConnection_t * pMqttConnection = NULL;
 
-    /* Create a new MQTT connection. */
+    /* Call disconnect with a failing send. */
     pMqttConnection = IotTestMqtt_createMqttConnection( false, &_networkInfo, 100 );
     TEST_ASSERT_NOT_NULL( pMqttConnection );
-
-    /* Call disconnect with a failing send. */
     _sendStatus = IOT_NETWORK_FAILURE;
     IotMqtt_Disconnect( pMqttConnection, 0 );
+    _sendStatus = IOT_NETWORK_SUCCESS;
+
+    /* Call disconnect with a failing close. */
+    pMqttConnection = IotTestMqtt_createMqttConnection( false, &_networkInfo, 100 );
+    TEST_ASSERT_NOT_NULL( pMqttConnection );
+    _closeStatus = IOT_NETWORK_FAILURE;
+    IotMqtt_Disconnect( pMqttConnection, IOT_MQTT_FLAG_CLEANUP_ONLY );
 }
 
 /*-----------------------------------------------------------*/
@@ -346,7 +368,7 @@ TEST( MQTT_Unit_Platform, PublishScheduleFailure )
 
     /* Send a QoS 0 publish that fails to schedule. */
     publishInfo.pTopicName = "test/";
-    publishInfo.topicNameLength = strlen( publishInfo.pTopicName );
+    publishInfo.topicNameLength = ( uint16_t ) strlen( publishInfo.pTopicName );
     publishInfo.pPayload = "";
     publishInfo.payloadLength = 0;
 
@@ -368,6 +390,44 @@ TEST( MQTT_Unit_Platform, PublishScheduleFailure )
 
 /*-----------------------------------------------------------*/
 
+/**
+ * @brief Tests the behavior of the client-to-server PUBACK when scheduling and
+  * serializing fail.
+ */
+TEST( MQTT_Unit_Platform, PubackScheduleSerializeFailure )
+{
+    IotMqttConnection_t pMqttConnection = IOT_MQTT_CONNECTION_INITIALIZER;
+    IotMqttSerializer_t serializer = IOT_MQTT_SERIALIZER_INITIALIZER;
+    IotTaskPool_t taskPool = IOT_SYSTEM_TASKPOOL;
+    uint32_t maxThreads = 0;
+
+    /* Create a new MQTT connection. */
+    pMqttConnection = IotTestMqtt_createMqttConnection( false, &_networkInfo, 0 );
+    TEST_ASSERT_NOT_NULL( pMqttConnection );
+
+    /* Set the task pool to an invalid state and cause all further scheduling to fail. */
+    maxThreads = taskPool->maxThreads;
+    taskPool->maxThreads = 0;
+
+    /* Call the function to send a PUBACK with scheduling failure. The failed PUBACK
+     * should be cleaned up and not create memory leaks. */
+    IotTestMqtt_sendPuback( pMqttConnection, 1 );
+
+    /* Restore the task pool to a valid state. */
+    taskPool->maxThreads = maxThreads;
+
+    /* Call the function to send PUBACK with serializer failure. The failed PUBACK
+     * should be cleaned up and not create memory leaks. */
+    serializer.serialize.puback = _serializePuback;
+    pMqttConnection->pSerializer = &serializer;
+    IotTestMqtt_sendPuback( pMqttConnection, 1 );
+
+    /* Clean up. */
+    IotMqtt_Disconnect( pMqttConnection, IOT_MQTT_FLAG_CLEANUP_ONLY );
+}
+
+/*-----------------------------------------------------------*/
+
 /**
  * @brief Tests the behavior of @ref mqtt_function_subscribeasync and
  * @ref mqtt_function_unsubscribeasync when scheduling fails.
@@ -383,7 +443,7 @@ TEST( MQTT_Unit_Platform, SubscriptionScheduleFailure )
 
     /* Set subscription parameters. */
     subscription.pTopicFilter = "test/";
-    subscription.topicFilterLength = strlen( subscription.pTopicFilter );
+    subscription.topicFilterLength = ( uint16_t ) strlen( subscription.pTopicFilter );
     subscription.callback.function = SUBSCRIPTION_CALLBACK_FUNCTION;
 
     /* Create a new MQTT connection. */

libraries/standard/mqtt/test/unit/iot_tests_mqtt_receive.c

@@ -115,17 +115,17 @@ static const uint8_t _pPingrespTemplate[] = { 0xd0, 0x00 };
 /**
  * @brief Initializer for operations in the tests.
  */
-#define INITIALIZE_OPERATION( name )                                                             \
-    {                                                                                            \
-        .link = { 0 }, .incomingPublish = false, .pMqttConnection = NULL,                        \
-        .jobStorage = IOT_TASKPOOL_JOB_STORAGE_INITIALIZER, .job = IOT_TASKPOOL_JOB_INITIALIZER, \
-        .u.operation =                                                                           \
-        {                                                                                        \
-            .jobReference = 1, .type = name, .flags = IOT_MQTT_FLAG_WAITABLE,                    \
-            .packetIdentifier = 1, .pMqttPacket = NULL, .packetSize = 0,                         \
-            .notify = { .callback = { 0 } }, .status = IOT_MQTT_STATUS_PENDING,                  \
-            .periodic = { .retry = { 0 } }                                                       \
-        }                                                                                        \
+#define INITIALIZE_OPERATION( name )                                                                             \
+    {                                                                                                            \
+        .link = { 0 }, .incomingPublish = false, .pMqttConnection = NULL,                                        \
+        .jobStorage = IOT_TASKPOOL_JOB_STORAGE_INITIALIZER, .job = IOT_TASKPOOL_JOB_INITIALIZER,                 \
+        .u.operation =                                                                                           \
+        {                                                                                                        \
+            .jobReference     = 1, .type        = name,                    .flags      = IOT_MQTT_FLAG_WAITABLE, \
+            .packetIdentifier = 1, .pMqttPacket = NULL,                    .packetSize =                      0, \
+            .notify           = { .callback = { 0 } },.status      = IOT_MQTT_STATUS_PENDING,                                       \
+            .periodic         = { .retry = { 0 } }                                                               \
+        }                                                                                                        \
     }
 
 /*-----------------------------------------------------------*/
@@ -539,6 +539,45 @@ static void _disconnectCallback( void * pCallbackContext,
 
 /*-----------------------------------------------------------*/
 
+/**
+ * @brief Common code for PUBLISH malloc failure tests.
+ */
+static void _publishMallocFail( IotMqttQos_t qos )
+{
+    int32_t i = 0;
+    bool status = false;
+
+    for( i = 0; ; i++ )
+    {
+        DECLARE_PACKET( _pPublishTemplate, pPublish, publishSize );
+
+        if( qos == IOT_MQTT_QOS_1 )
+        {
+            pPublish[ 0 ] = 0x32;
+        }
+
+        UnityMalloc_MakeMallocFailAfterCount( i );
+
+        /* Attempt to process a PUBLISH. Memory allocation will fail at various
+         * times during this call. */
+        status = _processPublish( pPublish, publishSize, 1 );
+
+        /* Exit once the publish is successfully processed. */
+        if( status == true )
+        {
+            break;
+        }
+
+        /* Network close function should not have been invoked. */
+        TEST_ASSERT_EQUAL_INT( false, _networkCloseCalled );
+        TEST_ASSERT_EQUAL_INT( false, _disconnectCallbackCalled );
+    }
+
+    UnityMalloc_MakeMallocFailAfterCount( -1 );
+}
+
+/*-----------------------------------------------------------*/
+
 /**
  * @brief Test group for MQTT Receive tests.
  */
@@ -637,6 +676,7 @@ TEST_GROUP_RUNNER( MQTT_Unit_Receive )
     RUN_TEST_CASE( MQTT_Unit_Receive, ConnackInvalid );
     RUN_TEST_CASE( MQTT_Unit_Receive, PublishValid );
     RUN_TEST_CASE( MQTT_Unit_Receive, PublishInvalid );
+    RUN_TEST_CASE( MQTT_Unit_Receive, PublishResourceFailure );
     RUN_TEST_CASE( MQTT_Unit_Receive, PubackValid );
     RUN_TEST_CASE( MQTT_Unit_Receive, PubackInvalid );
     RUN_TEST_CASE( MQTT_Unit_Receive, SubackValid );
@@ -815,7 +855,8 @@ TEST( MQTT_Unit_Receive, ReceiveMallocFail )
         /* Network close function should not have been invoked. */
         TEST_ASSERT_EQUAL_INT( false, _networkCloseCalled );
         TEST_ASSERT_EQUAL_INT( false, _disconnectCallbackCalled );
-    #else
+    #else /* if ( LIBRARY_LOG_LEVEL == IOT_LOG_NONE ) */
+
         /* Test tear down for this test group checks that deserializer overrides
          * were called. Set these values to true so that the checks pass. */
         _deserializeOverrideCalled = true;
@@ -1034,8 +1075,7 @@ TEST( MQTT_Unit_Receive, PublishValid )
                                                       1 ) );
     }
 
-    /* Process a valid QoS 1 PUBLISH. Prevent an attempt to send PUBACK by
-     * making no memory available for the PUBACK. */
+    /* Process a valid QoS 1 PUBLISH. */
     {
         DECLARE_PACKET( _pPublishTemplate, pPublish, publishSize );
         pPublish[ 0 ] = 0x32;
@@ -1216,6 +1256,32 @@ TEST( MQTT_Unit_Receive, PublishInvalid )
 
 /*-----------------------------------------------------------*/
 
+/**
+ * @brief Tests the behavior of @ref mqtt_function_receivecallback with errors
+ * such as memory allocation failure and closed connections.
+ */
+TEST( MQTT_Unit_Receive, PublishResourceFailure )
+{
+    /* Test the behavior when memory allocation fails for QoS 0 and 1 PUBLISH. */
+    _publishMallocFail( IOT_MQTT_QOS_0 );
+    _publishMallocFail( IOT_MQTT_QOS_1 );
+
+    /* Test the behavior when a closed connection is used. */
+    {
+        DECLARE_PACKET( _pPublishTemplate, pPublish, publishSize );
+
+        /* Mark the connection as closed, then attempt to process a PUBLISH with a closed connection. */
+        _pMqttConnection->disconnected = true;
+        TEST_ASSERT_EQUAL_INT( true, _processPublish( pPublish, publishSize, 0 ) );
+
+        /* Network close function should not have been invoked. */
+        TEST_ASSERT_EQUAL_INT( false, _networkCloseCalled );
+        TEST_ASSERT_EQUAL_INT( false, _disconnectCallbackCalled );
+    }
+}
+
+/*-----------------------------------------------------------*/
+
 /**
  * @brief Tests the behavior of @ref mqtt_function_receivecallback with a
  * spec-compliant PUBACK.