Compare commits

..
Author SHA1 Message Date
Niels Lohmann cc749ac699 Keep full byte2 x byte3 combinatorics in the wrong-fourth-byte sections
The maintainer wants exhaustive coverage of every byte combination here
rather than the representative-prefix reduction, matching the style of
the sibling "wrong second/third byte" sections in the same files.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 23:18:51 +02:00
Niels Lohmann 1c6ca81b8a Fix dead ill-formed-fourth-byte UTF-8 test sections (byte3/byte4 typo)
The "ill-formed: wrong fourth byte" SECTIONs in unit-unicode3.cpp,
unit-unicode4.cpp, and unit-unicode5.cpp guarded their loop with a check
on byte3 instead of byte4. Since the enclosing loop already restricts
byte3 to its valid range, the guard was always true and the section's
"continue" fired unconditionally, so check_utf8string()/check_utf8dump()
were never actually invoked for a malformed fourth byte.

Fixing the guard naively (byte3 -> byte4) would also have swept the full
byte2 x byte3 combinatorics for every byte4 value, adding millions of
redundant iterations: the lexer validates continuation bytes strictly in
sequence with early exit (see next_byte_in_range() in lexer.hpp), so once
byte2/byte3 are within their valid range, the byte4 outcome does not
depend on which valid byte2/byte3 values were chosen. Instead, byte2 and
byte3 are now held to a small hedge of representative valid prefixes
(range corners plus a midpoint) while byte4 is still swept exhaustively
over its full 0x00-0xFF range, since that is the actual property under
test. Also fixed the garbled "skip fourth second byte" comment in
unit-unicode3.cpp.

Verified offline: before the fix, the "wrong fourth byte" subcase
executes 0 assertions in all three files (proving it was dead code);
after the fix, it executes 11520 (unicode3), 34560 (unicode4), and 11520
(unicode5) assertions, and a deliberately reintroduced bug in the
lexer's byte4 range check causes it to fail (proving it is now
meaningful). Total per-file assertion counts grow by the same small
amounts, not by millions, and all other sections in these files still
pass unchanged.

Fixes #5416

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 23:05:42 +02:00
6 changed files with 8 additions and 139 deletions
+2 -14
View File
@@ -71,7 +71,7 @@ class serializer
, thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->thousands_sep)))
, decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->decimal_point)))
, indent_char(ichar)
, indent_string()
, indent_string(512, indent_char)
, error_handler(error_handler_)
{}
@@ -126,10 +126,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -203,10 +199,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -268,10 +260,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -1022,7 +1010,7 @@ class serializer
/// the indentation character
const char indent_char;
/// the indentation string (lazily allocated on first use by a pretty-print branch)
/// the indentation string
string_t indent_string;
/// error_handler how to react on decoding errors
+2 -14
View File
@@ -20150,7 +20150,7 @@ class serializer
, thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->thousands_sep)))
, decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->decimal_point)))
, indent_char(ichar)
, indent_string()
, indent_string(512, indent_char)
, error_handler(error_handler_)
{}
@@ -20205,10 +20205,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -20282,10 +20278,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -20347,10 +20339,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -21101,7 +21089,7 @@ class serializer
/// the indentation character
const char indent_char;
/// the indentation string (lazily allocated on first use by a pretty-print branch)
/// the indentation string
string_t indent_string;
/// error_handler how to react on decoding errors
-107
View File
@@ -14,40 +14,6 @@ using nlohmann::json;
#include <array>
#include <sstream>
#include <iomanip>
#include <cstdlib>
#include <new>
namespace
{
// heap allocation counter used by the regression test for issue #5413
// (https://github.com/nlohmann/json/issues/5413); disabled (and thus a
// no-op besides the counting) unless explicitly toggled on
bool count_heap_allocations = false; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
std::size_t heap_allocations = 0; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
} // namespace
void* operator new (std::size_t size) // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
if (count_heap_allocations)
{
++heap_allocations;
}
if (void* ptr = std::malloc(size)) // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
{
return ptr;
}
throw std::bad_alloc(); // NOLINT(hicpp-exception-baseclass)
}
void operator delete (void* ptr) noexcept // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
std::free(ptr); // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
}
void operator delete (void* ptr, std::size_t /*size*/) noexcept // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
std::free(ptr); // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
}
TEST_CASE("serialization")
{
@@ -416,76 +382,3 @@ TEST_CASE("dump for basic_json with long double number_float_t")
check_same(100.0L, 100.0);
}
}
TEST_CASE("regression test for issue #5413 - lazily allocated indent_string")
{
// the serializer used to unconditionally allocate a 512-byte
// indent_string in its constructor, even though it is only ever read
// inside the pretty_print branches of dump(). This wasted a heap
// allocation (and its matching deallocation) on every single compact
// (i.e. non-pretty, the default) dump() call. indent_string is now
// allocated lazily, the first time a pretty-print branch actually
// needs it -- so a compact dump() must perform strictly fewer heap
// allocations than a pretty dump() of the same value.
const json j = {{"level", "info"}, {"msg", "hello world"}, {"id", 12345}};
// warm up anything unrelated to indentation (e.g., one-time locale
// lookups) that might otherwise allocate on first use regardless of
// pretty-printing, so it does not skew the counts measured below
const auto warmup = j.dump();
const auto warmup_pretty = j.dump(4);
CHECK(!warmup.empty());
CHECK(!warmup_pretty.empty());
SECTION("compact dump() has a stable, minimal allocation count")
{
count_heap_allocations = true;
heap_allocations = 0;
const auto compact1 = j.dump();
const auto allocs_compact1 = heap_allocations;
heap_allocations = 0;
const auto compact2 = j.dump(-1);
const auto allocs_compact2 = heap_allocations;
count_heap_allocations = false;
CHECK(compact1 == compact2);
// dump() and dump(-1) both take the compact code path and must
// never touch indent_string, so they allocate identically often
CHECK(allocs_compact1 == allocs_compact2);
}
SECTION("first pretty dump() allocates more than a compact dump()")
{
// use a tiny value whose compact ({"a":1}, 7 bytes) and pretty
// ({"a": 1} with 1-space indent, 11 bytes) serializations both stay
// well inside every common std::string small-string-optimization
// buffer (>= 15 bytes on libstdc++/MSVC STL, >= 22 on libc++), so
// building the result string itself causes no heap allocation
// either way -- isolating indent_string as the only thing that can
// possibly account for a difference in allocation count
const json tiny = {{"a", 1}};
count_heap_allocations = true;
heap_allocations = 0;
const auto compact = tiny.dump();
const auto allocs_compact = heap_allocations;
heap_allocations = 0;
const auto pretty = tiny.dump(1);
const auto allocs_pretty = heap_allocations;
count_heap_allocations = false;
CHECK(compact == "{\"a\":1}");
CHECK(pretty == "{\n \"a\": 1\n}");
// a fresh serializer is created per dump() call; the pretty branch
// lazily allocates indent_string on its first use, so it must
// allocate at least once more than the compact branch, which never
// touches indent_string at all
CHECK(allocs_pretty > allocs_compact);
}
}
+2 -2
View File
@@ -304,8 +304,8 @@ TEST_CASE("Unicode (3/5)" * doctest::skip())
{
for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4)
{
// skip fourth second byte
if (0x80 <= byte3 && byte3 <= 0xBF)
// skip correct fourth byte
if (0x80 <= byte4 && byte4 <= 0xBF)
{
continue;
}
+1 -1
View File
@@ -305,7 +305,7 @@ TEST_CASE("Unicode (4/5)" * doctest::skip())
for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4)
{
// skip correct fourth byte
if (0x80 <= byte3 && byte3 <= 0xBF)
if (0x80 <= byte4 && byte4 <= 0xBF)
{
continue;
}
+1 -1
View File
@@ -305,7 +305,7 @@ TEST_CASE("Unicode (5/5)" * doctest::skip())
for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4)
{
// skip correct fourth byte
if (0x80 <= byte3 && byte3 <= 0xBF)
if (0x80 <= byte4 && byte4 <= 0xBF)
{
continue;
}