diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index b21d5b5a976604..94233c874889da 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -202,6 +202,24 @@ ReadHandler * InteractionModelEngine::ActiveHandlerAt(unsigned int aIndex) return ret; } +WriteHandler * InteractionModelEngine::ActiveWriteHandlerAt(unsigned int aIndex) +{ + unsigned int i = 0; + + for (auto & writeHandler : mWriteHandlers) + { + if (!writeHandler.IsFree()) + { + if (i == aIndex) + { + return &writeHandler; + } + i++; + } + } + return nullptr; +} + uint32_t InteractionModelEngine::GetNumActiveWriteHandlers() const { uint32_t numActive = 0; diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 28637499085f9a..9f21baf4b4726d 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -159,6 +159,11 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, */ ReadHandler * ActiveHandlerAt(unsigned int aIndex); + /** + * Returns the write handler at a particular index within the active handler list. + */ + WriteHandler * ActiveWriteHandlerAt(unsigned int aIndex); + /** * The Magic number of this InteractionModelEngine, the magic number is set during Init() */ diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 12a2086e7c48d3..36d92ca0851b55 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -31,7 +31,7 @@ namespace chip { namespace app { using namespace Protocols::InteractionModel; - +using Status = Protocols::InteractionModel::Status; constexpr uint8_t kListAttributeType = 0x48; CHIP_ERROR WriteHandler::Init() @@ -118,7 +118,14 @@ CHIP_ERROR WriteHandler::OnMessageReceived(Messaging::ExchangeContext * apExchan "OnMessageReceived should not be called on GroupExchangeContext"); if (!aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::WriteRequest)) { + if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse)) + { + CHIP_ERROR statusError = CHIP_NO_ERROR; + //Logging purpose to print the status error code + StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError); + } ChipLogDetail(DataManagement, "Unexpected message type %d", aPayloadHeader.GetMessageType()); + StatusResponse::Send(Status::InvalidAction, apExchangeContext, false /*aExpectResponse*/); Close(); return CHIP_ERROR_INVALID_MESSAGE_TYPE; } @@ -133,7 +140,7 @@ CHIP_ERROR WriteHandler::OnMessageReceived(Messaging::ExchangeContext * apExchan Close(); } } - else if (status != Protocols::InteractionModel::Status::Success) + else { err = StatusResponse::Send(status, apExchangeContext, false /*aExpectResponse*/); Close(); @@ -312,7 +319,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData // it with Busy status code. (dataAttributePath.IsListItemOperation() && !IsSameAttribute(mProcessingAttributePath, dataAttributePath))) { - err = AddStatus(dataAttributePath, StatusIB(Protocols::InteractionModel::Status::Busy)); + err = AddStatus(dataAttributePath, StatusIB(Status::Busy)); continue; } @@ -619,13 +626,11 @@ CHIP_ERROR WriteHandler::AddStatus(const ConcreteDataAttributePath & aPath, cons CHIP_ERROR WriteHandler::AddClusterSpecificSuccess(const ConcreteDataAttributePath & aPath, ClusterStatus aClusterStatus) { - using Protocols::InteractionModel::Status; return AddStatus(aPath, StatusIB(Status::Success, aClusterStatus)); } CHIP_ERROR WriteHandler::AddClusterSpecificFailure(const ConcreteDataAttributePath & aPath, ClusterStatus aClusterStatus) { - using Protocols::InteractionModel::Status; return AddStatus(aPath, StatusIB(Status::Failure, aClusterStatus)); } diff --git a/src/app/WriteHandler.h b/src/app/WriteHandler.h index 43163a2b8abab9..7a27266d4722d3 100644 --- a/src/app/WriteHandler.h +++ b/src/app/WriteHandler.h @@ -114,6 +114,7 @@ class WriteHandler : public Messaging::ExchangeDelegate } private: + friend class TestWriteInteraction; enum class State { Uninitialized = 0, // The handler has not been initialized @@ -121,9 +122,10 @@ class WriteHandler : public Messaging::ExchangeDelegate AddStatus, // The handler has added status code Sending, // The handler has sent out the write response }; - Protocols::InteractionModel::Status ProcessWriteRequest(System::PacketBufferHandle && aPayload, bool aIsTimedWrite); - Protocols::InteractionModel::Status HandleWriteRequestMessage(Messaging::ExchangeContext * apExchangeContext, - System::PacketBufferHandle && aPayload, bool aIsTimedWrite); + using Status = Protocols::InteractionModel::Status; + Status ProcessWriteRequest(System::PacketBufferHandle && aPayload, bool aIsTimedWrite); + Status HandleWriteRequestMessage(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload, + bool aIsTimedWrite); CHIP_ERROR FinalizeMessage(System::PacketBufferTLVWriter && aMessageWriter, System::PacketBufferHandle & packet); CHIP_ERROR SendWriteResponse(System::PacketBufferTLVWriter && aMessageWriter); diff --git a/src/app/tests/TestWriteInteraction.cpp b/src/app/tests/TestWriteInteraction.cpp index f1af80cef330b7..711577059ab8cb 100644 --- a/src/app/tests/TestWriteInteraction.cpp +++ b/src/app/tests/TestWriteInteraction.cpp @@ -32,7 +32,9 @@ #include #include +#include #include +#include using TestContext = chip::Test::AppContext; @@ -61,6 +63,7 @@ class TestWriteInteraction static void TestWriteRoundtripWithClusterObjects(nlTestSuite * apSuite, void * apContext); static void TestWriteRoundtripWithClusterObjectsVersionMatch(nlTestSuite * apSuite, void * apContext); static void TestWriteRoundtripWithClusterObjectsVersionMismatch(nlTestSuite * apSuite, void * apContext); + static void TestWriteHandlerReceiveInvalidMessage(nlTestSuite * apSuite, void * apContext); private: static void AddAttributeDataIB(nlTestSuite * apSuite, void * apContext, WriteClient & aWriteClient); @@ -90,13 +93,18 @@ class TestWriteClientCallback : public chip::app::WriteClient::Callback mStatus = status; mOnSuccessCalled++; } - void OnError(const WriteClient * apWriteClient, CHIP_ERROR chipError) override { mOnErrorCalled++; } + void OnError(const WriteClient * apWriteClient, CHIP_ERROR chipError) override + { + mOnErrorCalled++; + mLastErrorReason = app::StatusIB(chipError); + } void OnDone(WriteClient * apWriteClient) override { mOnDoneCalled++; } int mOnSuccessCalled = 0; int mOnErrorCalled = 0; int mOnDoneCalled = 0; StatusIB mStatus; + StatusIB mLastErrorReason; }; void TestWriteInteraction::AddAttributeDataIB(nlTestSuite * apSuite, void * apContext, WriteClient & aWriteClient) @@ -543,6 +551,74 @@ void TestWriteInteraction::TestWriteRoundtrip(nlTestSuite * apSuite, void * apCo engine->Shutdown(); } +// This test creates a chunked write request, we drop the second write chunk message, then write handler receives unknown +// report message and sends out a status report with invalid action. +void TestWriteInteraction::TestWriteHandlerReceiveInvalidMessage(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + + app::AttributePathParams attributePath(2, 3, 4); + + CHIP_ERROR err = CHIP_NO_ERROR; + Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + // Shouldn't have anything in the retransmit table when starting the test. + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + + TestWriteClientCallback writeCallback; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + app::WriteClient writeClient(&ctx.GetExchangeManager(), &writeCallback, Optional::Missing(), + static_cast(900) /* reserved buffer size */); + + ByteSpan list[5]; + + err = writeClient.EncodeAttribute(attributePath, app::DataModel::List(list, 5)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.GetLoopback().mSentMessageCount = 0; + ctx.GetLoopback().mNumMessagesToDrop = 1; + ctx.GetLoopback().mNumMessagesToAllowBeforeDropping = 2; + err = writeClient.SendWriteRequest(sessionHandle); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, InteractionModelEngine::GetInstance()->GetNumActiveWriteHandlers() == 1); + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 3); + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mDroppedMessageCount == 1); + + System::PacketBufferHandle msgBuf = System::PacketBufferHandle::New(kMaxSecureSduLengthBytes); + NL_TEST_ASSERT(apSuite, !msgBuf.IsNull()); + System::PacketBufferTLVWriter writer; + writer.Init(std::move(msgBuf)); + + ReportDataMessage::Builder response; + response.Init(&writer); + NL_TEST_ASSERT(apSuite, writer.Finalize(&msgBuf) == CHIP_NO_ERROR); + + PayloadHeader payloadHeader; + payloadHeader.SetExchangeID(0); + payloadHeader.SetMessageType(chip::Protocols::InteractionModel::MsgType::ReportData); + + auto * writeHandler = InteractionModelEngine::GetInstance()->ActiveWriteHandlerAt(0); + rm->ClearRetransTable(writeClient.mExchangeCtx.Get()); + rm->ClearRetransTable(writeHandler->mExchangeCtx.Get()); + ctx.GetLoopback().mSentMessageCount = 0; + ctx.GetLoopback().mNumMessagesToDrop = 0; + writeHandler->OnMessageReceived(writeHandler->mExchangeCtx.Get(), payloadHeader, std::move(msgBuf)); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, writeCallback.mLastErrorReason.mStatus == Protocols::InteractionModel::Status::InvalidAction); + NL_TEST_ASSERT(apSuite, InteractionModelEngine::GetInstance()->GetNumActiveWriteHandlers() == 0); + engine->Shutdown(); + ctx.ExpireSessionAliceToBob(); + ctx.ExpireSessionBobToAlice(); + ctx.CreateSessionAliceToBob(); + ctx.CreateSessionBobToAlice(); +} + } // namespace app } // namespace chip @@ -562,6 +638,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestWriteRoundtripWithClusterObjects", chip::app::TestWriteInteraction::TestWriteRoundtripWithClusterObjects), NL_TEST_DEF("TestWriteRoundtripWithClusterObjectsVersionMatch", chip::app::TestWriteInteraction::TestWriteRoundtripWithClusterObjectsVersionMatch), NL_TEST_DEF("TestWriteRoundtripWithClusterObjectsVersionMismatch", chip::app::TestWriteInteraction::TestWriteRoundtripWithClusterObjectsVersionMismatch), + NL_TEST_DEF("TestWriteHandlerReceiveInvalidMessage", chip::app::TestWriteInteraction::TestWriteHandlerReceiveInvalidMessage), NL_TEST_SENTINEL() }; // clang-format on