Fix issue 14566 (#15222)

Send status report when engine cannot put oversized attribute in report
project-chip · Oct 17, 2023 · 1768260 · 1768260
1 parent 54cf9aa
commit 1768260
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 4 deletions.
diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp
@@ -197,6 +197,26 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange
     return err;
 }
 
+CHIP_ERROR ReadHandler::SendStatusReport(Protocols::InteractionModel::Status aStatus)
+{
+    VerifyOrReturnLogError(IsReportable(), CHIP_ERROR_INCORRECT_STATE);
+    if (IsPriming() || IsChunkedReport())
+    {
+        mSessionHandle.Grab(mpExchangeCtx->GetSessionHandle());
+    }
+    else
+    {
+        VerifyOrReturnLogError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE);
+        VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE);
+        mpExchangeCtx = mpExchangeMgr->NewContext(mSessionHandle.Get(), this);
+    }
+    VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    mpExchangeCtx->SetResponseTimeout(kImMessageTimeout);
+
+    return StatusResponse::Send(aStatus, mpExchangeCtx,
+                                /* aExpectResponse = */ false);
+}
+
 CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, bool aMoreChunks)
 {
     VerifyOrReturnLogError(IsReportable(), CHIP_ERROR_INCORRECT_STATE);

diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h
@@ -176,6 +176,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
     const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; }
     void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; }
     uint32_t GetLastWrittenEventsBytes() { return mLastWrittenEventsBytes; }
+    CHIP_ERROR SendStatusReport(Protocols::InteractionModel::Status aStatus);
 
 private:
     friend class TestReadInteraction;

diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp
@@ -358,6 +358,7 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
     chip::System::PacketBufferHandle bufHandle = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
     uint16_t reservedSize                      = 0;
     bool hasMoreChunks                         = false;
+    bool needCloseReadHandler                  = false;
 
     // Reserved size for the MoreChunks boolean flag, which takes up 1 byte for the control tag and 1 byte for the context tag.
     const uint32_t kReservedSizeForMoreChunksFlag = 1 + 1;
@@ -420,8 +421,13 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
         if (!hasEncodedAttributes && !hasEncodedEvents && hasMoreChunks)
         {
             ChipLogError(DataManagement,
-                         "No data actually encoded but hasMoreChunks flag is set, abort report! (attribute too big?)");
-            ExitNow(err = CHIP_ERROR_INCORRECT_STATE);
+                         "No data actually encoded but hasMoreChunks flag is set, close read handler! (attribute too big?)");
+            err = apReadHandler->SendStatusReport(Protocols::InteractionModel::Status::ResourceExhausted);
+            if (err == CHIP_NO_ERROR)
+            {
+                needCloseReadHandler = true;
+            }
+            ExitNow();
         }
     }
 
@@ -465,7 +471,7 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
         //
         apReadHandler->Abort();
     }
-    else if (apReadHandler->IsType(ReadHandler::InteractionType::Read) && !hasMoreChunks)
+    else if ((apReadHandler->IsType(ReadHandler::InteractionType::Read) && !hasMoreChunks) || needCloseReadHandler)
     {
         //
         // In the case of successful report generation and we're on the last chunk of a read, we don't expect

diff --git a/src/controller/tests/TestReadChunking.cpp b/src/controller/tests/TestReadChunking.cpp
@@ -418,7 +418,7 @@ void TestCommandInteraction::TestBadChunking(nlTestSuite * apSuite, void * apCon
         NL_TEST_ASSERT(apSuite, readCallback.mAttributeCount == 0);
 
         // The server should shutted down, while the client is still alive (pending for the attribute data.)
-        NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 1);
+        NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
     }
 
     // Sanity check