Split the write-buffer helpers and write indentation directly

Follow-up to @gregmarr's review: put_chars() was doing four unrelated jobs, so
give the two that can be made safe their own entry points.

- put_literal(): takes the literal by reference and deduces the length from the
  array bound, so the 27 hand-counted lengths at the call sites can no longer
  drift from the literals they describe. A literal is checked at compile time to
  fit the buffer, so this path needs no write-through branch.

- put_buffer(): takes the fixed-size buffer itself rather than a bare pointer,
  so the length can be checked against the buffer's own bound.

- put_indent(): memsets the indentation into the write buffer, filling and
  flushing it as needed. This removes indent_string entirely, and with it both
  bugs of #5186: the indentation string was grown by doubling, which is not
  enough when indent_step more than doubles it (a heap over-read - dump(2000)
  read 2000 bytes out of a 1024-byte string), and the grown part was filled with
  a space instead of the configured indent_char. next_indent() keeps that PR's
  assertion against the unsigned indentation accumulation wrapping on deep
  nesting.

put_chars() keeps the two cases that are genuinely a pointer and a count: the
run-length copies out of the string being escaped, and to_chars() output.

Tests cover an indent_step wider than the write buffer, a non-space indentation
character past the old growth point, and nesting whose accumulated indentation
spans several buffer-fulls. All three fail against develop.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
Niels Lohmann
2026-08-19 22:43:17 +02:00
parent eef8e07386
commit a1e91474a8
3 changed files with 296 additions and 130 deletions
+48
View File
@@ -470,3 +470,51 @@ TEST_CASE("serialization of strings (bulk fast path)")
CHECK(j.dump(-1, ' ', false, json::error_handler_t::ignore) == "\"validmore\"");
}
}
TEST_CASE("indentation is written straight into the write buffer")
{
// put_indent() memsets the indentation into the write buffer instead of
// copying it out of a pre-grown indentation string. These cases cover an
// indentation wider than the buffer, a non-space indentation character, and
// nesting deep enough that the accumulated indentation spans several
// buffer-fulls - the situations the old grow-a-string approach got wrong.
SECTION("indent_step wider than the write buffer")
{
const json j = {{"a", 1}};
// 2000 > the 1024-byte write buffer, and > the 512 the indentation
// string used to start at
CHECK(j.dump(2000) == "{\n" + std::string(2000, ' ') + "\"a\": 1\n}");
}
SECTION("a non-space indentation character is used throughout")
{
const json j = {{"a", 1}};
// 600 is past the point where the indentation used to be grown, which
// is where a hard-coded space would have shown up
CHECK(j.dump(600, '\t') == "{\n" + std::string(600, '\t') + "\"a\": 1\n}");
CHECK(j.dump(3, '.') == "{\n...\"a\": 1\n}");
}
SECTION("accumulated indentation spans several buffer-fulls")
{
// five levels deep at 400 per level: the innermost value is indented by
// 2000 characters, reached in steps that each straddle the buffer end
json j = json::array({1});
for (int i = 0; i < 4; ++i)
{
j = json::array({j});
}
const std::string out = j.dump(400);
CHECK(out.find(std::string("\n") + std::string(2000, ' ') + "1\n") != std::string::npos);
CHECK(json::parse(out) == j);
}
SECTION("indentation is unchanged for ordinary widths")
{
const json j = {{"a", {1, 2}}, {"b", nullptr}};
CHECK(j.dump(2) == "{\n \"a\": [\n 1,\n 2\n ],\n \"b\": null\n}");
CHECK(j.dump(0) == "{\n\"a\": [\n1,\n2\n],\n\"b\": null\n}");
}
}