Merge branch 'develop' into claude/noexception-value-json-pointer-5672

Conflicts:
- tests/src/unit-disabled_exceptions.cpp: kept both new sections (#5672 value(json_pointer) test and develop's ordered_json growth test)

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

docs/mkdocs/docs/api/basic_json/emplace.md

@@ -69,5 +69,3 @@ Logarithmic in the size of the container, O(log(`size()`)).
 ## Version history
 
 - Since version 2.0.8.
-- Fixed in version 3.13.0: for [`ordered_json`](../ordered_json.md), the value could previously only be passed as an
-  rvalue; it can now also be passed as an lvalue or a `#!cpp const` lvalue, matching the behavior of `json`.

include/nlohmann/detail/json_pointer.hpp

@@ -679,19 +679,29 @@ class json_pointer
                         return nullptr;
                     }
 
-                    // may throw parse_error.106/109 for a malformed index; an
-                    // index that is syntactically valid but cannot be
-                    // represented (out_of_range.404/410) is treated like an
-                    // out-of-range index below
-                    typename BasicJsonType::size_type idx{};
-                    JSON_TRY
+                    // tokens that array_index() rejects with parse_error.106/109
+                    // are passed on to it; all other tokens that it would reject
+                    // with out_of_range.404/410 are detected here, so that this
+                    // also works without exceptions
+                    if (JSON_HEDLEY_UNLIKELY(reference_token.size() > 1 && !(reference_token[0] >= '1' && reference_token[0] <= '9')))
                     {
-                        idx = array_index<BasicJsonType>(reference_token);
+                        static_cast<void>(array_index<BasicJsonType>(reference_token)); // throws parse_error.106/109
                     }
-                    JSON_INTERNAL_CATCH (detail::out_of_range&)
+                    if (JSON_HEDLEY_UNLIKELY(reference_token.empty() || !std::all_of(reference_token.begin(), reference_token.end(), [](const char c)
+                {
+                    return c >= '0' && c <= '9';
+                })))
                     {
                         return nullptr;
                     }
+                    errno = 0; // strtoull() does not reset errno on success
+                    char* p_end = nullptr; // NOLINT(misc-const-correctness)
+                    const unsigned long long magnitude = std::strtoull(reference_token.data(), &p_end, 10); // NOLINT(runtime/int)
+                    if (JSON_HEDLEY_UNLIKELY(errno == ERANGE || magnitude >= static_cast<unsigned long long>((std::numeric_limits<typename BasicJsonType::size_type>::max)()))) // NOLINT(runtime/int)
+                    {
+                        return nullptr;
+                    }
+                    const auto idx = static_cast<typename BasicJsonType::size_type>(magnitude);
 
                     if (JSON_HEDLEY_UNLIKELY(idx >= ptr->m_data.m_value.array->size()))
                     {

include/nlohmann/ordered_map.hpp

@@ -72,9 +72,7 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
         return *this;
     }
 
-    template<class V, detail::enable_if_t<
-                 detail::is_constructible<T, V>::value, int> = 0>
-    std::pair<iterator, bool> emplace(const key_type& key, V && t)
+    std::pair<iterator, bool> emplace(const key_type& key, T&& t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -83,14 +81,13 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
                 return {it, false};
             }
         }
-        append(key, std::forward<V>(t));
+        append(key, std::forward<T>(t));
         return {std::prev(this->end()), true};
     }
 
-    template<class KeyType, class V, detail::enable_if_t<
-                 detail::conjunction<detail::is_usable_as_key_type<key_compare, key_type, KeyType>,
-                                     detail::is_constructible<T, V>>::value, int> = 0>
-    std::pair<iterator, bool> emplace(KeyType && key, V && t)
+    template<class KeyType, detail::enable_if_t<
+                 detail::is_usable_as_key_type<key_compare, key_type, KeyType>::value, int> = 0>
+    std::pair<iterator, bool> emplace(KeyType && key, T && t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -99,7 +96,7 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
                 return {it, false};
             }
         }
-        append(std::forward<KeyType>(key), std::forward<V>(t));
+        append(std::forward<KeyType>(key), std::forward<T>(t));
         return {std::prev(this->end()), true};
     }
 

single_include/nlohmann/json.hpp

@@ -20291,19 +20291,29 @@ class json_pointer
                         return nullptr;
                     }
 
-                    // may throw parse_error.106/109 for a malformed index; an
-                    // index that is syntactically valid but cannot be
-                    // represented (out_of_range.404/410) is treated like an
-                    // out-of-range index below
-                    typename BasicJsonType::size_type idx{};
-                    JSON_TRY
+                    // tokens that array_index() rejects with parse_error.106/109
+                    // are passed on to it; all other tokens that it would reject
+                    // with out_of_range.404/410 are detected here, so that this
+                    // also works without exceptions
+                    if (JSON_HEDLEY_UNLIKELY(reference_token.size() > 1 && !(reference_token[0] >= '1' && reference_token[0] <= '9')))
                     {
-                        idx = array_index<BasicJsonType>(reference_token);
+                        static_cast<void>(array_index<BasicJsonType>(reference_token)); // throws parse_error.106/109
                     }
-                    JSON_INTERNAL_CATCH (detail::out_of_range&)
+                    if (JSON_HEDLEY_UNLIKELY(reference_token.empty() || !std::all_of(reference_token.begin(), reference_token.end(), [](const char c)
+                {
+                    return c >= '0' && c <= '9';
+                })))
                     {
                         return nullptr;
                     }
+                    errno = 0; // strtoull() does not reset errno on success
+                    char* p_end = nullptr; // NOLINT(misc-const-correctness)
+                    const unsigned long long magnitude = std::strtoull(reference_token.data(), &p_end, 10); // NOLINT(runtime/int)
+                    if (JSON_HEDLEY_UNLIKELY(errno == ERANGE || magnitude >= static_cast<unsigned long long>((std::numeric_limits<typename BasicJsonType::size_type>::max)()))) // NOLINT(runtime/int)
+                    {
+                        return nullptr;
+                    }
+                    const auto idx = static_cast<typename BasicJsonType::size_type>(magnitude);
 
                     if (JSON_HEDLEY_UNLIKELY(idx >= ptr->m_data.m_value.array->size()))
                     {
@@ -26622,9 +26632,7 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
         return *this;
     }
 
-    template<class V, detail::enable_if_t<
-                 detail::is_constructible<T, V>::value, int> = 0>
-    std::pair<iterator, bool> emplace(const key_type& key, V && t)
+    std::pair<iterator, bool> emplace(const key_type& key, T&& t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -26633,14 +26641,13 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
                 return {it, false};
             }
         }
-        append(key, std::forward<V>(t));
+        append(key, std::forward<T>(t));
         return {std::prev(this->end()), true};
     }
 
-    template<class KeyType, class V, detail::enable_if_t<
-                 detail::conjunction<detail::is_usable_as_key_type<key_compare, key_type, KeyType>,
-                                     detail::is_constructible<T, V>>::value, int> = 0>
-    std::pair<iterator, bool> emplace(KeyType && key, V && t)
+    template<class KeyType, detail::enable_if_t<
+                 detail::is_usable_as_key_type<key_compare, key_type, KeyType>::value, int> = 0>
+    std::pair<iterator, bool> emplace(KeyType && key, T && t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -26649,7 +26656,7 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
                 return {it, false};
             }
         }
-        append(std::forward<KeyType>(key), std::forward<V>(t));
+        append(std::forward<KeyType>(key), std::forward<T>(t));
         return {std::prev(this->end()), true};
     }
 

tests/src/unit-disabled_exceptions.cpp

@@ -47,6 +47,21 @@ TEST_CASE("Tests with disabled exceptions")
         delete sax_no_exception::error_string; // NOLINT(cppcoreguidelines-owning-memory)
     }
 
+    SECTION("issue #5672 - value(json_pointer, default) must not abort for array tokens that are not a valid index")
+    {
+        const json j = {1, 2, 3};
+
+        // a syntactically valid index that is out of range for this array
+        CHECK(j.value("/7"_json_pointer, 42) == 42);
+        // a reference token that is not a number at all
+        CHECK(j.value("/1a"_json_pointer, 42) == 42);
+        // the empty reference token (JSON pointer "/")
+        CHECK(j.value("/"_json_pointer, 42) == 42);
+        // an index whose magnitude does not fit into size_type
+        CHECK(j.value("/99999999999999999999999"_json_pointer, 42) == 42);
+        CHECK(j.value("/18446744073709551615"_json_pointer, 42) == 42);
+    }
+
     SECTION("growing an ordered_json object")
     {
         auto j = nlohmann::ordered_json::object();

tests/src/unit-element_access2.cpp

@@ -516,6 +516,21 @@ TEST_CASE_TEMPLATE("element access 2", Json, nlohmann::json, nlohmann::ordered_j
                     CHECK(j_array.value("/-"_json_pointer, 42) == 42);
                     CHECK(j_array_const.value("/-"_json_pointer, 42) == 42);
 
+                    // Test an index with a non-digit after a valid leading digit; this is
+                    // out_of_range (not parse_error) and must not throw (see #5672)
+                    CHECK(j_array.value("/1a"_json_pointer, 42) == 42);
+                    CHECK(j_array_const.value("/1a"_json_pointer, 42) == 42);
+
+                    // Test the empty reference token (JSON pointer "/"); see #5672
+                    CHECK(j_array.value("/"_json_pointer, 42) == 42);
+                    CHECK(j_array_const.value("/"_json_pointer, 42) == 42);
+
+                    // Test an index whose magnitude does not fit into size_type (see #5672)
+                    CHECK(j_array.value("/99999999999999999999999"_json_pointer, 42) == 42);
+                    CHECK(j_array_const.value("/99999999999999999999999"_json_pointer, 42) == 42);
+                    CHECK(j_array.value("/18446744073709551615"_json_pointer, 42) == 42);
+                    CHECK(j_array_const.value("/18446744073709551615"_json_pointer, 42) == 42);
+
 #if !defined(JSON_NOEXCEPTION)
                     // Test malformed index (non-numeric) throws parse_error
                     CHECK_THROWS_WITH_AS(j_array.value("/foo"_json_pointer, 1), "[json.exception.parse_error.109] parse error: array index 'foo' is not a number", typename Json::parse_error&);

tests/src/unit-ordered_json.cpp

@@ -196,44 +196,3 @@ TEST_CASE("regression test - diff() must account for ordered_json member order")
         CHECK(a.patch(p) == b);
     }
 }
-
-TEST_CASE("regression test for issue #5673 - ordered_json::emplace with a non-rvalue value")
-{
-    SECTION("lvalue value")
-    {
-        ordered_json oj = ordered_json::object();
-        ordered_json value = 1;
-        auto res = oj.emplace("a", value);
-        CHECK(res.second == true);
-        CHECK(oj.dump() == "{\"a\":1}");
-    }
-
-    SECTION("const lvalue value")
-    {
-        ordered_json oj = ordered_json::object();
-        const ordered_json value = 1;
-        auto res = oj.emplace("a", value);
-        CHECK(res.second == true);
-        CHECK(oj.dump() == "{\"a\":1}");
-    }
-
-    SECTION("rvalue value")
-    {
-        ordered_json oj = ordered_json::object();
-        auto res = oj.emplace("a", ordered_json(1));
-        CHECK(res.second == true);
-        CHECK(oj.dump() == "{\"a\":1}");
-    }
-
-    SECTION("existing key is not overwritten (std::map-compatible semantics)")
-    {
-        ordered_json oj = ordered_json::object();
-        ordered_json value = 1;
-        oj.emplace("a", value);
-
-        ordered_json other_value = 2;
-        auto res = oj.emplace("a", other_value);
-        CHECK(res.second == false);
-        CHECK(oj.dump() == "{\"a\":1}");
-    }
-}

tests/src/unit-ordered_map.cpp

@@ -403,79 +403,6 @@ TEST_CASE("ordered_map")
             CHECK(om.size() == 4);
         }
     }
-
-    SECTION("emplace")
-    {
-        // regression test for issue #5673: the mapped-value parameter must
-        // accept lvalues and const lvalues, not just rvalues
-        ordered_map<std::string, std::string> om;
-        om["eins"] = "one";
-        om["zwei"] = "two";
-        om["drei"] = "three";
-
-        SECTION("with T&& (rvalue)")
-        {
-            auto res1 = om.emplace("eins", std::string("1"));
-            CHECK(res1.first == om.begin());
-            CHECK(res1.second == false);
-            CHECK(om.size() == 3);
-            CHECK(om.at("eins") == "one"); // existing key is not overwritten
-
-            auto res4 = om.emplace("vier", std::string("four"));
-            CHECK(res4.first == om.begin() + 3);
-            CHECK(res4.second == true);
-            CHECK(om.size() == 4);
-            CHECK(om.at("vier") == "four");
-        }
-
-        SECTION("with T& (lvalue)")
-        {
-            std::string one = "1";
-            std::string four = "four";
-
-            auto res1 = om.emplace("eins", one);
-            CHECK(res1.first == om.begin());
-            CHECK(res1.second == false);
-            CHECK(om.size() == 3);
-            CHECK(om.at("eins") == "one"); // existing key is not overwritten
-
-            auto res4 = om.emplace("vier", four);
-            CHECK(res4.first == om.begin() + 3);
-            CHECK(res4.second == true);
-            CHECK(om.size() == 4);
-            CHECK(om.at("vier") == "four");
-            CHECK(four == "four"); // source was copied, not moved from
-        }
-
-        SECTION("with const T&")
-        {
-            const std::string one = "1";
-            const std::string four = "four";
-
-            auto res1 = om.emplace("eins", one);
-            CHECK(res1.first == om.begin());
-            CHECK(res1.second == false);
-            CHECK(om.size() == 3);
-
-            auto res4 = om.emplace("vier", four);
-            CHECK(res4.first == om.begin() + 3);
-            CHECK(res4.second == true);
-            CHECK(om.size() == 4);
-            CHECK(om.at("vier") == "four");
-        }
-
-        SECTION("with key of key_type (non-template overload)")
-        {
-            const std::string key_vier{"vier"};
-            std::string four = "four";
-
-            auto res4 = om.emplace(key_vier, four);
-            CHECK(res4.first == om.begin() + 3);
-            CHECK(res4.second == true);
-            CHECK(om.size() == 4);
-            CHECK(om.at("vier") == "four");
-        }
-    }
 }
 
 TEST_CASE("ordered_map growth")