From 28146479bd4cb7b897ac5c528061e5bb2a515f74 Mon Sep 17 00:00:00 2001 From: Song GUO Date: Fri, 18 Mar 2022 22:03:18 +0800 Subject: [PATCH] [IM] Reject multiple commands in a same invoke transaction on the server side (#16338) --- src/app/CommandHandler.cpp | 10 ++++ src/app/tests/TestCommandInteraction.cpp | 59 ++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 0532f0852f6b0b..ebbb4dd4607f99 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -125,6 +126,15 @@ CHIP_ERROR CommandHandler::ProcessInvokeRequest(System::PacketBufferHandle && pa } invokeRequests.GetReader(&invokeRequestsReader); + + { + // We don't support handling multiple commands but the protocol is ready to support it in the future, reject all of them and + // IM Engine will send a status response. + size_t commandCount = 0; + TLV::Utilities::Count(invokeRequestsReader, commandCount, false /* recurse */); + VerifyOrReturnError(commandCount == 1, CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE); + } + while (CHIP_NO_ERROR == (err = invokeRequestsReader.Next())) { VerifyOrReturnError(TLV::AnonymousTag() == invokeRequestsReader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG); diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index df42d3fdb8c70d..5aeb21815c51b8 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -182,6 +182,7 @@ class TestCommandInteraction static void TestCommandHandlerWithSendEmptyResponse(nlTestSuite * apSuite, void * apContext); static void TestCommandHandlerWithProcessReceivedMsg(nlTestSuite * apSuite, void * apContext); static void TestCommandHandlerWithProcessReceivedEmptyDataMsg(nlTestSuite * apSuite, void * apContext); + static void TestCommandHandlerRejectMultipleCommands(nlTestSuite * apSuite, void * apContext); static void TestCommandSenderCommandSuccessResponseFlow(nlTestSuite * apSuite, void * apContext); static void TestCommandSenderCommandAsyncSuccessResponseFlow(nlTestSuite * apSuite, void * apContext); @@ -754,6 +755,63 @@ void TestCommandInteraction::TestCommandSenderAbruptDestruction(nlTestSuite * ap NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0); } +void TestCommandInteraction::TestCommandHandlerRejectMultipleCommands(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + + isCommandDispatched = false; + mockCommandSenderDelegate.ResetCounter(); + app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); + + { + // Command ID is not important here, since the command handler should reject the commands without handling it. + auto commandPathParams = MakeTestCommandPath(kTestCommandIdCommandSpecificResponse); + + commandSender.AllocateBuffer(); + + // CommandSender does not support sending multiple commands with public API, so we craft a message manaully. + for (int i = 0; i < 2; i++) + { + InvokeRequests::Builder & invokeRequests = commandSender.mInvokeRequestBuilder.GetInvokeRequests(); + CommandDataIB::Builder & invokeRequest = invokeRequests.CreateCommandData(); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequests.GetError()); + CommandPathIB::Builder & path = invokeRequest.CreatePath(); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetError()); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == path.Encode(commandPathParams)); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == + invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)), + TLV::kTLVType_Structure, + commandSender.mDataElementContainerType)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == invokeRequest.GetWriter()->EndContainer(commandSender.mDataElementContainerType)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.EndOfCommandDataIB().GetError()); + } + + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == commandSender.mInvokeRequestBuilder.GetInvokeRequests().EndOfInvokeRequests().GetError()); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.mInvokeRequestBuilder.EndOfInvokeRequestMessage().GetError()); + + commandSender.MoveToState(app::CommandSender::State::AddedCommand); + } + + err = commandSender.SendCommandRequest(ctx.GetSessionBobToAlice()); + + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, + mockCommandSenderDelegate.onResponseCalledTimes == 0 && mockCommandSenderDelegate.onFinalCalledTimes == 1 && + mockCommandSenderDelegate.onErrorCalledTimes == 1); + NL_TEST_ASSERT(apSuite, !chip::isCommandDispatched); + + NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + } // namespace app } // namespace chip @@ -772,6 +830,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestCommandHandlerWithProcessReceivedMsg", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedMsg), NL_TEST_DEF("TestCommandHandlerWithProcessReceivedNotExistCommand", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedNotExistCommand), NL_TEST_DEF("TestCommandHandlerWithProcessReceivedEmptyDataMsg", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedEmptyDataMsg), + NL_TEST_DEF("TestCommandHandlerRejectMultipleCommands", chip::app::TestCommandInteraction::TestCommandHandlerRejectMultipleCommands), NL_TEST_DEF("TestCommandSenderCommandSuccessResponseFlow", chip::app::TestCommandInteraction::TestCommandSenderCommandSuccessResponseFlow), NL_TEST_DEF("TestCommandSenderCommandAsyncSuccessResponseFlow", chip::app::TestCommandInteraction::TestCommandSenderCommandAsyncSuccessResponseFlow),