Address the review of json_document images

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

docs/mkdocs/docs/api/basic_json_document/load.md

@@ -110,17 +110,30 @@ enum class image_check
 
 How thoroughly `load()` validates `image` before trusting it.
 
-| value    | checks                                                                                                    | guarantees |
-|----------|--------------------------------------------------------------------------------------------------------------|------------|
-| `full`   | everything the parser itself guarantees: structure and bounds; that every string is valid UTF-8 (and, for a string still in the source text, that it contains no quote, backslash, or control character); and that every number token is well-formed and matches the value stored for it; strings and float tokens may share a range only if the ranges are identical (as nodes that share a value do), and never overlap otherwise | reading and serializing a checked image is safe and always produces valid JSON, exactly as for a parsed document |
-| `bounds` | structure and bounds only -- that every offset and count in the node index stays inside the image            | reading and serializing stay memory-safe, but a crafted image can hold strings that are not valid UTF-8 or that serialize to invalid JSON ([`dump()`](../basic_json_view/dump.md) writes them unchanged or throws [`type_error.316`](../../home/exceptions.md#jsonexceptiontype_error316)), and numbers whose values differ from their text |
-| `none`   | nothing                                                                                                       | images from a trusted source only -- reading a damaged image is undefined behavior |
+`full` (the default)
+:   Checks everything the parser itself guarantees: structure and bounds; that every string is valid UTF-8 (and, for a
+    string still in the source text, that it contains no quote, backslash, or control character); and that every number
+    token is well-formed and matches the value stored for it. Strings and float tokens may share a range only if the
+    ranges are identical (as nodes that share a value do), and never overlap otherwise. Reading and serializing a
+    checked image is safe and always produces valid JSON, exactly as for a parsed document. This is the right choice for
+    an image from anything you do not fully control -- a file, a cache shared with other processes, a peer on the
+    network.
 
-`full` is the default and the right choice for an image from anything you do not fully control -- a file, a cache
-shared with other processes, a peer on the network. `bounds` skips scanning the text and the decoded strings, so it
-fits a cache your own process just wrote and reads straight back, where damage would mean a bug or a hardware fault
-rather than adversarial input; it still cannot crash or read out of bounds. `none` skips validation entirely and
-should only be used for an image you trust as much as your own memory.
+`bounds`
+:   Checks structure and bounds only: that every offset and count in the node index stays inside the image. So
+    `load()`, [`root()`](root.md), element access, and [`materialize()`](../basic_json_view/materialize.md) can never
+    read outside the image, even on a damaged one. It does *not* guarantee that the image describes valid JSON: a
+    string that a `full` check would have rejected can make [`dump()`](../basic_json_view/dump.md) write invalid UTF-8
+    or invalid JSON, or throw [`type_error.316`](../../home/exceptions.md#jsonexceptiontype_error316), and a number can
+    read back with a value that does not match how it is spelled. An [editable document](../json_editable_document.md)
+    does not take such a string over: [`set`](set.md), [`push_back`](push_back.md) and [`insert`](insert.md) throw
+    `type_error.316` when they copy it from a view of the loaded document, and leave the editable document unchanged.
+    `bounds` skips scanning the text and the decoded strings, so it fits a cache your own process just wrote and reads
+    straight back, where damage would mean a bug or a hardware fault rather than adversarial input.
+
+`none`
+:   Checks nothing. Reading a damaged image is undefined behavior, so use it only for an image you trust as much as
+    your own memory.
 
 **Lifetime.** Overloads 1 and 2 borrow `image`: it must stay alive and byte-for-byte unchanged for as long as the
 returned document, and any [view](../basic_json_view/index.md) taken from it, is used -- exactly like a document
@@ -134,16 +147,6 @@ returned document, and any [view](../basic_json_view/index.md) taken from it, is
     ("unknown format"). Use images to cache a document within one build of the library, or to hand one to another
     process running the *same* build on the *same* (little-endian) machine -- not as a long-term storage format.
 
-**What `image_check::bounds` does not guarantee.** A bounds-checked image can never make `load()`,
-[`root()`](root.md), element access, or [`materialize()`](../basic_json_view/materialize.md) read outside the image,
-so those stay safe on a damaged one. It does *not* guarantee that the image describes valid JSON: a string
-that a `full` check would have rejected can make [`dump()`](../basic_json_view/dump.md) write invalid UTF-8 or invalid
-JSON, or throw `type_error.316`, and a number can read back with a value that does not match how it is spelled.
-Reserve `bounds` for images you already trust to be well-formed, and use it only to skip the extra scan.
-An [editable document](../json_editable_document.md) does not take such a string over either:
-[`set`](set.md), [`push_back`](push_back.md) and [`insert`](insert.md) throw `type_error.316` when they copy it from
-a view of the loaded document, and leave the editable document unchanged.
-
 ## Examples
 
 ??? example "Caching a document, ownership, and a rejected image"

docs/mkdocs/docs/features/index.md

@@ -14,8 +14,8 @@ C++ types, and finally serialize it again.
   [parsing untrusted input](parsing/untrusted_input.md).
 - [Zero-copy JSON views](json_view.md) — read a JSON text through a flat index instead of building a `json` tree;
   strings and numbers stay in the input and are only decoded when needed.
-  [Editable documents](json_view.md#editing-a-document) can also be modified, and [images](json_view.md#images) load a
-  parsed document again without parsing it.
+- [Editable documents](json_view.md#editing-a-document) — modify a parsed document without building a `json` tree.
+- [Images](json_view.md#images) — save a parsed document and load it again without parsing it.
 - [Comments](comments.md) and [trailing commas](trailing_commas.md) — opt-in relaxations of the JSON grammar.
 
 ## Accessing and modifying values

include/nlohmann/detail/view/image.hpp

@@ -8,11 +8,11 @@
 
 #pragma once
 
-#include <algorithm> // sort
+#include <algorithm> // min, sort
 #include <array> // array
-#include <cstddef> // size_t
+#include <cstddef> // offsetof, size_t
 #include <cstdint> // int64_t, uint8_t, uint16_t, uint32_t, uint64_t
-#include <cstring> // memcmp, memcpy
+#include <cstring> // memcmp, memcpy, memset
 #include <limits> // numeric_limits
 #include <string> // string
 #include <vector> // vector
@@ -162,7 +162,7 @@ inline void compact_nodes(const document_data& d, std::size_t arena_size, std::v
     copy_node(d.nodes);
     while (!stack.empty())
     {
-        frame& top = stack.back();
+        const frame top = stack.back();
         if (top.cur == top.end)
         {
             node& c = out[top.index];
@@ -171,20 +171,14 @@ inline void compact_nodes(const document_data& d, std::size_t arena_size, std::v
             stack.pop_back();
             continue;
         }
-        ++top.count;
-        const node* v = nullptr;
         if (top.object)
         {
             out.push_back(string_node(*top.cur));
-            v = document_data::deref(top.cur + 1);
-            top.cur = document_data::after(top.cur + 1);
         }
-        else
-        {
-            v = document_data::deref(top.cur);
-            top.cur = document_data::after(top.cur);
-        }
-        copy_node(v); // may grow the stack (top is not used afterwards)
+        const node* const value = top.cur + (top.object ? 1 : 0);
+        stack.back().cur = document_data::after(value);
+        ++stack.back().count;
+        copy_node(document_data::deref(value)); // may grow the stack
     }
 }
 
@@ -195,17 +189,15 @@ inline std::vector<std::uint8_t> save_image(const document_data& d)
     throw_type_error(320, "json_document images need a little-endian target"); // LCOV_EXCL_LINE
 #endif
     const std::size_t arena_size = d.arena_size;
-    const node* nodes = d.nodes;
-    std::size_t count = d.node_count;
     std::vector<node> compacted;
     std::string text_tail;
     std::string arena_tail;
     if (d.edits)
     {
         compact_nodes(d, arena_size, compacted, text_tail, arena_tail);
-        nodes = compacted.data();
-        count = compacted.size();
     }
+    const node* const nodes = d.edits ? compacted.data() : d.nodes;
+    const std::size_t count = d.edits ? compacted.size() : d.node_count;
     const std::size_t text_size = d.size + text_tail.size();
     const std::size_t total_arena = arena_size + arena_tail.size();
     if (NLOHMANN_VIEW_UNLIKELY(text_size >= image_limit || total_arena >= image_limit || count >= image_limit))
@@ -225,14 +217,13 @@ inline std::vector<std::uint8_t> save_image(const document_data& d)
     std::memcpy(o, &h, sizeof(h));
     o += sizeof(h);
     std::memcpy(o, nodes, count * sizeof(node));
-    // the hash indexes are rebuilt by load()
+    // the hash indexes are rebuilt by load(), whose check rejects any other
+    // extra of an object, and the same document saves to the same bytes
     for (std::size_t i = 0; i < count; ++i)
     {
         if (nodes[i].kind == static_cast<std::uint8_t>(value_t::object) && nodes[i].extra != 0)
         {
-            node n = nodes[i];
-            n.extra = 0;
-            std::memcpy(o + (i * sizeof(node)), &n, sizeof(node));
+            std::memset(o + (i * sizeof(node)) + offsetof(node, extra), 0, sizeof(nodes[i].extra));
         }
     }
     o += count * sizeof(node);
@@ -271,21 +262,22 @@ inline bool scan_number_token(const unsigned char* s, std::size_t len, number_to
     const unsigned char* p = s;
     t.negative = p != e && *p == '-';
     p += t.negative ? 1 : 0;
-    t.int_start = p;
     if (p == e)
     {
         return false;
     }
+    t.int_start = p;
     if (*p == '0')
     {
         ++p;
     }
     else if (*p >= '1' && *p <= '9')
     {
-        while (p != e && is_digit(*p))
+        do
         {
             ++p;
         }
+        while (p != e && is_digit(*p));
     }
     else
     {
@@ -318,11 +310,12 @@ inline bool scan_number_token(const unsigned char* s, std::size_t len, number_to
         {
             return false;
         }
-        while (p != e && is_digit(*p))
+        do
         {
             t.exponent = t.exponent < 100000 ? (t.exponent * 10) + (*p - '0') : t.exponent;
             ++p;
         }
+        while (p != e && is_digit(*p));
         t.exponent = exp_negative ? -t.exponent : t.exponent;
         t.is_float = true;
     }
@@ -333,7 +326,7 @@ inline bool scan_number_token(const unsigned char* s, std::size_t len, number_to
 /// writes "many" instead; the caller accepts both)
 inline std::uint16_t float_layout(const number_token& t)
 {
-    return static_cast<std::uint16_t>((t.int_digits < 255 ? t.int_digits : 255) | ((t.frac_digits < 255 ? t.frac_digits : 255) << 8u));
+    return static_cast<std::uint16_t>((std::min)(t.int_digits, std::size_t{255}) | ((std::min)(t.frac_digits, std::size_t{255}) << 8u));
 }
 
 /// whether a float token (well-formed, per scan_number_token) is finite as
@@ -735,14 +728,10 @@ inline void load_image(document_data& d, const std::uint8_t* image, std::size_t
     // the hash indexes of large objects, as after parsing
     for (std::size_t i = 0; i < count; ++i)
     {
-        node& n = d.nodes[i];
-        if (n.kind == static_cast<std::uint8_t>(value_t::object))
+        const node& n = d.nodes[i];
+        if (n.kind == static_cast<std::uint8_t>(value_t::object) && n.len >= document_data::index_min_members)
         {
-            n.extra = 0;
-            if (n.len >= document_data::index_min_members)
-            {
-                d.large_objects.push_back(static_cast<std::uint32_t>(i));
-            }
+            d.large_objects.push_back(static_cast<std::uint32_t>(i));
         }
     }
     build_object_indexes(d);

include/nlohmann/detail/view/number.hpp

@@ -45,10 +45,11 @@ inline bool float_token_layout(const char* first, const char* last, std::size_t&
     {
         return false;
     }
-    while (digit(p))
+    do
     {
         ++p;
     }
+    while (digit(p));
     dot = std::string::npos;
     if (p != last && *p == '.')
     {
@@ -57,10 +58,11 @@ inline bool float_token_layout(const char* first, const char* last, std::size_t&
         {
             return false;
         }
-        while (digit(p))
+        do
         {
             ++p;
         }
+        while (digit(p));
     }
     mantissa_end = static_cast<std::size_t>(p - first);
     if (p != last && (*p == 'e' || *p == 'E'))
@@ -71,10 +73,11 @@ inline bool float_token_layout(const char* first, const char* last, std::size_t&
         {
             return false;
         }
-        while (digit(p))
+        do
         {
             ++p;
         }
+        while (digit(p));
     }
     return p == last;
 }

include/nlohmann/json_view.hpp

@@ -1038,7 +1038,7 @@ class basic_json_document
         return m_data ? m_data->node_count : 0;
     }
 
-    /// bytes held by the document (index, decoded strings, owned text or image)
+    /// bytes held by the document (index, decoded strings, and owned text or image)
     std::size_t memory_usage() const noexcept
     {
         if (!m_data)