[tlv] TLVReader::Get CharSpan to Stop on ASCII Unit Separator (#24421)

* [tlv] TLVReader::Get CharSpan to Stop on ASCII Unit Separator The TLVReader::Get(CharSpan&) always stops at the first IS1 codepoint (ascii 0x1F). The rationale is that clients should never see garbage at the end of string due to localization IDs. * Addressed review comments * unitSeparator --> infoSeparator
project-chip · Sep 21, 2023 · 5219214 · 5219214
1 parent c6b06ee
commit 5219214
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 3 deletions.
diff --git a/src/lib/core/TLVReader.cpp b/src/lib/core/TLVReader.cpp
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020-2021 Project CHIP Authors
+ *    Copyright (c) 2020-2023 Project CHIP Authors
  *    Copyright (c) 2013-2017 Nest Labs, Inc.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -300,14 +300,27 @@ CHIP_ERROR TLVReader::Get(ByteSpan & v)
 
 CHIP_ERROR TLVReader::Get(CharSpan & v)
 {
+    constexpr int kUnicodeInformationSeparator1 = 0x1F;
+
     if (!TLVTypeIsUTF8String(ElementType()))
     {
         return CHIP_ERROR_WRONG_TLV_TYPE;
     }
 
     const uint8_t * bytes;
     ReturnErrorOnFailure(GetDataPtr(bytes)); // Does length sanity checks
-    v = CharSpan(Uint8::to_const_char(bytes), GetLength());
+
+    uint32_t len = GetLength();
+
+    // If Unicode Information Separator 1 (0x1f) is present in the string then method returns
+    // string ending at first appearance of the Information Separator 1.
+    const uint8_t * infoSeparator = reinterpret_cast<const uint8_t *>(memchr(bytes, kUnicodeInformationSeparator1, len));
+    if (infoSeparator != nullptr)
+    {
+        len = static_cast<uint32_t>(infoSeparator - bytes);
+    }
+
+    v = CharSpan(Uint8::to_const_char(bytes), len);
     return CHIP_NO_ERROR;
 }
 

diff --git a/src/lib/core/tests/TestTLV.cpp b/src/lib/core/tests/TestTLV.cpp
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020-2022 Project CHIP Authors
+ *    Copyright (c) 2020-2023 Project CHIP Authors
  *    Copyright (c) 2013-2017 Nest Labs, Inc.
  *    All rights reserved.
  *
@@ -2789,6 +2789,53 @@ void CheckTLVByteSpan(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, memcmp(readerSpan.data(), bytesBuffer, sizeof(bytesBuffer)) == 0);
 }
 
+void CheckTLVCharSpan(nlTestSuite * inSuite, void * inContext)
+{
+    struct CharSpanTestCase
+    {
+        const char * testString;
+        const char * expectedString;
+    };
+
+    // clang-format off
+    static CharSpanTestCase sCharSpanTestCases[] = {
+        // Test String                                                   Expected String from Get()
+        // =========================================================================================
+        {  "This is a test case #0",                                     "This is a test case #0"  },
+        {  "This is a test case #1\x1fTest Localized String Identifier", "This is a test case #1"  },
+        {  "This is a test case #2 \x1f abc \x1f def",                   "This is a test case #2 " },
+        {  "This is a test case #3\x1f",                                 "This is a test case #3"  },
+    };
+    // clang-format on
+
+    for (auto & testCase : sCharSpanTestCases)
+    {
+        uint8_t backingStore[100];
+        TLVWriter writer;
+        TLVReader reader;
+        CHIP_ERROR err = CHIP_NO_ERROR;
+
+        writer.Init(backingStore);
+
+        err = writer.PutString(ProfileTag(TestProfile_1, 1), testCase.testString);
+        NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+        err = writer.Finalize();
+        NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+        reader.Init(backingStore, writer.GetLengthWritten());
+        err = reader.Next();
+        NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+        chip::CharSpan readerSpan;
+        err = reader.Get(readerSpan);
+        NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+        NL_TEST_ASSERT(inSuite, strlen(testCase.expectedString) == readerSpan.size());
+        NL_TEST_ASSERT(inSuite, memcmp(readerSpan.data(), testCase.expectedString, strlen(testCase.expectedString)) == 0);
+    }
+}
+
 void CheckTLVSkipCircular(nlTestSuite * inSuite, void * inContext)
 {
     const size_t bufsize = 40; // large enough s.t. 2 elements fit, 3rd causes eviction
@@ -4466,6 +4513,7 @@ static const nlTest sTests[] =
     NL_TEST_DEF("CHIP TLV Printf, Circular TLV buf",   CheckTLVPutStringFCircular),
     NL_TEST_DEF("CHIP TLV Skip non-contiguous",        CheckTLVSkipCircular),
     NL_TEST_DEF("CHIP TLV ByteSpan",                   CheckTLVByteSpan),
+    NL_TEST_DEF("CHIP TLV CharSpan",                   CheckTLVCharSpan),
     NL_TEST_DEF("CHIP TLV Scoped Buffer",              CheckTLVScopedBuffer),
     NL_TEST_DEF("CHIP TLV Check reserve",              CheckCloseContainerReserve),
     NL_TEST_DEF("CHIP TLV Reader Fuzz Test",           TLVReaderFuzzTest),