Address review comments on nested indefinite-length CBOR strings

Rename is_chunk to inside_indefinite, update the stale test section
names, and use lowercase comments like the surrounding code.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
Niels Lohmann committed 2026-10-06 08:29:13 +02:00
1 parent 54beb8ffae
commit 65bcce35ba
3 files changed
+33 -33

No files matched your search

+14 -14
View File
@@ -1095,13 +1095,13 @@ class binary_reader
into the same string. into the same string.
@param[out] result string the bytes are appended to @param[out] result string the bytes are appended to
@param[in] is_chunk whether the bytes belong to an indefinite-length string @param[in] inside_indefinite whether the bytes belong to an indefinite-length string
@return whether string creation completed @return whether string creation completed
@pre @a current is not EOF @pre @a current is not EOF
*/ */
bool get_cbor_string_chunk(string_t& result, const bool is_chunk) bool get_cbor_string_chunk(string_t& result, const bool inside_indefinite)
{ {
switch (current) switch (current)
{ {
@@ -1162,7 +1162,7 @@ class binary_reader
{ {
auto last_token = get_token_string(); auto last_token = get_token_string();
return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read,
exception_message(concat("expected length specification (0x60-0x7B)", is_chunk ? "" : " or indefinite string type (0x7F)", "; last byte: 0x", last_token), "string"), nullptr)); exception_message(concat("expected length specification (0x60-0x7B)", inside_indefinite ? "" : " or indefinite string type (0x7F)", "; last byte: 0x", last_token), "string"), nullptr));
} }
} }
} }
@@ -1180,8 +1180,8 @@ class binary_reader
*/ */
bool get_cbor_string(string_t& result, const char* context = "string") bool get_cbor_string(string_t& result, const char* context = "string")
{ {
// Read chunks iteratively, but reject a second indefinite-length // read chunks iteratively, but reject a second indefinite-length
// level as required by RFC 8949, Section 3.2.3. // level as required by RFC 8949, Section 3.2.3
bool indefinite = false; bool indefinite = false;
while (true) while (true)
@@ -1202,8 +1202,8 @@ class binary_reader
continue; continue;
} }
// A break marker closes the indefinite-length string; outside // a break marker closes the indefinite-length string; outside
// of one it falls through to the error below. // of one it falls through to the error below
if (indefinite && current == 0xFF) if (indefinite && current == 0xFF)
{ {
return check_string_utf8(result, context); return check_string_utf8(result, context);
@@ -1306,13 +1306,13 @@ class binary_reader
read into the same byte array. read into the same byte array.
@param[out] result byte array the bytes are appended to @param[out] result byte array the bytes are appended to
@param[in] is_chunk whether the bytes belong to an indefinite-length string @param[in] inside_indefinite whether the bytes belong to an indefinite-length string
@return whether byte array creation completed @return whether byte array creation completed
@pre @a current is not EOF @pre @a current is not EOF
*/ */
bool get_cbor_binary_chunk(binary_t& result, const bool is_chunk) bool get_cbor_binary_chunk(binary_t& result, const bool inside_indefinite)
{ {
switch (current) switch (current)
{ {
@@ -1377,7 +1377,7 @@ class binary_reader
{ {
auto last_token = get_token_string(); auto last_token = get_token_string();
return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read,
exception_message(concat("expected length specification (0x40-0x5B)", is_chunk ? "" : " or indefinite binary array type (0x5F)", "; last byte: 0x", last_token), "binary"), nullptr)); exception_message(concat("expected length specification (0x40-0x5B)", inside_indefinite ? "" : " or indefinite binary array type (0x5F)", "; last byte: 0x", last_token), "binary"), nullptr));
} }
} }
} }
@@ -1395,8 +1395,8 @@ class binary_reader
*/ */
bool get_cbor_binary(binary_t& result) bool get_cbor_binary(binary_t& result)
{ {
// Read chunks iteratively, but reject a second indefinite-length // read chunks iteratively, but reject a second indefinite-length
// level as required by RFC 8949, Section 3.2.3. // level as required by RFC 8949, Section 3.2.3
bool indefinite = false; bool indefinite = false;
while (true) while (true)
@@ -1417,8 +1417,8 @@ class binary_reader
continue; continue;
} }
// A break marker closes the indefinite-length string; outside // a break marker closes the indefinite-length string; outside
// of one it falls through to the error below. // of one it falls through to the error below
if (indefinite && current == 0xFF) if (indefinite && current == 0xFF)
{ {
return true; return true;
+14 -14
View File
@@ -14812,13 +14812,13 @@ class binary_reader
into the same string. into the same string.
@param[out] result string the bytes are appended to @param[out] result string the bytes are appended to
@param[in] is_chunk whether the bytes belong to an indefinite-length string @param[in] inside_indefinite whether the bytes belong to an indefinite-length string
@return whether string creation completed @return whether string creation completed
@pre @a current is not EOF @pre @a current is not EOF
*/ */
bool get_cbor_string_chunk(string_t& result, const bool is_chunk) bool get_cbor_string_chunk(string_t& result, const bool inside_indefinite)
{ {
switch (current) switch (current)
{ {
@@ -14879,7 +14879,7 @@ class binary_reader
{ {
auto last_token = get_token_string(); auto last_token = get_token_string();
return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read,
exception_message(concat("expected length specification (0x60-0x7B)", is_chunk ? "" : " or indefinite string type (0x7F)", "; last byte: 0x", last_token), "string"), nullptr)); exception_message(concat("expected length specification (0x60-0x7B)", inside_indefinite ? "" : " or indefinite string type (0x7F)", "; last byte: 0x", last_token), "string"), nullptr));
} }
} }
} }
@@ -14897,8 +14897,8 @@ class binary_reader
*/ */
bool get_cbor_string(string_t& result, const char* context = "string") bool get_cbor_string(string_t& result, const char* context = "string")
{ {
// Read chunks iteratively, but reject a second indefinite-length // read chunks iteratively, but reject a second indefinite-length
// level as required by RFC 8949, Section 3.2.3. // level as required by RFC 8949, Section 3.2.3
bool indefinite = false; bool indefinite = false;
while (true) while (true)
@@ -14919,8 +14919,8 @@ class binary_reader
continue; continue;
} }
// A break marker closes the indefinite-length string; outside // a break marker closes the indefinite-length string; outside
// of one it falls through to the error below. // of one it falls through to the error below
if (indefinite && current == 0xFF) if (indefinite && current == 0xFF)
{ {
return check_string_utf8(result, context); return check_string_utf8(result, context);
@@ -15023,13 +15023,13 @@ class binary_reader
read into the same byte array. read into the same byte array.
@param[out] result byte array the bytes are appended to @param[out] result byte array the bytes are appended to
@param[in] is_chunk whether the bytes belong to an indefinite-length string @param[in] inside_indefinite whether the bytes belong to an indefinite-length string
@return whether byte array creation completed @return whether byte array creation completed
@pre @a current is not EOF @pre @a current is not EOF
*/ */
bool get_cbor_binary_chunk(binary_t& result, const bool is_chunk) bool get_cbor_binary_chunk(binary_t& result, const bool inside_indefinite)
{ {
switch (current) switch (current)
{ {
@@ -15094,7 +15094,7 @@ class binary_reader
{ {
auto last_token = get_token_string(); auto last_token = get_token_string();
return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read,
exception_message(concat("expected length specification (0x40-0x5B)", is_chunk ? "" : " or indefinite binary array type (0x5F)", "; last byte: 0x", last_token), "binary"), nullptr)); exception_message(concat("expected length specification (0x40-0x5B)", inside_indefinite ? "" : " or indefinite binary array type (0x5F)", "; last byte: 0x", last_token), "binary"), nullptr));
} }
} }
} }
@@ -15112,8 +15112,8 @@ class binary_reader
*/ */
bool get_cbor_binary(binary_t& result) bool get_cbor_binary(binary_t& result)
{ {
// Read chunks iteratively, but reject a second indefinite-length // read chunks iteratively, but reject a second indefinite-length
// level as required by RFC 8949, Section 3.2.3. // level as required by RFC 8949, Section 3.2.3
bool indefinite = false; bool indefinite = false;
while (true) while (true)
@@ -15134,8 +15134,8 @@ class binary_reader
continue; continue;
} }
// A break marker closes the indefinite-length string; outside // a break marker closes the indefinite-length string; outside
// of one it falls through to the error below. // of one it falls through to the error below
if (indefinite && current == 0xFF) if (indefinite && current == 0xFF)
{ {
return true; return true;
+5 -5
View File
@@ -2309,14 +2309,14 @@ TEST_CASE("CBOR indefinite-length strings do not recurse per chunk")
// chunks are now rejected at the second byte, without recursing. // chunks are now rejected at the second byte, without recursing.
json _; json _;
SECTION("many open levels are reported, not crashed on") SECTION("nested levels are rejected, not crashed on")
{ {
const std::vector<uint8_t> input(200000, 0x7F); const std::vector<uint8_t> input(200000, 0x7F);
CHECK_THROWS_WITH_AS(_ = json::from_cbor(input), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: indefinite-length string is not allowed inside indefinite-length string; last byte: 0x7F", json::parse_error&); CHECK_THROWS_WITH_AS(_ = json::from_cbor(input), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: indefinite-length string is not allowed inside indefinite-length string; last byte: 0x7F", json::parse_error&);
CHECK(json::from_cbor(input, true, false).is_discarded()); CHECK(json::from_cbor(input, true, false).is_discarded());
} }
SECTION("many open levels are reported, not crashed on (binary)") SECTION("nested levels are rejected, not crashed on (binary)")
{ {
const std::vector<uint8_t> input(200000, 0x5F); const std::vector<uint8_t> input(200000, 0x5F);
CHECK_THROWS_WITH_AS(_ = json::from_cbor(input), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR binary: indefinite-length binary array is not allowed inside indefinite-length binary array; last byte: 0x5F", json::parse_error&); CHECK_THROWS_WITH_AS(_ = json::from_cbor(input), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR binary: indefinite-length binary array is not allowed inside indefinite-length binary array; last byte: 0x5F", json::parse_error&);
@@ -2327,7 +2327,7 @@ TEST_CASE("CBOR indefinite-length strings do not recurse per chunk")
{ {
CHECK(json::from_cbor(std::vector<uint8_t>({0x7F, 0xFF})) == json("")); CHECK(json::from_cbor(std::vector<uint8_t>({0x7F, 0xFF})) == json(""));
CHECK(json::from_cbor(std::vector<uint8_t>({0x7F, 0x61, 0x61, 0xFF})) == json("a")); CHECK(json::from_cbor(std::vector<uint8_t>({0x7F, 0x61, 0x61, 0xFF})) == json("a"));
// Empty and nonempty definite-length chunks concatenate in order. // empty and nonempty definite-length chunks concatenate in order
CHECK(json::from_cbor(std::vector<uint8_t>({0x7F, 0x61, 'a', 0x60, 0x61, 'b', 0x61, 'c', 0xFF})) == json("abc")); CHECK(json::from_cbor(std::vector<uint8_t>({0x7F, 0x61, 'a', 0x60, 0x61, 'b', 0x61, 'c', 0xFF})) == json("abc"));
CHECK(json::from_cbor(std::vector<uint8_t>({0xA1, 0x7F, 0x61, 0x61, 0xFF, 0x01})) == json({{"a", 1}})); CHECK(json::from_cbor(std::vector<uint8_t>({0xA1, 0x7F, 0x61, 0x61, 0xFF, 0x01})) == json({{"a", 1}}));
} }
@@ -2895,10 +2895,10 @@ TEST_CASE("examples from RFC 8949 Appendix A")
{ {
const auto packed = utils::read_binary_file(TEST_DATA_DIRECTORY "/binary_data/cbor_binary.cbor"); const auto packed = utils::read_binary_file(TEST_DATA_DIRECTORY "/binary_data/cbor_binary.cbor");
json j; json j;
// The fixture's tail contains nested indefinite-length byte strings. // the fixture's tail contains nested indefinite-length byte strings.
CHECK_THROWS_WITH_AS(j = json::from_cbor(packed), "[json.exception.parse_error.113] parse error at byte 513: syntax error while parsing CBOR binary: indefinite-length binary array is not allowed inside indefinite-length binary array; last byte: 0x5F", json::parse_error&); CHECK_THROWS_WITH_AS(j = json::from_cbor(packed), "[json.exception.parse_error.113] parse error at byte 513: syntax error while parsing CBOR binary: indefinite-length binary array is not allowed inside indefinite-length binary array; last byte: 0x5F", json::parse_error&);
// Keep the byte-for-byte decoding check for its valid prefix: the first // keep the byte-for-byte decoding check for its valid prefix: the first
// 512 encoded bytes contain 468 payload bytes in definite-length chunks. // 512 encoded bytes contain 468 payload bytes in definite-length chunks.
auto valid_prefix = packed; auto valid_prefix = packed;
valid_prefix.resize(512); valid_prefix.resize(512);