Fix the oversized list handling in ClusterStateCache

project-chip · May 7, 2022 · 1070327 · 1070327
1 parent 94756b0
commit 1070327
Show file tree

Hide file tree

Showing 5 changed files with 551 additions and 58 deletions.
diff --git a/src/app/ClusterStateCache.cpp b/src/app/ClusterStateCache.cpp
@@ -28,8 +28,6 @@ CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPat
                                           const StatusIB & aStatus)
 {
     AttributeState state;
-    System::PacketBufferHandle handle;
-    System::PacketBufferTLVWriter writer;
     bool endpointIsNew = false;
 
     if (mCache.find(aPath.mEndpointId) == mCache.end())
@@ -44,20 +42,15 @@ CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPat
 
     if (apData)
     {
-        handle = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
-
-        writer.Init(std::move(handle), false);
-
+        Platform::ScopedMemoryBuffer<uint8_t> backingBuffer;
+        uint32_t totalBufSize = apData->GetTotalLength();
+        backingBuffer.Calloc(totalBufSize);
+        VerifyOrReturnError(backingBuffer.Get() != nullptr, CHIP_ERROR_NO_MEMORY);
+        TLV::ScopedBufferTLVWriter writer(std::move(backingBuffer), totalBufSize);
         ReturnErrorOnFailure(writer.CopyElement(TLV::AnonymousTag(), *apData));
-        ReturnErrorOnFailure(writer.Finalize(&handle));
-
-        //
-        // Compact the buffer down to a more reasonably sized packet buffer
-        // if we can.
-        //
-        handle.RightSize();
 
-        state.Set<System::PacketBufferHandle>(std::move(handle));
+        ReturnErrorOnFailure(writer.Finalize(backingBuffer));
+        state.Set<Platform::ScopedMemoryBuffer<uint8_t>>(std::move(backingBuffer));
 
         //
         // Clear out the committed data version and only set it again once we have received all data for this cluster.
@@ -204,22 +197,15 @@ void ClusterStateCache::OnReportEnd()
 CHIP_ERROR ClusterStateCache::Get(const ConcreteAttributePath & path, TLV::TLVReader & reader)
 {
     CHIP_ERROR err;
-
     auto attributeState = GetAttributeState(path.mEndpointId, path.mClusterId, path.mAttributeId, err);
     ReturnErrorOnFailure(err);
-
     if (attributeState->Is<StatusIB>())
     {
         return CHIP_ERROR_IM_STATUS_CODE_RECEIVED;
     }
 
-    System::PacketBufferTLVReader bufReader;
-
-    bufReader.Init(attributeState->Get<System::PacketBufferHandle>().Retain());
-    ReturnErrorOnFailure(bufReader.Next());
-
-    reader.Init(bufReader);
-    return CHIP_NO_ERROR;
+    reader.Init(attributeState->Get<Platform::ScopedMemoryBuffer<uint8_t>>().Get(), attributeState->Get<Platform::ScopedMemoryBuffer<uint8_t>>().GetSize());
+    return reader.Next();
 }
 
 CHIP_ERROR ClusterStateCache::Get(EventNumber eventNumber, TLV::TLVReader & reader)
@@ -336,10 +322,11 @@ void ClusterStateCache::OnAttributeData(const ConcreteDataAttributePath & aPath,
     mCallback.OnAttributeData(aPath, apData ? &dataSnapshot : nullptr, aStatus);
 }
 
-CHIP_ERROR ClusterStateCache::GetVersion(EndpointId mEndpointId, ClusterId mClusterId, Optional<DataVersion> & aVersion)
+CHIP_ERROR ClusterStateCache::GetVersion(const ConcreteClusterPath & aPath , Optional<DataVersion> & aVersion)
 {
+    VerifyOrReturnError(aPath.IsValidConcreteClusterPath(), CHIP_ERROR_INVALID_ARGUMENT);
     CHIP_ERROR err;
-    auto clusterState = GetClusterState(mEndpointId, mClusterId, err);
+    auto clusterState = GetClusterState(aPath.mEndpointId, aPath.mClusterId, err);
     ReturnErrorOnFailure(err);
     aVersion = clusterState->mCommittedDataVersion;
     return CHIP_NO_ERROR;
@@ -417,8 +404,8 @@ void ClusterStateCache::GetSortedFilters(std::vector<std::pair<DataVersionFilter
                 }
                 else
                 {
-                    System::PacketBufferTLVReader bufReader;
-                    bufReader.Init(attributeIter.second.Get<System::PacketBufferHandle>().Retain());
+                    TLV::TLVReader bufReader;
+                    bufReader.Init(attributeIter.second.Get<Platform::ScopedMemoryBuffer<uint8_t>>().Get(), attributeIter.second.Get<Platform::ScopedMemoryBuffer<uint8_t>>().GetSize());
                     ReturnOnFailure(bufReader.Next());
                     // Skip to the end of the element.
                     ReturnOnFailure(bufReader.Skip());

diff --git a/src/app/ClusterStateCache.h b/src/app/ClusterStateCache.h
@@ -236,7 +236,7 @@ class ClusterStateCache : protected ReadClient::Callback
      * current data version for the cluster (which may have no value if we don't have a known data version
      * for it, for example because none of our paths were wildcards that covered the whole cluster).
      */
-    CHIP_ERROR GetVersion(EndpointId mEndpointId, ClusterId mClusterId, Optional<DataVersion> & aVersion);
+    CHIP_ERROR GetVersion(const ConcreteClusterPath & path, Optional<DataVersion> & aVersion);
 
     /*
      * Get highest received event number.
@@ -483,7 +483,7 @@ class ClusterStateCache : protected ReadClient::Callback
     }
 
 private:
-    using AttributeState = Variant<System::PacketBufferHandle, StatusIB>;
+    using AttributeState = Variant<Platform::ScopedMemoryBuffer<uint8_t>, StatusIB>;
     // mPendingDataVersion represents a tentative data version for a cluster that we have gotten some reports for.
     //
     // mCurrentDataVersion represents a known data version for a cluster.  In order for this to have a