From 32866c0324566cdf970874ac450d159cf61b1531 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 27 May 2022 14:40:17 -0400 Subject: [PATCH] Add a test-only API to allow subscribing with invalid intervals. (#18855) This allows chip-tool to test server behavior when MinIntervalFloor > MaxIntervalCeiling. Fixes https://github.com/project-chip/connectedhomeip/issues/18839 --- src/app/ReadClient.cpp | 26 ++++++++++--------- src/app/ReadClient.h | 15 ++++++++++- .../interaction_model/InteractionModel.cpp | 11 +++++++- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 6ff7801934d0dd..6a0c50702b2bcd 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -840,11 +840,16 @@ CHIP_ERROR ReadClient::SendAutoResubscribeRequest(ReadPrepareParams && aReadPrep return err; } -CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPrepareParams) +CHIP_ERROR ReadClient::SendSubscribeRequest(const ReadPrepareParams & aReadPrepareParams) { - CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrReturnError(aReadPrepareParams.mMinIntervalFloorSeconds <= aReadPrepareParams.mMaxIntervalCeilingSeconds, + CHIP_ERROR_INVALID_ARGUMENT); + return SendSubscribeRequestImpl(aReadPrepareParams); +} - VerifyOrReturnError(ClientState::Idle == mState, err = CHIP_ERROR_INCORRECT_STATE); +CHIP_ERROR ReadClient::SendSubscribeRequestImpl(const ReadPrepareParams & aReadPrepareParams) +{ + VerifyOrReturnError(ClientState::Idle == mState, CHIP_ERROR_INCORRECT_STATE); // Todo: Remove the below, Update span in ReadPrepareParams Span attributePaths(aReadPrepareParams.mpAttributePathParamsList, @@ -853,9 +858,6 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara Span dataVersionFilters(aReadPrepareParams.mpDataVersionFilterList, aReadPrepareParams.mDataVersionFilterListSize); - VerifyOrReturnError(aReadPrepareParams.mMinIntervalFloorSeconds <= aReadPrepareParams.mMaxIntervalCeilingSeconds, - err = CHIP_ERROR_INVALID_ARGUMENT); - System::PacketBufferHandle msgBuf; System::PacketBufferTLVWriter writer; SubscribeRequestMessage::Builder request; @@ -870,14 +872,14 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara if (!attributePaths.empty()) { AttributePathIBs::Builder & attributePathListBuilder = request.CreateAttributeRequests(); - ReturnErrorOnFailure(err = attributePathListBuilder.GetError()); + ReturnErrorOnFailure(attributePathListBuilder.GetError()); ReturnErrorOnFailure(GenerateAttributePaths(attributePathListBuilder, attributePaths)); } if (!eventPaths.empty()) { EventPathIBs::Builder & eventPathListBuilder = request.CreateEventRequests(); - ReturnErrorOnFailure(err = eventPathListBuilder.GetError()); + ReturnErrorOnFailure(eventPathListBuilder.GetError()); ReturnErrorOnFailure(GenerateEventPaths(eventPathListBuilder, eventPaths)); Optional eventMin; @@ -885,12 +887,12 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara if (eventMin.HasValue()) { EventFilterIBs::Builder & eventFilters = request.CreateEventFilters(); - ReturnErrorOnFailure(err = request.GetError()); + ReturnErrorOnFailure(request.GetError()); ReturnErrorOnFailure(eventFilters.GenerateEventFilter(eventMin.Value())); } } - ReturnErrorOnFailure(err = request.IsFabricFiltered(aReadPrepareParams.mIsFabricFiltered).GetError()); + ReturnErrorOnFailure(request.IsFabricFiltered(aReadPrepareParams.mIsFabricFiltered).GetError()); bool encodedDataVersionList = false; TLV::TLVWriter backup; @@ -912,13 +914,13 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara request.Rollback(backup); } - ReturnErrorOnFailure(err = request.EndOfSubscribeRequestMessage().GetError()); + ReturnErrorOnFailure(request.EndOfSubscribeRequestMessage().GetError()); ReturnErrorOnFailure(writer.Finalize(&msgBuf)); VerifyOrReturnError(aReadPrepareParams.mSessionHolder, CHIP_ERROR_MISSING_SECURE_SESSION); mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get().Value(), this); - VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY); + VerifyOrReturnError(mpExchangeCtx != nullptr, CHIP_ERROR_NO_MEMORY); mpExchangeCtx->SetResponseTimeout(kImMessageTimeout); ReturnErrorOnFailure(mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeRequest, std::move(msgBuf), diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 68b2f63b1ab815..65f0e808e43c61 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -302,6 +302,16 @@ class ReadClient : public Messaging::ExchangeDelegate // that's the only case when the consumer moved a ReadParams into the client. CHIP_ERROR SendAutoResubscribeRequest(ReadPrepareParams && aReadPrepareParams); + // Like SendSubscribeRequest, but allows sending certain forms of invalid + // subscribe requests that servers are expected to reject, for testing + // purposes. Should only be called from tests. +#if CONFIG_IM_BUILD_FOR_UNIT_TEST + CHIP_ERROR SendSubscribeRequestWithoutValidation(const ReadPrepareParams & aReadPrepareParams) + { + return SendSubscribeRequestImpl(aReadPrepareParams); + } +#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST + private: friend class TestReadInteraction; friend class InteractionModelEngine; @@ -358,7 +368,10 @@ class ReadClient : public Messaging::ExchangeDelegate bool ResubscribeIfNeeded(); // Specialized request-sending functions. CHIP_ERROR SendReadRequest(ReadPrepareParams & aReadPrepareParams); - CHIP_ERROR SendSubscribeRequest(ReadPrepareParams & aSubscribePrepareParams); + // SendSubscribeRequest performs som validation on aSubscribePrepareParams + // and then calls SendSubscribeRequestImpl. + CHIP_ERROR SendSubscribeRequest(const ReadPrepareParams & aSubscribePrepareParams); + CHIP_ERROR SendSubscribeRequestImpl(const ReadPrepareParams & aSubscribePrepareParams); void UpdateDataVersionFilters(const ConcreteDataAttributePath & aPath); static void OnResubscribeTimerCallback(System::Layer * apSystemLayer, void * apAppState); diff --git a/src/app/tests/suites/commands/interaction_model/InteractionModel.cpp b/src/app/tests/suites/commands/interaction_model/InteractionModel.cpp index f52e5ae5261a90..2d144ab7f58add 100644 --- a/src/app/tests/suites/commands/interaction_model/InteractionModel.cpp +++ b/src/app/tests/suites/commands/interaction_model/InteractionModel.cpp @@ -300,7 +300,16 @@ CHIP_ERROR InteractionModelReports::ReportAttribute(DeviceProxy * device, std::v auto client = std::make_unique(InteractionModelEngine::GetInstance(), device->GetExchangeManager(), mBufferedReadAdapter, interactionType); - ReturnErrorOnFailure(client->SendRequest(params)); + if (interactionType == ReadClient::InteractionType::Read) + { + ReturnErrorOnFailure(client->SendRequest(params)); + } + else + { + // We want to allow certain kinds of spec-invalid subscriptions so we + // can test how the server reacts to them. + ReturnErrorOnFailure(client->SendSubscribeRequestWithoutValidation(params)); + } mReadClients.push_back(std::move(client)); return CHIP_NO_ERROR; }