From 41679805b0fdfe417ee4c3b8175c0677ab7ca9fc Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 3 May 2023 15:25:16 -0400 Subject: [PATCH] Fix TLVPacketBufferBackingStore's length tracking. (#26343) * Fix TLVPacketBufferBackingStore's length tracking. We need to pass in the head buffer when setting the length of the current buffer, so that the total lengths of the buffers in the chain can be updated properly. Also adds some basic tests for this totally untested class. * Fix build issue due to ambiguous Put calls. * Disable test on nrfconnect: we can't reduce packetbuffer size there. --- src/system/TLVPacketBufferBackingStore.cpp | 2 +- src/system/tests/BUILD.gn | 7 + .../tests/TestTLVPacketBufferBackingStore.cpp | 273 ++++++++++++++++++ 3 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 src/system/tests/TestTLVPacketBufferBackingStore.cpp diff --git a/src/system/TLVPacketBufferBackingStore.cpp b/src/system/TLVPacketBufferBackingStore.cpp index f7a0702755528f..5059f837ff69a3 100644 --- a/src/system/TLVPacketBufferBackingStore.cpp +++ b/src/system/TLVPacketBufferBackingStore.cpp @@ -76,7 +76,7 @@ CHIP_ERROR TLVPacketBufferBackingStore::FinalizeBuffer(chip::TLV::TLVWriter & wr { return CHIP_ERROR_INVALID_ARGUMENT; } - mCurrentBuffer->SetDataLength(static_cast(length)); + mCurrentBuffer->SetDataLength(static_cast(length), mHeadBuffer); return CHIP_NO_ERROR; } diff --git a/src/system/tests/BUILD.gn b/src/system/tests/BUILD.gn index 9d4e1c2b7958f3..3c90b372df69f3 100644 --- a/src/system/tests/BUILD.gn +++ b/src/system/tests/BUILD.gn @@ -35,6 +35,13 @@ chip_test_suite("tests") { test_sources += [ "TestSystemScheduleWork.cpp" ] } + # SystemPacketBuffer on nrfconnect uses LwIP buffers, which ignore the + # requested allocation size and always allocate at max-size. So our test, + # which tries to size-limit the buffers, does not work correctly there. + if (chip_device_platform != "nrfconnect") { + test_sources += [ "TestTLVPacketBufferBackingStore.cpp" ] + } + cflags = [ "-Wconversion" ] public_deps = [ diff --git a/src/system/tests/TestTLVPacketBufferBackingStore.cpp b/src/system/tests/TestTLVPacketBufferBackingStore.cpp new file mode 100644 index 00000000000000..fccced4cfbb4f4 --- /dev/null +++ b/src/system/tests/TestTLVPacketBufferBackingStore.cpp @@ -0,0 +1,273 @@ +/* + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include + +#include +#include +#include +#include +#include + +#include + +using ::chip::Platform::ScopedMemoryBuffer; +using ::chip::System::PacketBuffer; +using ::chip::System::PacketBufferHandle; +using ::chip::System::PacketBufferTLVReader; +using ::chip::System::PacketBufferTLVWriter; +using namespace ::chip; + +namespace { + +class TLVPacketBufferBackingStoreTest +{ +public: + static int TestSetup(void * inContext); + static int TestTeardown(void * inContext); + + static void BasicEncodeDecode(nlTestSuite * inSuite, void * inContext); + static void MultiBufferEncode(nlTestSuite * inSuite, void * inContext); +}; + +int TLVPacketBufferBackingStoreTest::TestSetup(void * inContext) +{ + chip::Platform::MemoryInit(); + + return SUCCESS; +} + +int TLVPacketBufferBackingStoreTest::TestTeardown(void * inContext) +{ + chip::Platform::MemoryShutdown(); + + return SUCCESS; +} + +/** + * Test that we can do a basic encode to TLV followed by decode. + */ +void TLVPacketBufferBackingStoreTest::BasicEncodeDecode(nlTestSuite * inSuite, void * inContext) +{ + auto buffer = PacketBufferHandle::New(PacketBuffer::kMaxSizeWithoutReserve, 0); + + PacketBufferTLVWriter writer; + writer.Init(std::move(buffer)); + + TLV::TLVType outerContainerType; + CHIP_ERROR error = writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Array, outerContainerType); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Put(TLV::AnonymousTag(), static_cast(7)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Put(TLV::AnonymousTag(), static_cast(8)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Put(TLV::AnonymousTag(), static_cast(9)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.EndContainer(outerContainerType); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Finalize(&buffer); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + // Array start/end is 2 bytes. Each entry is also 2 bytes: control + + // value. So 8 bytes total. + NL_TEST_ASSERT(inSuite, !buffer->HasChainedBuffer()); + NL_TEST_ASSERT(inSuite, buffer->TotalLength() == 8); + NL_TEST_ASSERT(inSuite, buffer->DataLength() == 8); + + PacketBufferTLVReader reader; + reader.Init(std::move(buffer)); + + error = reader.Next(TLV::kTLVType_Array, TLV::AnonymousTag()); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.EnterContainer(outerContainerType); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.Next(TLV::kTLVType_UnsignedInteger, TLV::AnonymousTag()); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + uint8_t value; + error = reader.Get(value); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, value == 7); + + error = reader.Next(TLV::kTLVType_UnsignedInteger, TLV::AnonymousTag()); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.Get(value); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, value == 8); + + error = reader.Next(TLV::kTLVType_UnsignedInteger, TLV::AnonymousTag()); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.Get(value); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, value == 9); + + error = reader.Next(); + NL_TEST_ASSERT(inSuite, error == CHIP_END_OF_TLV); + + error = reader.ExitContainer(outerContainerType); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.Next(); + NL_TEST_ASSERT(inSuite, error == CHIP_END_OF_TLV); +} + +/** + * Test that we can do an encode that's going to split across multiple buffers correctly. + */ +void TLVPacketBufferBackingStoreTest::MultiBufferEncode(nlTestSuite * inSuite, void * inContext) +{ + // Start with a too-small buffer. + auto buffer = PacketBufferHandle::New(2, 0); + + PacketBufferTLVWriter writer; + writer.Init(std::move(buffer), /* useChainedBuffers = */ true); + + TLV::TLVType outerContainerType; + CHIP_ERROR error = writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Array, outerContainerType); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Put(TLV::AnonymousTag(), static_cast(7)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Put(TLV::AnonymousTag(), static_cast(8)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + // Something to make sure we have 3 buffers. + uint8_t bytes[2000] = { 0 }; + error = writer.Put(TLV::AnonymousTag(), ByteSpan(bytes)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.EndContainer(outerContainerType); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Finalize(&buffer); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + // Array start/end is 2 bytes. First two entries are 2 bytes each. + // Third entry is 1 control byte, 2 length bytes, 2000 bytes of data, + // for a total of 2009 bytes. + constexpr size_t totalSize = 2009; + NL_TEST_ASSERT(inSuite, buffer->HasChainedBuffer()); + NL_TEST_ASSERT(inSuite, buffer->TotalLength() == totalSize); + NL_TEST_ASSERT(inSuite, buffer->DataLength() == 2); + auto nextBuffer = buffer->Next(); + NL_TEST_ASSERT(inSuite, nextBuffer->HasChainedBuffer()); + NL_TEST_ASSERT(inSuite, nextBuffer->TotalLength() == totalSize - 2); + NL_TEST_ASSERT(inSuite, nextBuffer->DataLength() == PacketBuffer::kMaxSizeWithoutReserve); + nextBuffer = nextBuffer->Next(); + NL_TEST_ASSERT(inSuite, !nextBuffer->HasChainedBuffer()); + NL_TEST_ASSERT(inSuite, nextBuffer->TotalLength() == nextBuffer->DataLength()); + NL_TEST_ASSERT(inSuite, nextBuffer->DataLength() == totalSize - 2 - PacketBuffer::kMaxSizeWithoutReserve); + + // PacketBufferTLVReader cannot handle non-contiguous buffers, and our + // buffers are too big to stick into a single packet buffer. + ScopedMemoryBuffer buf; + NL_TEST_ASSERT(inSuite, buf.Calloc(totalSize)); + size_t offset = 0; + while (!buffer.IsNull()) + { + memcpy(buf.Get() + offset, buffer->Start(), buffer->DataLength()); + offset += buffer->DataLength(); + buffer.Advance(); + NL_TEST_ASSERT(inSuite, offset < totalSize || (offset == totalSize && buffer.IsNull())); + } + + TLV::TLVReader reader; + reader.Init(buf.Get(), totalSize); + + error = reader.Next(TLV::kTLVType_Array, TLV::AnonymousTag()); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.EnterContainer(outerContainerType); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.Next(TLV::kTLVType_UnsignedInteger, TLV::AnonymousTag()); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + uint8_t value; + error = reader.Get(value); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, value == 7); + + error = reader.Next(TLV::kTLVType_UnsignedInteger, TLV::AnonymousTag()); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.Get(value); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, value == 8); + + error = reader.Next(TLV::kTLVType_ByteString, TLV::AnonymousTag()); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + ByteSpan byteValue; + error = reader.Get(byteValue); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, byteValue.size() == sizeof(bytes)); + + error = reader.Next(); + NL_TEST_ASSERT(inSuite, error == CHIP_END_OF_TLV); + + error = reader.ExitContainer(outerContainerType); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = reader.Next(); + NL_TEST_ASSERT(inSuite, error == CHIP_END_OF_TLV); +} + +/** + * Test Suite. It lists all the test functions. + */ +// clang-format off +const nlTest sTests[] = +{ + NL_TEST_DEF("BasicEncodeDecode", TLVPacketBufferBackingStoreTest::BasicEncodeDecode), + NL_TEST_DEF("MultiBufferEncode", TLVPacketBufferBackingStoreTest::MultiBufferEncode), + + NL_TEST_SENTINEL() +}; +// clang-format on + +} // anonymous namespace + +int TestTLVPacketBufferBackingStore() +{ + // clang-format off + nlTestSuite theSuite = { + .name ="chip-tlv-packet-buffer-backing-store", + .tests = &sTests[0], + .setup = TLVPacketBufferBackingStoreTest::TestSetup, + .tear_down = TLVPacketBufferBackingStoreTest::TestTeardown, + }; + // clang-format on + + // Run test suite. + nlTestRunner(&theSuite, nullptr); + + return (nlTestRunnerStats(&theSuite)); +} + +CHIP_REGISTER_TEST_SUITE(TestTLVPacketBufferBackingStore)