From d9b2f5b68992d088bcea6e4281955b35b397b578 Mon Sep 17 00:00:00 2001 From: Song GUO Date: Fri, 1 Dec 2023 18:18:58 +0800 Subject: [PATCH] [icd] fix DefaultICDClientStorage::LoadFabricList (#30762) --- .../icd/client/DefaultICDClientStorage.cpp | 12 +- src/app/tests/TestDefaultICDClientStorage.cpp | 112 ++++++++++-------- 2 files changed, 69 insertions(+), 55 deletions(-) diff --git a/src/app/icd/client/DefaultICDClientStorage.cpp b/src/app/icd/client/DefaultICDClientStorage.cpp index ce503706846434..b7fa634064826f 100644 --- a/src/app/icd/client/DefaultICDClientStorage.cpp +++ b/src/app/icd/client/DefaultICDClientStorage.cpp @@ -23,6 +23,7 @@ #include #include #include +#include namespace { // FabricIndex is uint8_t, the tlv size with anonymous tag is 1(control bytes) + 1(value) = 2 @@ -31,6 +32,9 @@ constexpr size_t kFabricIndexTlvSize = 2; // The array itself has a control byte and an end-of-array marker. constexpr size_t kArrayOverHead = 2; constexpr size_t kFabricIndexMax = 255; + +constexpr size_t kMaxFabricListTlvLength = kFabricIndexTlvSize * kFabricIndexMax + kArrayOverHead; +static_assert(kMaxFabricListTlvLength <= std::numeric_limits::max(), "Expected size for fabric list TLV is too large!"); } // namespace namespace chip { @@ -72,14 +76,12 @@ CHIP_ERROR DefaultICDClientStorage::UpdateFabricList(FabricIndex fabricIndex) CHIP_ERROR DefaultICDClientStorage::LoadFabricList() { Platform::ScopedMemoryBuffer backingBuffer; - size_t len = kFabricIndexTlvSize * kFabricIndexMax + kArrayOverHead; - VerifyOrReturnError(CanCastTo(len), CHIP_ERROR_BUFFER_TOO_SMALL); - ReturnErrorCodeIf(!backingBuffer.Calloc(len), CHIP_ERROR_NO_MEMORY); - uint16_t length = static_cast(len); + ReturnErrorCodeIf(!backingBuffer.Calloc(kMaxFabricListTlvLength), CHIP_ERROR_NO_MEMORY); + uint16_t length = kMaxFabricListTlvLength; ReturnErrorOnFailure( mpClientInfoStore->SyncGetKeyValue(DefaultStorageKeyAllocator::ICDFabricList().KeyName(), backingBuffer.Get(), length)); - TLV::ScopedBufferTLVReader reader(std::move(backingBuffer), len); + TLV::ScopedBufferTLVReader reader(std::move(backingBuffer), length); ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Array, TLV::AnonymousTag())); TLV::TLVType arrayType; ReturnErrorOnFailure(reader.EnterContainer(arrayType)); diff --git a/src/app/tests/TestDefaultICDClientStorage.cpp b/src/app/tests/TestDefaultICDClientStorage.cpp index ffddf526d763f7..399f7689aee2ef 100644 --- a/src/app/tests/TestDefaultICDClientStorage.cpp +++ b/src/app/tests/TestDefaultICDClientStorage.cpp @@ -57,62 +57,74 @@ void TestClientInfoCount(nlTestSuite * apSuite, void * apContext) FabricIndex fabricId = 1; NodeId nodeId1 = 6666; NodeId nodeId2 = 6667; - DefaultICDClientStorage manager; + TestPersistentStorageDelegate clientInfoStorage; TestSessionKeystoreImpl keystore; - err = manager.Init(&clientInfoStorage, &keystore); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = manager.UpdateFabricList(fabricId); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - // Write some ClientInfos and see the counts are correct - ICDClientInfo clientInfo1; - clientInfo1.peer_node = ScopedNodeId(nodeId1, fabricId); - ICDClientInfo clientInfo2; - clientInfo2.peer_node = ScopedNodeId(nodeId2, fabricId); - ICDClientInfo clientInfo3; - clientInfo3.peer_node = ScopedNodeId(nodeId1, fabricId); - err = manager.SetKey(clientInfo1, ByteSpan(kKeyBuffer1)); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = manager.StoreEntry(clientInfo1); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - - err = manager.SetKey(clientInfo2, ByteSpan(kKeyBuffer2)); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = manager.StoreEntry(clientInfo2); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - - err = manager.SetKey(clientInfo3, ByteSpan(kKeyBuffer3)); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = manager.StoreEntry(clientInfo3); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - // Make sure iterator counts correctly - auto * iterator = manager.IterateICDClientInfo(); - // same nodeId for clientInfo2 and clientInfo3, so the new one replace old one - NL_TEST_ASSERT(apSuite, iterator->Count() == 2); - - ICDClientInfo clientInfo; - NL_TEST_ASSERT(apSuite, iterator->Next(clientInfo)); - NL_TEST_ASSERT(apSuite, clientInfo.peer_node.GetNodeId() == nodeId2); - NL_TEST_ASSERT(apSuite, iterator->Next(clientInfo)); - NL_TEST_ASSERT(apSuite, clientInfo.peer_node.GetNodeId() == nodeId1); - - iterator->Release(); - - // Delete all and verify iterator counts 0 - err = manager.DeleteAllEntries(fabricId); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - iterator = manager.IterateICDClientInfo(); - NL_TEST_ASSERT(apSuite, iterator->Count() == 0); + { + DefaultICDClientStorage manager; + err = manager.Init(&clientInfoStorage, &keystore); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = manager.UpdateFabricList(fabricId); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + // Write some ClientInfos and see the counts are correct + ICDClientInfo clientInfo1; + clientInfo1.peer_node = ScopedNodeId(nodeId1, fabricId); + ICDClientInfo clientInfo2; + clientInfo2.peer_node = ScopedNodeId(nodeId2, fabricId); + ICDClientInfo clientInfo3; + clientInfo3.peer_node = ScopedNodeId(nodeId1, fabricId); + err = manager.SetKey(clientInfo1, ByteSpan(kKeyBuffer1)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = manager.StoreEntry(clientInfo1); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + err = manager.SetKey(clientInfo2, ByteSpan(kKeyBuffer2)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = manager.StoreEntry(clientInfo2); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + err = manager.SetKey(clientInfo3, ByteSpan(kKeyBuffer3)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = manager.StoreEntry(clientInfo3); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + // Make sure iterator counts correctly + auto * iterator = manager.IterateICDClientInfo(); + // same nodeId for clientInfo2 and clientInfo3, so the new one replace old one + NL_TEST_ASSERT(apSuite, iterator->Count() == 2); + + ICDClientInfo clientInfo; + NL_TEST_ASSERT(apSuite, iterator->Next(clientInfo)); + NL_TEST_ASSERT(apSuite, clientInfo.peer_node.GetNodeId() == nodeId2); + NL_TEST_ASSERT(apSuite, iterator->Next(clientInfo)); + NL_TEST_ASSERT(apSuite, clientInfo.peer_node.GetNodeId() == nodeId1); + + iterator->Release(); + + // Delete all and verify iterator counts 0 + err = manager.DeleteAllEntries(fabricId); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + iterator = manager.IterateICDClientInfo(); + NL_TEST_ASSERT(apSuite, iterator->Count() == 0); + + // Verify ClientInfos manually count correctly + size_t count = 0; + while (iterator->Next(clientInfo)) + { + count++; + } + iterator->Release(); + NL_TEST_ASSERT(apSuite, count == 0); + } - // Verify ClientInfos manually count correctly - size_t count = 0; - while (iterator->Next(clientInfo)) { - count++; + DefaultICDClientStorage manager; + err = manager.Init(&clientInfoStorage, &keystore); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = manager.UpdateFabricList(fabricId); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); } - iterator->Release(); - NL_TEST_ASSERT(apSuite, count == 0); } void TestClientInfoCountMultipleFabric(nlTestSuite * apSuite, void * apContext)