add check to make sure minInterval is less than maxInterval for subsc…

…ribe request (#11771)

src/app/ReadClient.cpp

@@ -624,6 +624,8 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara
  SuccessOrExit(err);
  }
 
+ VerifyOrExit(aReadPrepareParams.mMinIntervalFloorSeconds < aReadPrepareParams.mMaxIntervalCeilingSeconds,
+ err = CHIP_ERROR_INVALID_ARGUMENT);
  request.MinIntervalSeconds(aReadPrepareParams.mMinIntervalFloorSeconds)
  .MaxIntervalSeconds(aReadPrepareParams.mMaxIntervalCeilingSeconds)
  .KeepSubscriptions(aReadPrepareParams.mKeepSubscriptions)
@@ -648,10 +650,7 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara
 exit:
  if (err != CHIP_NO_ERROR)
  {
- AbortExistingExchangeContext();
- }
- if (err != CHIP_NO_ERROR)
- {
+ ChipLogError(DataManagement, "Failed to send subscribe request: %" CHIP_ERROR_FORMAT, err.Format());
  Shutdown();
  }
  return err;

src/app/ReadHandler.cpp

@@ -506,17 +506,17 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse()
  writer.Init(std::move(packet));
 
  SubscribeResponseMessage::Builder response;
- ReturnLogErrorOnFailure(response.Init(&writer));
+ ReturnErrorOnFailure(response.Init(&writer));
  response.SubscriptionId(mSubscriptionId)
  .MinIntervalFloorSeconds(mMinIntervalFloorSeconds)
  .MaxIntervalCeilingSeconds(mMaxIntervalCeilingSeconds)
  .EndOfSubscribeResponseMessage();
- ReturnLogErrorOnFailure(response.GetError());
+ ReturnErrorOnFailure(response.GetError());
 
- ReturnLogErrorOnFailure(writer.Finalize(&packet));
+ ReturnErrorOnFailure(writer.Finalize(&packet));
  VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
- ReturnLogErrorOnFailure(RefreshSubscribeSyncTimer());
+ ReturnErrorOnFailure(RefreshSubscribeSyncTimer());
  mInitialReport = false;
  MoveToState(HandlerState::GeneratingReports);
  if (mpDelegate != nullptr)
@@ -531,11 +531,11 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP
  System::PacketBufferTLVReader reader;
  reader.Init(std::move(aPayload));
 
- ReturnLogErrorOnFailure(reader.Next());
+ ReturnErrorOnFailure(reader.Next());
  SubscribeRequestMessage::Parser subscribeRequestParser;
- ReturnLogErrorOnFailure(subscribeRequestParser.Init(reader));
+ ReturnErrorOnFailure(subscribeRequestParser.Init(reader));
 #if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
- ReturnLogErrorOnFailure(subscribeRequestParser.CheckSchemaValidity());
+ ReturnErrorOnFailure(subscribeRequestParser.CheckSchemaValidity());
 #endif
 
  AttributePathIBs::Parser attributePathListParser;
@@ -546,9 +546,9 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP
  }
  else if (err == CHIP_NO_ERROR)
  {
- ReturnLogErrorOnFailure(ProcessAttributePathList(attributePathListParser));
+ ReturnErrorOnFailure(ProcessAttributePathList(attributePathListParser));
  }
- ReturnLogErrorOnFailure(err);
+ ReturnErrorOnFailure(err);
 
  EventPaths::Parser eventPathListParser;
  err = subscribeRequestParser.GetEventPaths(&eventPathListParser);
@@ -558,13 +558,14 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP
  }
  else if (err == CHIP_NO_ERROR)
  {
- ReturnLogErrorOnFailure(ProcessEventPaths(eventPathListParser));
+ ReturnErrorOnFailure(ProcessEventPaths(eventPathListParser));
  }
- ReturnLogErrorOnFailure(err);
+ ReturnErrorOnFailure(err);
 
- ReturnLogErrorOnFailure(subscribeRequestParser.GetMinIntervalSeconds(&mMinIntervalFloorSeconds));
- ReturnLogErrorOnFailure(subscribeRequestParser.GetMaxIntervalSeconds(&mMaxIntervalCeilingSeconds));
- ReturnLogErrorOnFailure(Crypto::DRBG_get_bytes(reinterpret_cast<uint8_t *>(&mSubscriptionId), sizeof(mSubscriptionId)));
+ ReturnErrorOnFailure(subscribeRequestParser.GetMinIntervalSeconds(&mMinIntervalFloorSeconds));
+ ReturnErrorOnFailure(subscribeRequestParser.GetMaxIntervalSeconds(&mMaxIntervalCeilingSeconds));
+ VerifyOrReturnError(mMinIntervalFloorSeconds < mMaxIntervalCeilingSeconds, CHIP_ERROR_INVALID_ARGUMENT);
+ ReturnErrorOnFailure(Crypto::DRBG_get_bytes(reinterpret_cast<uint8_t *>(&mSubscriptionId), sizeof(mSubscriptionId)));
 
  MoveToState(HandlerState::GeneratingReports);
 

src/app/tests/TestReadInteraction.cpp

@@ -276,6 +276,7 @@ class TestReadInteraction
  static void TestSubscribeEarlyShutdown(nlTestSuite * apSuite, void * apContext);
  static void TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite * apSuite, void * apContext);
  static void TestReadInvalidAttributePathRoundtrip(nlTestSuite * apSuite, void * apContext);
+ static void TestSubscribeInvalidIterval(nlTestSuite * apSuite, void * apContext);
 
 private:
  static void GenerateReportData(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload,
@@ -1111,6 +1112,42 @@ void TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite
  engine->Shutdown();
 }
 
+void TestReadInteraction::TestSubscribeInvalidIterval(nlTestSuite * apSuite, void * apContext)
+{
+ TestContext & ctx = *static_cast<TestContext *>(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);
+
+ GenerateEvents(apSuite, apContext);
+
+ MockInteractionModelApp delegate;
+ auto * engine = chip::app::InteractionModelEngine::GetInstance();
+ err = engine->Init(&ctx.GetExchangeManager(), &delegate);
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+ NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse);
+
+ ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
+ chip::app::AttributePathParams attributePathParams[1];
+ readPrepareParams.mpAttributePathParamsList = attributePathParams;
+ readPrepareParams.mpAttributePathParamsList[0].mEndpointId = kTestEndpointId;
+ readPrepareParams.mpAttributePathParamsList[0].mClusterId = kTestClusterId;
+ readPrepareParams.mpAttributePathParamsList[0].mAttributeId = 1;
+
+ readPrepareParams.mAttributePathParamsListSize = 1;
+
+ readPrepareParams.mSessionHandle = ctx.GetSessionBobToAlice();
+ readPrepareParams.mMinIntervalFloorSeconds = 5;
+ readPrepareParams.mMaxIntervalCeilingSeconds = 5;
+ printf("\nSend subscribe request message to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId);
+
+ err = chip::app::InteractionModelEngine::GetInstance()->SendSubscribeRequest(readPrepareParams, &delegate);
+ NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
+ engine->Shutdown();
+}
+
 } // namespace app
 } // namespace chip
 
@@ -1138,6 +1175,7 @@ const nlTest sTests[] =
  NL_TEST_DEF("TestSubscribeEarlyShutdown", chip::app::TestReadInteraction::TestSubscribeEarlyShutdown),
  NL_TEST_DEF("TestSubscribeInvalidAttributePathRoundtrip", chip::app::TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip),
  NL_TEST_DEF("TestReadInvalidAttributePathRoundtrip", chip::app::TestReadInteraction::TestReadInvalidAttributePathRoundtrip),
+ NL_TEST_DEF("TestSubscribeInvalidIterval", chip::app::TestReadInteraction::TestSubscribeInvalidIterval),
  NL_TEST_SENTINEL()
 };
 // clang-format on