Simplify TLV <-> JSON parsing (#32342)
* Simplify TLV <-> JSON parsing
- Remove error-prone/repeated `strtol` usage, replace with std::from_chars
- Remove needless validation logic done de-facto by other parts
Fixes #32341
Testing done:
- Added test cases for edges of octet strings.
- All existing test cases pass otherwise.
* Restyled by clang-format
* Fix build where CHIP_ERROR doesn't format as string
* Fix CI again
* Simplify further
* Restyled by clang-format
* Fix lint
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/scripts/tools/check_includes_config.py b/scripts/tools/check_includes_config.py
index 6943c26..f471588 100644
--- a/scripts/tools/check_includes_config.py
+++ b/scripts/tools/check_includes_config.py
@@ -164,7 +164,7 @@
'src/app/PendingResponseTrackerImpl.h': {'unordered_set'},
# Not intended for embedded clients
- 'src/lib/support/jsontlv/JsonToTlv.cpp': {'sstream'},
+ 'src/lib/support/jsontlv/JsonToTlv.cpp': {'sstream', 'string', 'vector'},
'src/lib/support/jsontlv/JsonToTlv.h': {'string'},
'src/lib/support/jsontlv/TlvToJson.h': {'string'},
'src/lib/support/jsontlv/TextFormat.h': {'string'},
diff --git a/src/lib/support/jsontlv/JsonToTlv.cpp b/src/lib/support/jsontlv/JsonToTlv.cpp
index 0cc8161..11bfbca 100644
--- a/src/lib/support/jsontlv/JsonToTlv.cpp
+++ b/src/lib/support/jsontlv/JsonToTlv.cpp
@@ -15,8 +15,14 @@
* limitations under the License.
*/
+#include <stdint.h>
+
#include <algorithm>
-#include <errno.h>
+#include <charconv>
+#include <sstream>
+#include <string>
+#include <vector>
+
#include <json/json.h>
#include <lib/support/Base64.h>
#include <lib/support/SafeInt.h>
@@ -98,69 +104,6 @@
return CHIP_NO_ERROR;
}
-bool IsUnsignedInteger(const std::string & s)
-{
- size_t len = s.length();
- if (len == 0)
- {
- return false;
- }
- for (size_t i = 0; i < len; i++)
- {
- if (!isdigit(s[i]))
- {
- return false;
- }
- }
- return true;
-}
-
-bool IsSignedInteger(const std::string & s)
-{
- if (s.length() == 0)
- {
- return false;
- }
- if (s[0] == '-')
- {
- return IsUnsignedInteger(s.substr(1));
- }
- return IsUnsignedInteger(s);
-}
-
-bool IsValidBase64String(const std::string & s)
-{
- const std::string base64Chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
- size_t len = s.length();
-
- // Check if the length is a multiple of 4
- if (len % 4 != 0)
- {
- return false;
- }
-
- size_t paddingLen = 0;
- if (s[len - 1] == '=')
- {
- paddingLen++;
- if (s[len - 2] == '=')
- {
- paddingLen++;
- }
- }
-
- // Check for invalid characters
- for (char c : s.substr(0, len - paddingLen))
- {
- if (base64Chars.find(c) == std::string::npos)
- {
- return false;
- }
- }
-
- return true;
-}
-
struct ElementContext
{
std::string jsonName;
@@ -205,7 +148,17 @@
return CHIP_NO_ERROR;
}
-CHIP_ERROR ParseJsonName(const std::string name, ElementContext & elementCtx, uint32_t implicitProfileId)
+template <typename T>
+CHIP_ERROR ParseNumericalField(const std::string & decimalString, T & outValue)
+{
+ const char * start_ptr = decimalString.data();
+ const char * end_ptr = decimalString.data() + decimalString.size();
+ auto [last_converted_ptr, _] = std::from_chars(start_ptr, end_ptr, outValue, 10);
+ VerifyOrReturnError(last_converted_ptr == end_ptr, CHIP_ERROR_INVALID_ARGUMENT);
+ return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR ParseJsonName(const std::string & name, ElementContext & elementCtx, uint32_t implicitProfileId)
{
uint32_t tagNumber = 0;
const char * elementType = nullptr;
@@ -216,28 +169,12 @@
if (nameFields.size() == 2)
{
- VerifyOrReturnError(IsUnsignedInteger(nameFields[0]), CHIP_ERROR_INVALID_ARGUMENT);
-
- char * endPtr;
- errno = 0;
- unsigned long result = strtoul(nameFields[0].c_str(), &endPtr, 10);
- VerifyOrReturnError(nameFields[0].c_str() != endPtr, CHIP_ERROR_INVALID_ARGUMENT);
- VerifyOrReturnError((errno != ERANGE && result <= UINT32_MAX), CHIP_ERROR_INVALID_ARGUMENT);
-
- tagNumber = static_cast<uint32_t>(result);
+ ReturnErrorOnFailure(ParseNumericalField(nameFields[0], tagNumber));
elementType = nameFields[1].c_str();
}
else if (nameFields.size() == 3)
{
- VerifyOrReturnError(IsUnsignedInteger(nameFields[1]), CHIP_ERROR_INVALID_ARGUMENT);
-
- char * endPtr;
- errno = 0;
- unsigned long result = strtoul(nameFields[1].c_str(), &endPtr, 10);
- VerifyOrReturnError(nameFields[1].c_str() != endPtr, CHIP_ERROR_INVALID_ARGUMENT);
- VerifyOrReturnError((errno != ERANGE && result <= UINT32_MAX), CHIP_ERROR_INVALID_ARGUMENT);
-
- tagNumber = static_cast<uint32_t>(result);
+ ReturnErrorOnFailure(ParseNumericalField(nameFields[1], tagNumber));
elementType = nameFields[2].c_str();
}
else
@@ -278,16 +215,14 @@
switch (elementCtx.type.tlvType)
{
case TLV::kTLVType_UnsignedInteger: {
- uint64_t v;
+ uint64_t v = 0;
if (val.isUInt64())
{
v = val.asUInt64();
}
else if (val.isString())
{
- const std::string valAsString = val.asString();
- VerifyOrReturnError(IsUnsignedInteger(valAsString), CHIP_ERROR_INVALID_ARGUMENT);
- v = std::strtoull(valAsString.c_str(), nullptr, 10);
+ ReturnErrorOnFailure(ParseNumericalField(val.asString(), v));
}
else
{
@@ -298,16 +233,14 @@
}
case TLV::kTLVType_SignedInteger: {
- int64_t v;
+ int64_t v = 0;
if (val.isInt64())
{
v = val.asInt64();
}
else if (val.isString())
{
- const std::string valAsString = val.asString();
- VerifyOrReturnError(IsSignedInteger(valAsString), CHIP_ERROR_INVALID_ARGUMENT);
- v = std::strtoll(valAsString.c_str(), nullptr, 10);
+ ReturnErrorOnFailure(ParseNumericalField(val.asString(), v));
}
else
{
@@ -377,20 +310,23 @@
size_t encodedLen = valAsString.length();
VerifyOrReturnError(CanCastTo<uint16_t>(encodedLen), CHIP_ERROR_INVALID_ARGUMENT);
- VerifyOrReturnError(IsValidBase64String(valAsString), CHIP_ERROR_INVALID_ARGUMENT);
+ // Check if the length is a multiple of 4 as strict padding is required.
+ VerifyOrReturnError(encodedLen % 4 == 0, CHIP_ERROR_INVALID_ARGUMENT);
Platform::ScopedMemoryBuffer<uint8_t> byteString;
byteString.Alloc(BASE64_MAX_DECODED_LEN(static_cast<uint16_t>(encodedLen)));
VerifyOrReturnError(byteString.Get() != nullptr, CHIP_ERROR_NO_MEMORY);
auto decodedLen = Base64Decode(valAsString.c_str(), static_cast<uint16_t>(encodedLen), byteString.Get());
+ VerifyOrReturnError(decodedLen < UINT16_MAX, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(writer.PutBytes(tag, byteString.Get(), decodedLen));
break;
}
case TLV::kTLVType_UTF8String: {
VerifyOrReturnError(val.isString(), CHIP_ERROR_INVALID_ARGUMENT);
- ReturnErrorOnFailure(writer.PutString(tag, val.asCString()));
+ const std::string valAsString = val.asString();
+ ReturnErrorOnFailure(writer.PutString(tag, valAsString.data(), static_cast<uint32_t>(valAsString.size())));
break;
}
diff --git a/src/lib/support/tests/TestJsonToTlv.cpp b/src/lib/support/tests/TestJsonToTlv.cpp
index 539d272..3649764 100644
--- a/src/lib/support/tests/TestJsonToTlv.cpp
+++ b/src/lib/support/tests/TestJsonToTlv.cpp
@@ -151,6 +151,12 @@
"}\n";
ConvertJsonToTlvAndValidate(byteSpan, jsonString);
+ // Empty bytes.
+ jsonString = "{\n"
+ " \"1:BYTES\" : \"\"\n"
+ "}\n";
+ ConvertJsonToTlvAndValidate(ByteSpan{}, jsonString);
+
DataModel::Nullable<uint8_t> nullValue;
jsonString = "{\n"
" \"1:NULL\" : null\n"
diff --git a/src/lib/support/tests/TestJsonToTlvToJson.cpp b/src/lib/support/tests/TestJsonToTlvToJson.cpp
index 5762034..b4f0a13 100644
--- a/src/lib/support/tests/TestJsonToTlvToJson.cpp
+++ b/src/lib/support/tests/TestJsonToTlvToJson.cpp
@@ -15,6 +15,8 @@
* limitations under the License.
*/
+#include <stdio.h>
+
#include <app-common/zap-generated/cluster-objects.h>
#include <app/data-model/Decode.h>
#include <app/data-model/Encode.h>
@@ -328,6 +330,29 @@
CheckValidConversion(jsonString, tlvSpan, jsonString);
}
+// Octet String, empty
+void TestConverter_OctetString_Empty(nlTestSuite * inSuite, void * inContext)
+{
+ gSuite = inSuite;
+
+ uint8_t buf[256];
+ TLV::TLVWriter writer;
+ TLV::TLVType containerType;
+
+ writer.Init(buf);
+ NL_TEST_ASSERT(gSuite, CHIP_NO_ERROR == writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, containerType));
+ NL_TEST_ASSERT(gSuite, CHIP_NO_ERROR == writer.PutBytes(TLV::ContextTag(1), nullptr, 0));
+ NL_TEST_ASSERT(gSuite, CHIP_NO_ERROR == writer.EndContainer(containerType));
+ NL_TEST_ASSERT(gSuite, CHIP_NO_ERROR == writer.Finalize());
+
+ std::string jsonString = "{\n"
+ " \"1:BYTES\" : \"\"\n"
+ "}\n";
+
+ ByteSpan tlvSpan(buf, writer.GetLengthWritten());
+ CheckValidConversion(jsonString, tlvSpan, jsonString);
+}
+
// Null
void TestConverter_Null(nlTestSuite * inSuite, void * inContext)
{
@@ -1794,6 +1819,22 @@
" \"1:BYTES\" : \"AAECwQ=\"\n"
"}\n";
+ std::string invalidBytesBase64Value4 = "{\n"
+ " \"1:BYTES\" : \"AAECwQ\"\n"
+ "}\n";
+
+ std::string invalidBytesBase64Padding1 = "{\n"
+ " \"1:BYTES\" : \"=\"\n"
+ "}\n";
+
+ std::string invalidBytesBase64Padding2 = "{\n"
+ " \"1:BYTES\" : \"==\"\n"
+ "}\n";
+
+ std::string invalidBytesBase64Padding3 = "{\n"
+ " \"1:BYTES\" : \"===\"\n"
+ "}\n";
+
std::string invalidPositiveInfinityValue = "{\n"
" \"1:DOUBLE\" : \"+Infinity\"\n"
"}\n";
@@ -1815,7 +1856,11 @@
{ invalidNameWithInvalidTypeField, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Name With Invalid Type Field" },
{ invalidBytesBase64Value1, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Base64 Encoding: Invalid Character" },
{ invalidBytesBase64Value2, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Base64 Encoding: Invalid Character" },
- { invalidBytesBase64Value3, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Base64 Encoding: Invalid length" },
+ { invalidBytesBase64Value3, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Base64 Encoding: Invalid padding (missing 1)" },
+ { invalidBytesBase64Value4, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Base64 Encoding: Invalid padding (missing 2)" },
+ { invalidBytesBase64Padding1, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Base64 Encoding: Invalid padding (start 1)" },
+ { invalidBytesBase64Padding2, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Base64 Encoding: Invalid padding (start 2)" },
+ { invalidBytesBase64Padding3, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Base64 Encoding: Invalid padding (start 3)" },
{ invalidPositiveInfinityValue, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Double Positive Infinity Encoding" },
{ invalidFloatValueAsString, CHIP_ERROR_INVALID_ARGUMENT, "Invalid Float Value Encoding as a String" },
};
@@ -1827,6 +1872,16 @@
MutableByteSpan tlvSpan(buf);
err = JsonToTlv(testCase.mJsonString, tlvSpan);
NL_TEST_ASSERT(inSuite, err == testCase.mExpectedResult);
+#if CHIP_CONFIG_ERROR_FORMAT_AS_STRING
+ if (err != testCase.mExpectedResult)
+ {
+ std::string errStr{ err.Format() };
+ std::string expectedErrStr{ testCase.mExpectedResult.Format() };
+
+ printf("Case: %s, Error: %" CHIP_ERROR_FORMAT ", Expected: %" CHIP_ERROR_FORMAT ", Data: %s\n", testCase.mNameString,
+ errStr.c_str(), expectedErrStr.c_str(), testCase.mJsonString.c_str());
+ }
+#endif // CHIP_CONFIG_ERROR_FORMAT_AS_STRING
}
}
@@ -1887,6 +1942,7 @@
NL_TEST_DEF("Test Json Tlv Converter - Unsigned Integer 8-Bytes", TestConverter_UnsignedInt_8Bytes),
NL_TEST_DEF("Test Json Tlv Converter - UTF-8 String Hello!", TestConverter_UTF8String_Hello),
NL_TEST_DEF("Test Json Tlv Converter - Octet String", TestConverter_OctetString),
+ NL_TEST_DEF("Test Json Tlv Converter - Empty Octet String", TestConverter_OctetString_Empty),
NL_TEST_DEF("Test Json Tlv Converter - Null", TestConverter_Null),
NL_TEST_DEF("Test Json Tlv Converter - Floating Point Single Precision 0.0", TestConverter_Float_0),
NL_TEST_DEF("Test Json Tlv Converter - Floating Point Single Precision 1/3", TestConverter_Float_1third),
