diff --git a/docs/mkdocs/docs/api/basic_json_document/index.md b/docs/mkdocs/docs/api/basic_json_document/index.md index 9b1cb35d7..3d7f41aad 100644 --- a/docs/mkdocs/docs/api/basic_json_document/index.md +++ b/docs/mkdocs/docs/api/basic_json_document/index.md @@ -111,8 +111,9 @@ points apply to every edit: the end, and, with [`number_format::source`](../basic_json_view/number_format.md), keeps the spelling of every number that was not itself edited — see [Editing a document](../../features/json_view/editing.md) for why this matters. -- [`read()`](read.md) discards all edits, [`shrink_to_fit()`](shrink_to_fit.md) does not move the node index once - there are edits, and [`memory_usage()`](memory_usage.md) includes the memory edits use. +- [`read()`](read.md) discards all edits (so its `input` must not be a string obtained from this document), + [`shrink_to_fit()`](shrink_to_fit.md) does not move the node index once there are edits, and + [`memory_usage()`](memory_usage.md) includes the memory edits use. [`source_offset()`](../basic_json_view/source_offset.md) of a value introduced by an edit is `#!cpp static_cast(-1)`, the same value it reports for a decoded string. diff --git a/docs/mkdocs/docs/api/basic_json_document/read.md b/docs/mkdocs/docs/api/basic_json_document/read.md index 1552c1007..de3246f6d 100644 --- a/docs/mkdocs/docs/api/basic_json_document/read.md +++ b/docs/mkdocs/docs/api/basic_json_document/read.md @@ -46,6 +46,34 @@ Linear in the length of the input. Every view taken from `#!cpp *this` before the call — including the previous [`root()`](root.md) — is invalidated, whether or not the new parse succeeds; take fresh views from [`root()`](root.md) afterward. +!!! warning "`input` must not point into the document itself" + + `read()` releases the document's previous text, its decoded strings, and (for an editable document) its edits + *before* it parses `input`. An `input` that is, or points into, any of them violates a precondition: it may seem to + work, parse empty or garbled text, be reported by a sanitizer, or read freed memory (undefined behavior). + + This applies to every `string_view` obtained from the document whose characters live in memory that `read()` + releases: + + - [`source()`](source.md) of a document that owns its text; + - [`get_string()`](../basic_json_view/get_string.md) (also as `get()` or a `string_view` member of a + decoded struct) of a string with escapes, of a string of a document that owns its text, and of an edited string; + - [`number_token()`](../basic_json_view/number_token.md) of a number of a document that owns its text, and of a + number written by an edit; + - the key of an object member (`key()`) if it has escapes, was written by an edit, or the document owns its text. + + It does not apply to a text that the caller owns and the document borrows, nor to the strings, keys, and tokens that + are read from such a text unchanged, nor to the decoded strings of a borrowed [image](load.md), which live in the + caller's image: `read()` never touches them. To parse JSON that a document holds in a string of its own (for + example, JSON embedded in a string value), copy the string first, for example with + `#!cpp std::string inner(doc.root()["payload"].get_string()); doc.read(inner);`. + + A debug build (one without `NDEBUG`) checks this with `assert()`: an `input` in the document's text, decoded + strings, or image, or in the most recent buffer of strings written by edits, ends the program. (Strings written + by earlier edits are not covered.) A release build does not check, to keep the cost of reusing a document at a + minimum. The check uses `assert()` directly, not [`JSON_ASSERT`](../macros/json_assert.md), which `json.hpp` + undefines at its end. + `input` is borrowed or owned by the same rules as [`parse()`](parse.md#notes); a document can borrow on one call and own on the next, since ownership is decided freshly each time. An input that is copied (see the table in [`parse()`](parse.md#notes)) is written into the document's previous owned copy, reusing its memory; a moved-in diff --git a/docs/mkdocs/docs/api/basic_json_document/source.md b/docs/mkdocs/docs/api/basic_json_document/source.md index a0c865195..860315944 100644 --- a/docs/mkdocs/docs/api/basic_json_document/source.md +++ b/docs/mkdocs/docs/api/basic_json_document/source.md @@ -24,6 +24,9 @@ Constant. For a borrowed document, `source()` points directly into the caller's buffer, so it is only valid while that buffer is; see [`owns_source`](owns_source.md). +The text of a document that owns it must not be passed to [`read()`](read.md) of the same document: `read()` releases +an owned text before it parses its input. Copy it first. (A borrowed text belongs to the caller and is not affected.) + ## Examples ??? example diff --git a/docs/mkdocs/docs/api/basic_json_view/get_string.md b/docs/mkdocs/docs/api/basic_json_view/get_string.md index 542ce4688..d17d80855 100644 --- a/docs/mkdocs/docs/api/basic_json_view/get_string.md +++ b/docs/mkdocs/docs/api/basic_json_view/get_string.md @@ -46,6 +46,10 @@ The returned [`string_view_t`](index.md#member-types) is valid exactly as long a source text, which for a borrowed document must remain valid **and unmodified** for as long as the document exists, since floating-point values and unescaped strings are read from the source lazily. +A string with escapes, a string of a document that owns its text, and an edited string must not be passed to +[`read()`](../basic_json_document/read.md) of the document it came from: `read()` releases them before it parses its +input. Copy the string first. (A string read unchanged from a borrowed text is not affected.) + ## Examples ??? example diff --git a/docs/mkdocs/docs/api/basic_json_view/number_token.md b/docs/mkdocs/docs/api/basic_json_view/number_token.md index 8ee8ce348..4811b65f7 100644 --- a/docs/mkdocs/docs/api/basic_json_view/number_token.md +++ b/docs/mkdocs/docs/api/basic_json_view/number_token.md @@ -38,9 +38,14 @@ by the grammar; trailing zeros in the fraction; the case and sign of the exponen useful precisely where that form must survive — a price or an identifier that must be reproduced exactly, or a number too large for any of `BasicJsonType`'s number types to hold without loss. -The returned [`string_view_t`](index.md#member-types) always points into the document's -[`source()`](../basic_json_document/source.md) text — numbers are never decoded into the document's separate string -buffer — and is valid exactly as long as that text is, see the [validity rules](index.md) of `basic_json_view`. +The returned [`string_view_t`](index.md#member-types) points into the document's +[`source()`](../basic_json_document/source.md) text or, for a number written by an +[edit](../basic_json_document/index.md#edits), into storage owned by the document (the token of an edited `NaN` is +`nan`). It is valid exactly as long as that text or storage is, see the [validity rules](index.md) of +`basic_json_view`. Numbers are never decoded into the document's separate string buffer. + +For the same reason, the token must not be passed to [`read()`](../basic_json_document/read.md) of the document it came +from if that document owns its text or the number was written by an edit: `read()` releases both before it parses. ## Examples diff --git a/docs/mkdocs/docs/features/json_view/lifetime.md b/docs/mkdocs/docs/features/json_view/lifetime.md index 2ecc8b711..4bb01a743 100644 --- a/docs/mkdocs/docs/features/json_view/lifetime.md +++ b/docs/mkdocs/docs/features/json_view/lifetime.md @@ -50,6 +50,13 @@ The same holds for what a view hands out without copying: a string from [`number_token()`](../../api/basic_json_view/number_token.md), and [`string_view` members](reading.md#string_view-members) of decoded structs are valid only as long as the document and, if it borrows, its source text. +This includes [`read()`](../../api/basic_json_document/read.md): it releases the document's text, decoded strings, and +edits before it parses its input, so the input must not be a `string_view` obtained from the document itself: its +[`source()`](../../api/basic_json_document/source.md), a string from `get_string()`, a token from `number_token()`, or a +key from an iterator (those of a borrowed text that the caller owns, read unchanged, are not affected). To re-parse a +string that the document holds, for example JSON embedded in a string value, copy the string into a `#!cpp std::string` +first and pass the copy; see [`read()`](../../api/basic_json_document/read.md#notes) for the exact list. + ??? example "Example: borrowed and owned documents, and when views become invalid" ```cpp diff --git a/include/nlohmann/detail/view/document_data.hpp b/include/nlohmann/detail/view/document_data.hpp index 2c5f97767..c1e179e93 100644 --- a/include/nlohmann/detail/view/document_data.hpp +++ b/include/nlohmann/detail/view/document_data.hpp @@ -12,6 +12,7 @@ #include // size_t #include // uint8_t, uint32_t #include // memcmp, memcpy +#include // less #include // numeric_limits #include // map #include // unique_ptr @@ -149,6 +150,23 @@ struct document_data node_capacity = inline_capacity; } + /// whether the n bytes at p overlap memory that the next parse releases or + /// reuses: the owned text, the decoded strings, an owned image, and the + /// current buffer of strings written by edits (earlier edit buffers are not + /// covered, as their sizes are not kept); for the debug check in read() + bool holds(const char* p, std::size_t n) const noexcept + { + const auto overlaps = [p, n](const char* b, std::size_t m) noexcept + { + // (std::less: a total order also for pointers into different objects) + return n != 0 && m != 0 && std::less()(p, b + m) && std::less()(b, p + n); + }; + return overlaps(owned.data(), owned.capacity()) + || overlaps(arena.data(), arena.capacity()) + || overlaps(reinterpret_cast(owned_image.data()), owned_image.capacity()) // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + || (edits && !edits->texts.empty() && overlaps(edits->texts.back().get(), edits->text_cap)); + } + /// the largest node count whose size in bytes fits a std::size_t static constexpr std::size_t max_nodes() noexcept { diff --git a/include/nlohmann/detail/view/macro_scope.hpp b/include/nlohmann/detail/view/macro_scope.hpp index f19492648..7340181d7 100644 --- a/include/nlohmann/detail/view/macro_scope.hpp +++ b/include/nlohmann/detail/view/macro_scope.hpp @@ -71,6 +71,11 @@ #define NLOHMANN_VIEW_THROW JSON_THROW_USER #endif +// debug checks of preconditions (assert(), off with NDEBUG): JSON_ASSERT is +// one of the macros that json.hpp undefines at its end +#include // assert +#define NLOHMANN_VIEW_ASSERT(x) assert(x) + // the parser stores a node's first word at once where the layout of `node` is // known to be little-endian (MSVC targets are); elsewhere field by field #if (defined(__BYTE_ORDER__) && defined(__ORDER_LITTLE_ENDIAN__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) || defined(_MSC_VER) diff --git a/include/nlohmann/detail/view/macro_unscope.hpp b/include/nlohmann/detail/view/macro_unscope.hpp index 0c4fab4d2..e0fb6554b 100644 --- a/include/nlohmann/detail/view/macro_unscope.hpp +++ b/include/nlohmann/detail/view/macro_unscope.hpp @@ -18,6 +18,7 @@ #undef NLOHMANN_VIEW_NOCLONE #undef NLOHMANN_VIEW_NODISCARD #undef NLOHMANN_VIEW_THROW +#undef NLOHMANN_VIEW_ASSERT #undef NLOHMANN_VIEW_LITTLE_ENDIAN #undef NLOHMANN_VIEW_REPEAT16 #undef NLOHMANN_VIEW_REPEAT_SECOND_8 diff --git a/include/nlohmann/json_view.hpp b/include/nlohmann/json_view.hpp index 64aa8f723..b922d82dd 100644 --- a/include/nlohmann/json_view.hpp +++ b/include/nlohmann/json_view.hpp @@ -1531,6 +1531,10 @@ class basic_json_document /// sentinel: src[size] is readable and 0 (std::string, C strings) void build(const char* src, std::size_t size, on_error allow_exceptions, bool comments, bool trailing_commas, bool owned, bool sentinel) { + // precondition of read(): a borrowed input is not held by this document, + // which releases what it holds below (checked in debug builds only, as + // it would cost every read() of a small message a few percent) + NLOHMANN_VIEW_ASSERT(owned || !m_data || !m_data->holds(src, size)); ensure_data(src, size); document_data& d = *m_data; if (!owned) @@ -1620,6 +1624,10 @@ class basic_json_document template void read_kind(const T& s, on_error ae, bool c, bool tc, std::integral_constant /*unused*/) { + // (the precondition of read() as for a borrowed input, although only an + // input in the owned text breaks a copy: the same call borrows, for + // example a std::string_view in C++17, and must not depend on that) + NLOHMANN_VIEW_ASSERT(!m_data || !m_data->holds(reinterpret_cast(s.data()), static_cast(s.size()))); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) build_copy(s, ae, c, tc); } @@ -1638,9 +1646,24 @@ class basic_json_document template void read_kind(T&& input, on_error ae, bool c, bool tc, std::integral_constant /*unused*/) { + NLOHMANN_VIEW_ASSERT(!held(input)); // (see the copy_range overload) build_copy(std::forward(input), ae, c, tc); } + /// whether an input read through the input adapter is held by this document: + /// string_view_t before C++17 (a string handed out by a view), for the + /// debug check of read(); other inputs of this kind do not point into it + bool held(const detail::view::string_ref& s) const noexcept + { + return m_data && m_data->holds(s.data(), s.size()); + } + + template + bool held(const T& /*input*/) const noexcept + { + return false; + } + template void read_range(IteratorType first, IteratorType last, on_error ae, bool c, bool tc) { diff --git a/single_include/nlohmann/json_view.hpp b/single_include/nlohmann/json_view.hpp index f69d1ae43..b2523284d 100644 --- a/single_include/nlohmann/json_view.hpp +++ b/single_include/nlohmann/json_view.hpp @@ -106,6 +106,7 @@ #include // size_t #include // uint8_t, uint32_t #include // memcmp, memcpy +#include // less #include // numeric_limits #include // map #include // unique_ptr @@ -188,6 +189,11 @@ #define NLOHMANN_VIEW_THROW JSON_THROW_USER #endif +// debug checks of preconditions (assert(), off with NDEBUG): JSON_ASSERT is +// one of the macros that json.hpp undefines at its end +#include // assert +#define NLOHMANN_VIEW_ASSERT(x) assert(x) + // the parser stores a node's first word at once where the layout of `node` is // known to be little-endian (MSVC targets are); elsewhere field by field #if (defined(__BYTE_ORDER__) && defined(__ORDER_LITTLE_ENDIAN__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) || defined(_MSC_VER) @@ -488,6 +494,23 @@ struct document_data node_capacity = inline_capacity; } + /// whether the n bytes at p overlap memory that the next parse releases or + /// reuses: the owned text, the decoded strings, an owned image, and the + /// current buffer of strings written by edits (earlier edit buffers are not + /// covered, as their sizes are not kept); for the debug check in read() + bool holds(const char* p, std::size_t n) const noexcept + { + const auto overlaps = [p, n](const char* b, std::size_t m) noexcept + { + // (std::less: a total order also for pointers into different objects) + return n != 0 && m != 0 && std::less()(p, b + m) && std::less()(b, p + n); + }; + return overlaps(owned.data(), owned.capacity()) + || overlaps(arena.data(), arena.capacity()) + || overlaps(reinterpret_cast(owned_image.data()), owned_image.capacity()) // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + || (edits && !edits->texts.empty() && overlaps(edits->texts.back().get(), edits->text_cap)); + } + /// the largest node count whose size in bytes fits a std::size_t static constexpr std::size_t max_nodes() noexcept { @@ -11630,6 +11653,10 @@ class basic_json_document /// sentinel: src[size] is readable and 0 (std::string, C strings) void build(const char* src, std::size_t size, on_error allow_exceptions, bool comments, bool trailing_commas, bool owned, bool sentinel) { + // precondition of read(): a borrowed input is not held by this document, + // which releases what it holds below (checked in debug builds only, as + // it would cost every read() of a small message a few percent) + NLOHMANN_VIEW_ASSERT(owned || !m_data || !m_data->holds(src, size)); ensure_data(src, size); document_data& d = *m_data; if (!owned) @@ -11719,6 +11746,10 @@ class basic_json_document template void read_kind(const T& s, on_error ae, bool c, bool tc, std::integral_constant /*unused*/) { + // (the precondition of read() as for a borrowed input, although only an + // input in the owned text breaks a copy: the same call borrows, for + // example a std::string_view in C++17, and must not depend on that) + NLOHMANN_VIEW_ASSERT(!m_data || !m_data->holds(reinterpret_cast(s.data()), static_cast(s.size()))); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) build_copy(s, ae, c, tc); } @@ -11737,9 +11768,24 @@ class basic_json_document template void read_kind(T&& input, on_error ae, bool c, bool tc, std::integral_constant /*unused*/) { + NLOHMANN_VIEW_ASSERT(!held(input)); // (see the copy_range overload) build_copy(std::forward(input), ae, c, tc); } + /// whether an input read through the input adapter is held by this document: + /// string_view_t before C++17 (a string handed out by a view), for the + /// debug check of read(); other inputs of this kind do not point into it + bool held(const detail::view::string_ref& s) const noexcept + { + return m_data && m_data->holds(s.data(), s.size()); + } + + template + bool held(const T& /*input*/) const noexcept + { + return false; + } + template void read_range(IteratorType first, IteratorType last, on_error ae, bool c, bool tc) { @@ -11895,6 +11941,7 @@ class tuple_element> // NOLINT(cert #undef NLOHMANN_VIEW_NOCLONE #undef NLOHMANN_VIEW_NODISCARD #undef NLOHMANN_VIEW_THROW +#undef NLOHMANN_VIEW_ASSERT #undef NLOHMANN_VIEW_LITTLE_ENDIAN #undef NLOHMANN_VIEW_REPEAT16 #undef NLOHMANN_VIEW_REPEAT_SECOND_8 diff --git a/tests/src/unit-json_view_builder.cpp b/tests/src/unit-json_view_builder.cpp index 988c25771..ea3c12de7 100644 --- a/tests/src/unit-json_view_builder.cpp +++ b/tests/src/unit-json_view_builder.cpp @@ -906,3 +906,71 @@ TEST_CASE("json_view node array size limit") d->reserve(cap); CHECK(d->nodes == nodes); } + +TEST_CASE("json_view document_data::holds() (the debug check of read())") +{ + std::unique_ptr d(document_data::create(0)); + const std::string other = "[1,2,3]"; + // an empty document holds nothing + CHECK(!d->holds(other.data(), other.size())); + + SECTION("owned text") + { + d->owned = "{\"a\":\"some text that does not fit a short string\"}"; + const char* const b = d->owned.data(); + CHECK(d->holds(b, 1)); + CHECK(d->holds(b + 5, 3)); + CHECK(d->holds(b + d->owned.size() - 1, 1)); + // also after clear(), which keeps the buffer that the next parse writes + d->owned.clear(); + CHECK(d->holds(b, 4)); + CHECK(!d->holds(other.data(), other.size())); + CHECK(!d->holds(b, 0)); + } + + SECTION("decoded strings") + { + d->arena.assign(32, 'x'); + CHECK(d->holds(d->arena.data() + 31, 1)); + CHECK(d->holds(d->arena.data() + 31, 8)); // (overlapping the end) + CHECK(!d->holds(other.data(), other.size())); + } + + SECTION("owned image") + { + d->owned_image.assign(16, 0); + CHECK(d->holds(reinterpret_cast(d->owned_image.data()) + 8, 2)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + } + + SECTION("current edit buffer") + { + d->edits.reset(new document_data::edit_state()); // NOLINT(cppcoreguidelines-owning-memory) + CHECK(!d->holds(other.data(), other.size())); + d->edits->texts.emplace_back(new char[16]); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) + d->edits->text_cap = 16; + CHECK(d->holds(d->edits->texts.back().get() + 3, 3)); + CHECK(!d->holds(other.data(), other.size())); + } +} + +TEST_CASE("json_view read() of inputs the check must accept") +{ + // in a debug build each read() below runs the check; none of the inputs + // is held by the document + const std::string text = R"({"a":"x\ny","b":[1,2]})"; + nlohmann::json_document d; + d.read(text); // borrowed + d.read(text); // the same borrowed text again + CHECK(d.root()["b"][1].get() == 2); + const std::string copy(d.root()["a"].get_string()); // a decoded string, copied first + d.read(std::string(text)); // owned + d.read(R"(["x"])"); + d.read(text); + CHECK(copy == "x\ny"); + nlohmann::json_editable_document e; + e.read(text); + e.set(e.root(), "a", "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]"); + const std::string edited(e.root()["a"].get_string()); // an edited string, copied first + e.read(edited); + CHECK(e.root().size() == 16); +}