From 92efde82ddfa9f7f0f8853452e38092d4ff7db4d Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 23 Feb 2023 11:02:13 -0500 Subject: [PATCH] Handle an (invalid) empty InvokeResponses list. (#25197) Without this change, InvokeInteraction could end up not calling either of its callbacks if the server responded with an empty InvokeResponses list. --- src/controller/InvokeInteraction.h | 8 +++++--- src/controller/TypedCommandCallback.h | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/controller/InvokeInteraction.h b/src/controller/InvokeInteraction.h index 2e8816553bb8b3..d167cde140db23 100644 --- a/src/controller/InvokeInteraction.h +++ b/src/controller/InvokeInteraction.h @@ -51,6 +51,9 @@ InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, const SessionHan const Optional & timedInvokeTimeoutMs, const Optional & responseTimeout = NullOptional) { + // InvokeCommandRequest expects responses, so cannot happen over a group session. + VerifyOrReturnError(!sessionHandle->IsGroupSession(), CHIP_ERROR_INVALID_ARGUMENT); + app::CommandPathParams commandPath = { endpointId, 0, RequestObjectT::GetClusterId(), RequestObjectT::GetCommandId(), (app::CommandPathFlags::kEndpointIdValid) }; @@ -91,14 +94,13 @@ InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, const SessionHan } /* - * A typed group command invocation function that takes as input a cluster-object representation of a command request and - * callbacks when completed trought the done callback + * A typed group command invocation function that takes as input a cluster-object representation of a command request. * * The RequestObjectT is generally expected to be a ClusterName::Commands::CommandName::Type struct, but any object * that can be encoded using the DataModel::Encode machinery and exposes the GetClusterId() and GetCommandId() functions * and a ResponseType type is expected to work. * - * Since this sends a group command, no response will be received and all allocated rescources will be cleared before exing this + * Since this sends a group command, no response will be received and all allocated resources will be cleared before exiting this * function */ template diff --git a/src/controller/TypedCommandCallback.h b/src/controller/TypedCommandCallback.h index c8ab4108115299..066e403a84ff19 100644 --- a/src/controller/TypedCommandCallback.h +++ b/src/controller/TypedCommandCallback.h @@ -72,7 +72,20 @@ class TypedCommandCallback final : public app::CommandSender::Callback mOnError(aError); } - void OnDone(app::CommandSender * apCommandSender) override { mOnDone(apCommandSender); } + void OnDone(app::CommandSender * apCommandSender) override + { + if (!mCalledCallback) + { + // This can happen if the server sends a response with an empty + // InvokeResponses list. Since we are not sending wildcard command + // paths, that's not a valid response and we should treat it as an + // error. Use the error we would have gotten if we in fact expected + // a nonempty list. + OnError(apCommandSender, CHIP_END_OF_TLV); + } + + mOnDone(apCommandSender); + } OnSuccessCallbackType mOnSuccess; OnErrorCallbackType mOnError;