From 1ada682e0a6eeb60c5dc2b4553a1c93106cb1b6d Mon Sep 17 00:00:00 2001 From: Kamil Kasperczyk <66371704+kkasperczyk-no@users.noreply.github.com> Date: Wed, 15 Nov 2023 17:38:04 +0100 Subject: [PATCH] Fix handling command handler status for group messages (#30484) The command handler status is added for the response purposes even in case of invoking commands targeted to the group. In case of using more than one endpoint on a single Matter node that was added to the same group, it leads to the application crash. The direct reason is a state check, that succeeds only for the first endpoint and fails for the subsequent calls. Added a new state flag, which informs if the command that is currently handled origins from groupcast communication or not. In case of targeting the group, application returns prematurely and do not try to add status. Fixes: #30472 --- src/app/CommandHandler.cpp | 6 +++++- src/app/CommandHandler.h | 13 ++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index a447597cc97cea..8fd13bab7247f0 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -251,6 +251,8 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem ConcreteCommandPath concretePath(0, 0, 0); TLV::TLVReader commandDataReader; + SetGroupRequest(false); + // NOTE: errors may occur before the concrete command path is even fully decoded. err = aCommandElement.GetPath(&commandPath); @@ -352,6 +354,7 @@ Status CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aComman Credentials::GroupDataProvider::GroupEndpoint mapping; Credentials::GroupDataProvider * groupDataProvider = Credentials::GetGroupDataProvider(); Credentials::GroupDataProvider::EndpointIterator * iterator; + SetGroupRequest(true); err = aCommandElement.GetPath(&commandPath); VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction); @@ -462,7 +465,8 @@ CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aComman void CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus, const char * context) { - + // Return prematurely in case of requests targeted to a group that should not add the status for response purposes. + VerifyOrReturn(!IsGroupRequest()); VerifyOrDie(FallibleAddStatus(aCommandPath, aStatus, context) == CHIP_NO_ERROR); } diff --git a/src/app/CommandHandler.h b/src/app/CommandHandler.h index 3c9a86c97b6052..65542ef0587f55 100644 --- a/src/app/CommandHandler.h +++ b/src/app/CommandHandler.h @@ -406,6 +406,16 @@ class CommandHandler : public Messaging::ExchangeDelegate return FinishCommand(/* aEndDataStruct = */ false); } + /** + * Check whether the InvokeRequest we are handling is targeted to a group. + */ + bool IsGroupRequest() { return mGroupRequest; } + + /** + * Sets the state flag to keep the information that request we are handling is targeted to a group. + */ + void SetGroupRequest(bool isGroupRequest) { mGroupRequest = isGroupRequest; } + Messaging::ExchangeHolder mExchangeCtx; Callback * mpCallback = nullptr; InvokeResponseMessage::Builder mInvokeResponseBuilder; @@ -416,7 +426,8 @@ class CommandHandler : public Messaging::ExchangeDelegate bool mSentStatusResponse = false; - State mState = State::Idle; + State mState = State::Idle; + bool mGroupRequest = false; chip::System::PacketBufferTLVWriter mCommandMessageWriter; TLV::TLVWriter mBackupWriter; bool mBufferAllocated = false;