diff --git a/source/core_mqtt_agent.c b/source/core_mqtt_agent.c index 13262cf..b13bb44 100644 --- a/source/core_mqtt_agent.c +++ b/source/core_mqtt_agent.c @@ -604,16 +604,40 @@ static void handleAcks( const MQTTAgentContext_t * pAgentContext, uint8_t packetType ) { uint8_t * pSubackCodes = NULL; + MQTTStatus_t status = pDeserializedInfo->deserializationResult; assert( pAckInfo != NULL ); assert( pAckInfo->pOriginalCommand != NULL ); - /* A SUBACK's status codes start 2 bytes after the variable header. */ - pSubackCodes = ( packetType == MQTT_PACKET_TYPE_SUBACK ) ? ( &pPacketInfo->pRemainingData[ 2U ] ) : NULL; + /* MQTT 5 places a variable-length properties section after the packet ID. + * Use the parser's reason-code view rather than the MQTT 3 fixed offset, + * which would expose the property length as the first grant code. */ + if( packetType == MQTT_PACKET_TYPE_SUBACK ) + { + const MQTTAgentSubscribeArgs_t * pArgs = + ( const MQTTAgentSubscribeArgs_t * ) pAckInfo->pOriginalCommand->pArgs; + + if( ( pAckInfo->pOriginalCommand->commandType != SUBSCRIBE ) || + ( pDeserializedInfo->pReasonCode == NULL ) || + ( pDeserializedInfo->pReasonCode->reasonCode == NULL ) || + ( pDeserializedInfo->pReasonCode->reasonCodeLength != pArgs->numSubscriptions ) ) + { + /* Callbacks read one code per requested filter. Never hand them a + * short or malformed array, even if the packet itself parsed. */ + status = MQTTBadResponse; + } + else + { + /* Preserve the callback's mutable pointer type without casting + * away const from the parser's view of the same packet buffer. */ + pSubackCodes = &pPacketInfo->pRemainingData[ + pDeserializedInfo->pReasonCode->reasonCode - pPacketInfo->pRemainingData ]; + } + } concludeCommand( pAgentContext, pAckInfo->pOriginalCommand, - pDeserializedInfo->deserializationResult, + status, pSubackCodes ); /* Clear the entry from the list. */ diff --git a/test/unit-test/mqtt_agent_utest.c b/test/unit-test/mqtt_agent_utest.c index f7d4543..353e5d2 100644 --- a/test/unit-test/mqtt_agent_utest.c +++ b/test/unit-test/mqtt_agent_utest.c @@ -50,6 +50,7 @@ struct MQTTAgentMessageContext struct MQTTAgentCommandContext { MQTTStatus_t returnStatus; + uint8_t * pSubackCodes; }; /** @@ -246,6 +247,7 @@ static void stubCompletionCallback( MQTTAgentCommandContext_t * pCommandCompleti if( pCommandCompletionContext != NULL ) { pCommandCompletionContext->returnStatus = pReturnInfo->returnCode; + pCommandCompletionContext->pSubackCodes = pReturnInfo->pSubackCodes; } commandCompleteCallbackCount++; @@ -387,6 +389,141 @@ static void setupAgentContext( MQTTAgentContext_t * pAgentContext ) pAgentContext->mqttContext.nextPacketId = 1U; } +/** + * @brief Exercise SUBACK completion with a parser-provided reason-code view. + */ +static void checkSubackCompletion( uint8_t * pRemainingData, + MQTTReasonCodeInfo_t * pReasonCodes, + size_t subscriptionCount, + MQTTAgentCommandType_t commandType, + MQTTStatus_t expectedStatus ) +{ + MQTTAgentContext_t agentContext; + MQTTAgentCommand_t command = { 0 }; + MQTTAgentSubscribeArgs_t subscribeArgs = { 0 }; + MQTTAgentCommandContext_t completion = { 0 }; + MQTTPacketInfo_t packetInfo = { 0 }; + MQTTDeserializedInfo_t deserializedInfo = { 0 }; + + setupAgentContext( &agentContext ); + subscribeArgs.numSubscriptions = subscriptionCount; + command.commandType = commandType; + command.pArgs = &subscribeArgs; + command.pCmdContext = &completion; + command.pCommandCompleteCallback = stubCompletionCallback; + agentContext.pPendingAcks[ 0 ].packetId = 1U; + agentContext.pPendingAcks[ 0 ].pOriginalCommand = &command; + packetInfo.type = MQTT_PACKET_TYPE_SUBACK; + packetInfo.pRemainingData = pRemainingData; + deserializedInfo.packetIdentifier = 1U; + deserializedInfo.deserializationResult = MQTTSuccess; + deserializedInfo.pReasonCode = pReasonCodes; + + TEST_ASSERT_TRUE( agentContext.mqttContext.appCallback( &agentContext.mqttContext, + &packetInfo, + &deserializedInfo, + NULL, NULL, NULL ) ); + TEST_ASSERT_EQUAL( expectedStatus, completion.returnStatus ); + + if( expectedStatus == MQTTSuccess ) + { + TEST_ASSERT_EQUAL_PTR( pReasonCodes->reasonCode, completion.pSubackCodes ); + } + else + { + TEST_ASSERT_NULL( completion.pSubackCodes ); + } + + TEST_ASSERT_EQUAL( 1U, commandCompleteCallbackCount ); + TEST_ASSERT_EQUAL( 1U, commandReleaseCallCount ); + TEST_ASSERT_EQUAL( 0U, agentContext.pPendingAcks[ 0 ].packetId ); + TEST_ASSERT_NULL( agentContext.pPendingAcks[ 0 ].pOriginalCommand ); +} + +/** + * @brief A single-filter refusal follows the MQTT 5 property-length byte. + */ +void test_MQTTAgent_Suback_single_refusal( void ) +{ + uint8_t remainingData[] = { 0U, 1U, 0U, 0x87U }; + MQTTReasonCodeInfo_t reasonCodes = { &remainingData[ 3 ], 1U }; + + checkSubackCompletion( remainingData, &reasonCodes, 1U, SUBSCRIBE, MQTTSuccess ); +} + +/** + * @brief A batch must expose the last filter's refusal as well as the first grant. + */ +void test_MQTTAgent_Suback_mixed_grants( void ) +{ + uint8_t remainingData[] = { 0U, 1U, 0U, 1U, 0x87U }; + MQTTReasonCodeInfo_t reasonCodes = { &remainingData[ 3 ], 2U }; + + checkSubackCompletion( remainingData, &reasonCodes, 2U, SUBSCRIBE, MQTTSuccess ); +} + +/** + * @brief Reason String properties must not be exposed as grant codes. + */ +void test_MQTTAgent_Suback_with_properties( void ) +{ + uint8_t remainingData[] = { 0U, 1U, 5U, 0x1FU, 0U, 2U, 'n', 'o', 0x87U }; + MQTTReasonCodeInfo_t reasonCodes = { &remainingData[ 8 ], 1U }; + + checkSubackCompletion( remainingData, &reasonCodes, 1U, SUBSCRIBE, MQTTSuccess ); +} + +/** + * @brief Reject a short array before callbacks read one code per filter. + */ +void test_MQTTAgent_Suback_short_reason_array( void ) +{ + uint8_t remainingData[] = { 0U, 1U, 0U, 1U }; + MQTTReasonCodeInfo_t reasonCodes = { &remainingData[ 3 ], 1U }; + + checkSubackCompletion( remainingData, &reasonCodes, 2U, SUBSCRIBE, MQTTBadResponse ); +} + +/** + * @brief Reject an array with more codes than requested filters. + */ +void test_MQTTAgent_Suback_long_reason_array( void ) +{ + uint8_t remainingData[] = { 0U, 1U, 0U, 1U, 0U }; + MQTTReasonCodeInfo_t reasonCodes = { &remainingData[ 3 ], 2U }; + + checkSubackCompletion( remainingData, &reasonCodes, 1U, SUBSCRIBE, MQTTBadResponse ); +} + +/** + * @brief Missing parser metadata must not be reported as a successful SUBACK. + */ +void test_MQTTAgent_Suback_missing_reason_info( void ) +{ + checkSubackCompletion( NULL, NULL, 1U, SUBSCRIBE, MQTTBadResponse ); +} + +/** + * @brief Missing reason-code storage must not be handed to the callback. + */ +void test_MQTTAgent_Suback_null_reason_array( void ) +{ + MQTTReasonCodeInfo_t reasonCodes = { NULL, 1U }; + + checkSubackCompletion( NULL, &reasonCodes, 1U, SUBSCRIBE, MQTTBadResponse ); +} + +/** + * @brief A SUBACK must match a SUBSCRIBE before interpreting its arguments. + */ +void test_MQTTAgent_Suback_wrong_command_type( void ) +{ + uint8_t remainingData[] = { 0U, 1U, 0U, 1U }; + MQTTReasonCodeInfo_t reasonCodes = { &remainingData[ 3 ], 1U }; + + checkSubackCompletion( remainingData, &reasonCodes, 1U, PUBLISH, MQTTBadResponse ); +} + /** * @brief Helper function to test API functions of the form * MQTTStatus_t func( MQTTAgentContext_t *, MQTTAgentCommandInfo_t * )