Skip to content

Commit

Permalink
Simplify TLV <-> JSON parsing (#32342)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@tcarmelveilleux @restyled-commits
tcarmelveilleux and restyled-commits authored Feb 28, 2024
1 parent 1c2f889 commit d8edd82
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 95 deletions.
2 changes: 1 addition & 1 deletion scripts/tools/check_includes_config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'},
Expand Down
122 changes: 29 additions & 93 deletions src/lib/support/jsontlv/JsonToTlv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -98,69 +104,6 @@ CHIP_ERROR JsonTypeStrToTlvType(const char * elementType, ElementTypeContext & t
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;
Expand Down Expand Up @@ -205,7 +148,17 @@ CHIP_ERROR InternalConvertTlvTag(uint32_t tagNumber, TLV::Tag & tag, const uint3
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;
Expand All @@ -216,28 +169,12 @@ CHIP_ERROR ParseJsonName(const std::string name, ElementContext & elementCtx, ui

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
Expand Down Expand Up @@ -278,16 +215,14 @@ CHIP_ERROR EncodeTlvElement(const Json::Value & val, TLV::TLVWriter & writer, co
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
{
Expand All @@ -298,16 +233,14 @@ CHIP_ERROR EncodeTlvElement(const Json::Value & val, TLV::TLVWriter & writer, co
}

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
{
Expand Down Expand Up @@ -377,20 +310,23 @@ CHIP_ERROR EncodeTlvElement(const Json::Value & val, TLV::TLVWriter & writer, co
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;
}

Expand Down
6 changes: 6 additions & 0 deletions src/lib/support/tests/TestJsonToTlv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ void TestConverter(nlTestSuite * inSuite, void * inContext)
"}\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"
Expand Down
58 changes: 57 additions & 1 deletion src/lib/support/tests/TestJsonToTlvToJson.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -328,6 +330,29 @@ void TestConverter_OctetString(nlTestSuite * inSuite, void * inContext)
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)
{
Expand Down Expand Up @@ -1794,6 +1819,22 @@ void TestConverter_JsonToTlv_ErrorCases(nlTestSuite * inSuite, void * inContext)
" \"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";
Expand All @@ -1815,7 +1856,11 @@ void TestConverter_JsonToTlv_ErrorCases(nlTestSuite * inSuite, void * inContext)
{ 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" },
};
Expand All @@ -1827,6 +1872,16 @@ void TestConverter_JsonToTlv_ErrorCases(nlTestSuite * inSuite, void * inContext)
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
}
}

Expand Down Expand Up @@ -1887,6 +1942,7 @@ const nlTest sTests[] = {
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),
Expand Down

0 comments on commit d8edd82

Please sign in to comment.