Skip to content

Commit

Permalink
Fix incorrect boundary condition for MakeZclCharString. (#9486)
Browse files Browse the repository at this point in the history
A length of 255 is not valid for ZCL strings: it means "invalid string".
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Sep 23, 2021
1 parent ebd0414 commit 2206569
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
7 changes: 4 additions & 3 deletions src/lib/support/ZclString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@

namespace chip {

// ZCL strings are stored as pascal-strings (first byte contains the length of the
// data), so the maximum string length is the maximum of uint8_t
constexpr size_t kBufferMaximumSize = std::numeric_limits<uint8_t>::max();
// ZCL strings are stored as pascal-strings (first byte contains the length of
// the data), and a length of 255 means "invalid string" so the maximum actually
// allowed string length is 254.
constexpr size_t kBufferMaximumSize = 254;

CHIP_ERROR MakeZclCharString(MutableByteSpan & buffer, const char * cString)
{
Expand Down
21 changes: 18 additions & 3 deletions src/lib/support/tests/TestZclString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,21 @@ static void TestZclStringLessThanMaximumSize_Length_64(nlTestSuite * inSuite, vo
}

static void TestZclStringEqualsMaximumSize(nlTestSuite * inSuite, void * inContext)
{
uint8_t bufferMemory[256];
MutableByteSpan zclString(bufferMemory);
chip::Platform::ScopedMemoryBuffer<char> cString254;
NL_TEST_ASSERT(inSuite, cString254.Calloc(1024));
memset(cString254.Get(), 'A', 254);

CHIP_ERROR err = MakeZclCharString(zclString, cString254.Get());

NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, zclString.data()[0] == 254);
NL_TEST_ASSERT(inSuite, allCharactersSame(zclString.data()) == true);
}

static void TestSizeZclStringBiggerThanMaximumSize_Length_255(nlTestSuite * inSuite, void * inContext)
{
uint8_t bufferMemory[256];
MutableByteSpan zclString(bufferMemory);
Expand All @@ -87,9 +102,8 @@ static void TestZclStringEqualsMaximumSize(nlTestSuite * inSuite, void * inConte

CHIP_ERROR err = MakeZclCharString(zclString, cString255.Get());

NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, zclString.data()[0] == 255);
NL_TEST_ASSERT(inSuite, allCharactersSame(zclString.data()) == true);
NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INBOUND_MESSAGE_TOO_BIG);
NL_TEST_ASSERT(inSuite, zclString.data()[0] == 0);
}

static void TestSizeZclStringBiggerThanMaximumSize_Length_256(nlTestSuite * inSuite, void * inContext)
Expand Down Expand Up @@ -148,6 +162,7 @@ int TestZclString_Teardown(void * inContext)
static const nlTest sTests[] = { NL_TEST_DEF_FN(TestZclStringWhenBufferIsZero),
NL_TEST_DEF_FN(TestZclStringLessThanMaximumSize_Length_64),
NL_TEST_DEF_FN(TestZclStringEqualsMaximumSize),
NL_TEST_DEF_FN(TestSizeZclStringBiggerThanMaximumSize_Length_255),
NL_TEST_DEF_FN(TestSizeZclStringBiggerThanMaximumSize_Length_256),
NL_TEST_DEF_FN(TestZclStringBiggerThanMaximumSize_Length_257),
NL_TEST_SENTINEL() };
Expand Down

0 comments on commit 2206569

Please sign in to comment.