From 4a8e698e132923b71c33b59daf81f6ebebfd772d Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Mon, 8 Aug 2022 16:03:35 -0700 Subject: [PATCH] Fix status report handling in read handler (#21374) * Fix status report handling in read handler * address comments * Update src/app/tests/TestReadInteraction.cpp Co-authored-by: Boris Zbarsky * Update src/app/tests/TestReadInteraction.cpp Co-authored-by: Boris Zbarsky * Update src/app/tests/TestReadInteraction.cpp Co-authored-by: Boris Zbarsky * Update src/app/tests/TestReadInteraction.cpp Co-authored-by: Boris Zbarsky * Fix 1. Once message is passed to OnReadInitialRequest in read handler, if anything bad in incoming message, handler would send status report with invalid action, if there is not enough resource for paths in IM engines, we would send status report with exhansted path. 2. Fix fake build in linux, it seems this build don't have IM schema check. Usually without CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK, if schema error happens, we would get a little bit more detailed error, here, we add additional error check in TestReadHandlerInvalidAttributePath. * address comments Co-authored-by: Boris Zbarsky --- src/app/InteractionModelEngine.cpp | 12 +- src/app/ReadHandler.cpp | 58 ++--- src/app/ReadHandler.h | 7 +- src/app/tests/TestReadInteraction.cpp | 314 +++++++++++++++++++++++++- 4 files changed, 339 insertions(+), 52 deletions(-) diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index b21d5b5a976604..e2acd4ac659834 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -406,7 +406,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest reader.Init(aPayload.Retain()); ReadRequestMessage::Parser readRequestParser; - VerifyOrReturnError(readRequestParser.Init(reader) == CHIP_NO_ERROR, Status::Failure); + VerifyOrReturnError(readRequestParser.Init(reader) == CHIP_NO_ERROR, Status::InvalidAction); { size_t requestedAttributePathCount = 0; @@ -458,15 +458,9 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest return Status::ResourceExhausted; } - CHIP_ERROR err = handler->OnInitialRequest(std::move(aPayload)); - if (err == CHIP_ERROR_NO_MEMORY) - { - return Status::ResourceExhausted; - } + handler->OnInitialRequest(std::move(aPayload)); - // TODO: Should probably map various TLV errors into InvalidAction, here - // or inside the read handler. - return StatusIB(err).mStatus; + return Status::Success; } Protocols::InteractionModel::Status InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index ff04308c06f269..4422edaf6d0e35 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -36,6 +36,7 @@ namespace chip { namespace app { +using Status = Protocols::InteractionModel::Status; ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext, InteractionType aInteractionType) : @@ -87,7 +88,7 @@ void ReadHandler::Close() mManagementCallback.OnDone(*this); } -CHIP_ERROR ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload) +void ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload) { CHIP_ERROR err = CHIP_NO_ERROR; System::PacketBufferHandle response; @@ -103,6 +104,12 @@ CHIP_ERROR ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload) if (err != CHIP_NO_ERROR) { + Status status = Status::InvalidAction; + if (err.IsIMStatus()) + { + status = StatusIB(err).mStatus; + } + StatusResponse::Send(status, mExchangeCtx.Get(), /* aExpectResponse = */ false); Close(); } else @@ -110,15 +117,17 @@ CHIP_ERROR ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload) // Force us to be in a dirty state so we get processed by the reporting mFlags.Set(ReadHandlerFlags::ForceDirty); } - - return err; } -CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload) +CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload, + bool & aSendStatusResponse) { CHIP_ERROR err = CHIP_NO_ERROR; + aSendStatusResponse = true; CHIP_ERROR statusError = CHIP_NO_ERROR; SuccessOrExit(err = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError)); + // Since this is a valid Status Response message, we don't have to send a Status Response in reply to it. + aSendStatusResponse = false; SuccessOrExit(err = statusError); switch (mState) { @@ -162,11 +171,6 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange } exit: - if (err != CHIP_NO_ERROR) - { - Close(); - } - return err; } @@ -249,15 +253,26 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b CHIP_ERROR ReadHandler::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) { - CHIP_ERROR err = CHIP_NO_ERROR; - + CHIP_ERROR err = CHIP_NO_ERROR; + bool sendStatusResponse = true; if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse)) { - err = OnStatusResponse(apExchangeContext, std::move(aPayload)); + err = OnStatusResponse(apExchangeContext, std::move(aPayload), sendStatusResponse); } else { - err = OnUnknownMsgType(apExchangeContext, aPayloadHeader, std::move(aPayload)); + ChipLogDetail(DataManagement, "ReadHandler:: Msg type %d not supported", aPayloadHeader.GetMessageType()); + err = CHIP_ERROR_INVALID_MESSAGE_TYPE; + } + + if (sendStatusResponse) + { + StatusResponse::Send(Status::InvalidAction, apExchangeContext, false /*aExpectResponse*/); + } + + if (err != CHIP_NO_ERROR) + { + Close(); } return err; } @@ -269,14 +284,6 @@ bool ReadHandler::IsFromSubscriber(Messaging::ExchangeContext & apExchangeContex GetAccessingFabricIndex() == apExchangeContext.GetSessionHandle()->GetFabricIndex()); } -CHIP_ERROR ReadHandler::OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, - System::PacketBufferHandle && aPayload) -{ - ChipLogDetail(DataManagement, "Msg type %d not supported", aPayloadHeader.GetMessageType()); - Close(); - return CHIP_ERROR_INVALID_MESSAGE_TYPE; -} - void ReadHandler::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) { ChipLogError(DataManagement, "Time out! failed to receive status response from Exchange: " ChipLogFormatExchange, @@ -298,14 +305,7 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa ReturnErrorOnFailure(readRequestParser.Init(reader)); #if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK - err = readRequestParser.CheckSchemaValidity(); - if (err != CHIP_NO_ERROR) - { - // The actual error we want to return to our consumer is an IM "invalid - // action" error, not whatever internal error CheckSchemaValidity - // happens to come up with. - return CHIP_IM_GLOBAL_STATUS(InvalidAction); - } + ReturnErrorOnFailure(readRequestParser.CheckSchemaValidity()); #endif err = readRequestParser.GetAttributeRequests(&attributePathListParser); diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 548df710a96b2c..3e192a2abf5f59 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -240,7 +240,7 @@ class ReadHandler : public Messaging::ExchangeDelegate * @retval #CHIP_NO_ERROR On success. * */ - CHIP_ERROR OnInitialRequest(System::PacketBufferHandle && aPayload); + void OnInitialRequest(System::PacketBufferHandle && aPayload); /** * Send ReportData to initiator @@ -369,12 +369,11 @@ class ReadHandler : public Messaging::ExchangeDelegate CHIP_ERROR ProcessAttributePathList(AttributePathIBs::Parser & aAttributePathListParser); CHIP_ERROR ProcessEventPaths(EventPathIBs::Parser & aEventPathsParser); CHIP_ERROR ProcessEventFilters(EventFilterIBs::Parser & aEventFiltersParser); - CHIP_ERROR OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); + CHIP_ERROR OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload, + bool & aSendStatusResponse); CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override; void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override; - CHIP_ERROR OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, - System::PacketBufferHandle && aPayload); void MoveToState(const HandlerState aTargetState); const char * GetStateStr() const; diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 720ab0f06324a8..2b78cb8f4c9388 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -329,6 +330,11 @@ class TestReadInteraction static void TestSubscribeClientReceiveUnsolicitedReportMessageWithInvalidSubscriptionId(nlTestSuite * apSuite, void * apContext); static void TestReadChunkingInvalidSubscriptionId(nlTestSuite * apSuite, void * apContext); + static void TestReadHandlerMalformedReadRequest1(nlTestSuite * apSuite, void * apContext); + static void TestReadHandlerMalformedReadRequest2(nlTestSuite * apSuite, void * apContext); + static void TestSubscribeSendUnknownMessage(nlTestSuite * apSuite, void * apContext); + static void TestSubscribeSendInvalidStatusReport(nlTestSuite * apSuite, void * apContext); + static void TestReadHandlerInvalidSubscribeRequest(nlTestSuite * apSuite, void * apContext); private: static void GenerateReportData(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload, @@ -515,7 +521,9 @@ void TestReadInteraction::TestReadHandler(nlTestSuite * apSuite, void * apContex err = writer.Finalize(&readRequestbuf); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = readHandler.OnInitialRequest(std::move(readRequestbuf)); + // Call ProcessReadRequest directly, because OnInitialRequest sends status + // messages on the wire instead of returning an error. + err = readHandler.ProcessReadRequest(std::move(readRequestbuf)); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); } @@ -642,22 +650,27 @@ void TestReadInteraction::TestReadHandlerInvalidAttributePath(nlTestSuite * apSu AttributePathIB::Builder & attributePathBuilder = attributePathListBuilder.CreatePath(); NL_TEST_ASSERT(apSuite, attributePathListBuilder.GetError() == CHIP_NO_ERROR); - attributePathBuilder.Node(1).Endpoint(2).Cluster(3).ListIndex(5).EndOfAttributePathIB(); + attributePathBuilder.Node(1).Endpoint(2).Cluster(3).EndOfAttributePathIB(); err = attributePathBuilder.GetError(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); attributePathListBuilder.EndOfAttributePathIBs(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - readRequestBuilder.IsFabricFiltered(false).EndOfReadRequestMessage(); + readRequestBuilder.EndOfReadRequestMessage(); NL_TEST_ASSERT(apSuite, readRequestBuilder.GetError() == CHIP_NO_ERROR); err = writer.Finalize(&readRequestbuf); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = readHandler.OnInitialRequest(std::move(readRequestbuf)); - NL_TEST_ASSERT(apSuite, err == CHIP_IM_GLOBAL_STATUS(InvalidAction)); + err = readHandler.ProcessReadRequest(std::move(readRequestbuf)); + ChipLogError(DataManagement, "The error is %s", ErrorStr(err)); +#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK + NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_IM_MALFORMED_READ_REQUEST_MESSAGE); +#else + NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_END_OF_TLV); +#endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK // - // In the call above to OnInitialRequest, the handler will not actually close out the EC since + // In the call above to ProcessReadRequest, the handler will not actually close out the EC since // it expects the ExchangeManager to do so automatically given it's not calling WillSend() on the EC, // and is not sending a response back. // @@ -2928,7 +2941,6 @@ void TestReadInteraction::TestSubscribeClientReceiveWellFormedStatusResponse(nlT // The ReadHandler's exchange is still open when we synthesize the StatusResponse. // Since we synthesized the StatusResponse to the ReadClient, instead of sending it from the ReadHandler, // the only messages here are the ReadClient's StatusResponse to the unexpected message and an MRP ack. - // The ReadHandler should have sent a StatusResponse too, but it's buggy and does not do that. NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 2); NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_ERROR_INVALID_MESSAGE_TYPE); @@ -3017,7 +3029,6 @@ void TestReadInteraction::TestSubscribeClientReceiveInvalidReportMessage(nlTestS // The ReadHandler's exchange is still open when we synthesize the ReportData. // Since we synthesized the ReportData to the ReadClient, instead of sending it from the ReadHandler, // the only messages here are the ReadClient's StatusResponse to the unexpected message and an MRP ack. - // The ReadHandler should have sent a StatusResponse too, but it's buggy and does not do that. NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 2); NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_ERROR_END_OF_TLV); @@ -3097,8 +3108,7 @@ void TestReadInteraction::TestSubscribeClientReceiveUnsolicitedInvalidReportMess // The server sends a data report. // The client receives the data report data and sends out status report with invalid action. - // The server should respond with a status report of its own, leading to 4 messages (because - // the client would ack the server's status report), but it's buggy and just sends an ack to the status report it got. + // The server acks the status report. NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 3); } NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); @@ -3362,6 +3372,285 @@ void TestReadInteraction::TestReadChunkingInvalidSubscriptionId(nlTestSuite * ap ctx.CreateSessionBobToAlice(); } +// Read Client sends a malformed read request, interaction model engine fails to parse the request and generates a status report to +// client, and client is closed. +void TestReadInteraction::TestReadHandlerMalformedReadRequest1(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + 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); + + MockInteractionModelApp delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Read); + System::PacketBufferHandle msgBuf; + ReadRequestMessage::Builder request; + System::PacketBufferTLVWriter writer; + + chip::app::InitWriterWithSpaceReserved(writer, 0); + err = request.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = writer.Finalize(&msgBuf); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + auto exchange = readClient.mpExchangeMgr->NewContext(readPrepareParams.mSessionHolder.Get().Value(), &readClient); + NL_TEST_ASSERT(apSuite, exchange != nullptr); + readClient.mExchange.Grab(exchange); + readClient.MoveToState(app::ReadClient::ClientState::AwaitingInitialReport); + err = readClient.mExchange->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf), + Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + engine->Shutdown(); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + +// Read Client sends a malformed read request, read handler fails to parse the request and generates a status report to client, and +// client is closed. +void TestReadInteraction::TestReadHandlerMalformedReadRequest2(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + 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); + + MockInteractionModelApp delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Read); + System::PacketBufferHandle msgBuf; + ReadRequestMessage::Builder request; + System::PacketBufferTLVWriter writer; + + chip::app::InitWriterWithSpaceReserved(writer, 0); + err = request.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(apSuite, request.EndOfReadRequestMessage().GetError() == CHIP_NO_ERROR); + err = writer.Finalize(&msgBuf); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + auto exchange = readClient.mpExchangeMgr->NewContext(readPrepareParams.mSessionHolder.Get().Value(), &readClient); + NL_TEST_ASSERT(apSuite, exchange != nullptr); + readClient.mExchange.Grab(exchange); + readClient.MoveToState(app::ReadClient::ClientState::AwaitingInitialReport); + err = readClient.mExchange->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf), + Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + ChipLogError(DataManagement, "The error is %s", ErrorStr(delegate.mError)); + NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + engine->Shutdown(); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + +// Read Client creates a subscription with the server, server sends chunked reports, after the handler sends out the first chunked +// report, client sends out invalid write request message, handler sends status report with invalid action and closes +void TestReadInteraction::TestSubscribeSendUnknownMessage(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + 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); + + MockInteractionModelApp delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + chip::app::AttributePathParams attributePathParams[1]; + // Mock Attribute 4 is a big attribute, with 6 large OCTET_STRING + attributePathParams[0].mEndpointId = Test::kMockEndpoint3; + attributePathParams[0].mClusterId = Test::MockClusterId(2); + attributePathParams[0].mAttributeId = Test::MockAttributeId(4); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = 1; + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Subscribe); + + ctx.GetLoopback().mSentMessageCount = 0; + ctx.GetLoopback().mNumMessagesToDrop = 1; + ctx.GetLoopback().mNumMessagesToAllowBeforeDropping = 1; + ctx.GetLoopback().mDroppedMessageCount = 0; + err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 2); + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mDroppedMessageCount == 1); + + ctx.GetLoopback().mSentMessageCount = 0; + rm->ClearRetransTable(readClient.mExchange.Get()); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers() == 1); + NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); + // Server sends out status report, client should send status report along with Piggybacking ack, but we don't do that + // Instead, we send out unknown message to server,but server is still waiting for ack, so we need + // clear out retranstable + rm->ClearRetransTable(engine->ActiveHandlerAt(0)->mExchangeCtx.Get()); + System::PacketBufferHandle msgBuf; + ReadRequestMessage::Builder request; + System::PacketBufferTLVWriter writer; + chip::app::InitWriterWithSpaceReserved(writer, 0); + request.Init(&writer); + writer.Finalize(&msgBuf); + + err = readClient.mExchange->SendMessage(Protocols::InteractionModel::MsgType::WriteRequest, std::move(msgBuf)); + ctx.DrainAndServiceIO(); + // client sends invalid write request, server sends out status report with invalid action and closes, client replies with + // status report server replies with MRP Ack + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 4); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers() == 0); + } + + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + engine->Shutdown(); + ctx.ExpireSessionAliceToBob(); + ctx.ExpireSessionBobToAlice(); + ctx.CreateSessionAliceToBob(); + ctx.CreateSessionBobToAlice(); +} + +// Read Client creates a subscription with the server, server sends chunked reports, after the handler sends out invalid status +// report, client sends out invalid status report message, handler sends status report with invalid action and close +void TestReadInteraction::TestSubscribeSendInvalidStatusReport(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + 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); + + MockInteractionModelApp delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + chip::app::AttributePathParams attributePathParams[1]; + // Mock Attribute 4 is a big attribute, with 6 large OCTET_STRING + attributePathParams[0].mEndpointId = Test::kMockEndpoint3; + attributePathParams[0].mClusterId = Test::MockClusterId(2); + attributePathParams[0].mAttributeId = Test::MockAttributeId(4); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = 1; + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Subscribe); + + ctx.GetLoopback().mSentMessageCount = 0; + ctx.GetLoopback().mNumMessagesToDrop = 1; + ctx.GetLoopback().mNumMessagesToAllowBeforeDropping = 1; + ctx.GetLoopback().mDroppedMessageCount = 0; + + err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 2); + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mDroppedMessageCount == 1); + ctx.GetLoopback().mSentMessageCount = 0; + rm->ClearRetransTable(readClient.mExchange.Get()); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers() == 1); + NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); + rm->ClearRetransTable(engine->ActiveHandlerAt(0)->mExchangeCtx.Get()); + + System::PacketBufferHandle msgBuf; + StatusResponseMessage::Builder request; + System::PacketBufferTLVWriter writer; + chip::app::InitWriterWithSpaceReserved(writer, 0); + request.Init(&writer); + writer.Finalize(&msgBuf); + + err = readClient.mExchange->SendMessage(Protocols::InteractionModel::MsgType::StatusResponse, std::move(msgBuf)); + ctx.DrainAndServiceIO(); + + // client sends malformed status response, server sends out status report with invalid action and close, client replies with + // status report server replies with MRP Ack + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 4); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers() == 0); + } + + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + engine->Shutdown(); + ctx.ExpireSessionAliceToBob(); + ctx.ExpireSessionBobToAlice(); + ctx.CreateSessionAliceToBob(); + ctx.CreateSessionBobToAlice(); +} + +// Read Client sends a malformed subscribe request, the server fails to parse the request and generates a status report to the +// client, and client closes itself. +void TestReadInteraction::TestReadHandlerInvalidSubscribeRequest(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + 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); + + MockInteractionModelApp delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Subscribe); + System::PacketBufferHandle msgBuf; + ReadRequestMessage::Builder request; + System::PacketBufferTLVWriter writer; + + chip::app::InitWriterWithSpaceReserved(writer, 0); + err = request.Init(&writer); + err = writer.Finalize(&msgBuf); + + auto exchange = readClient.mpExchangeMgr->NewContext(readPrepareParams.mSessionHolder.Get().Value(), &readClient); + NL_TEST_ASSERT(apSuite, exchange != nullptr); + readClient.mExchange.Grab(exchange); + readClient.MoveToState(app::ReadClient::ClientState::AwaitingInitialReport); + err = readClient.mExchange->SendMessage(Protocols::InteractionModel::MsgType::SubscribeRequest, std::move(msgBuf), + Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction)); + } + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + engine->Shutdown(); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + } // namespace app } // namespace chip @@ -3401,6 +3690,11 @@ const nlTest sTests[] = NL_TEST_DEF("TestSubscribeClientReceiveInvalidSubscribeResponseMessage", chip::app::TestReadInteraction::TestSubscribeClientReceiveInvalidSubscribeResponseMessage), NL_TEST_DEF("TestSubscribeClientReceiveUnsolicitedReportMessageWithInvalidSubscriptionId", chip::app::TestReadInteraction::TestSubscribeClientReceiveUnsolicitedReportMessageWithInvalidSubscriptionId), NL_TEST_DEF("TestReadChunkingInvalidSubscriptionId", chip::app::TestReadInteraction::TestReadChunkingInvalidSubscriptionId), + NL_TEST_DEF("TestReadHandlerMalformedReadRequest1", chip::app::TestReadInteraction::TestReadHandlerMalformedReadRequest1), + NL_TEST_DEF("TestReadHandlerMalformedReadRequest2", chip::app::TestReadInteraction::TestReadHandlerMalformedReadRequest2), + NL_TEST_DEF("TestSubscribeSendUnknownMessage", chip::app::TestReadInteraction::TestSubscribeSendUnknownMessage), + NL_TEST_DEF("TestSubscribeSendInvalidStatusReport", chip::app::TestReadInteraction::TestSubscribeSendInvalidStatusReport), + NL_TEST_DEF("TestReadHandlerInvalidSubscribeRequest", chip::app::TestReadInteraction::TestReadHandlerInvalidSubscribeRequest), NL_TEST_DEF("TestSubscribeUrgentWildcardEvent", chip::app::TestReadInteraction::TestSubscribeUrgentWildcardEvent), NL_TEST_DEF("TestSubscribeWildcard", chip::app::TestReadInteraction::TestSubscribeWildcard), NL_TEST_DEF("TestSubscribePartialOverlap", chip::app::TestReadInteraction::TestSubscribePartialOverlap),