Fix Clang deprecation warning for json_pointer operator== with ordered_json

is_comparable used a flat && chain to both exclude json_pointer/string
comparisons (added for #4621) and check whether Compare(A, B) is well-formed.
Naming std::is_constructible<decltype(...)> as a later operand of that chain
still causes the decltype to be substituted regardless of the first
operand's value, since the operands aren't lazily deferred like
std::conjunction would defer them. That instantiates the transparent
std::equal_to<>::operator() used by ordered_json, whose noexcept-specifier
evaluates the deprecated json_pointer/string operator==, which Clang (unlike
GCC in this case) warns about even though the result is discarded.

Split is_comparable so the Compare(A, B) checks live in a separate helper
that is only referenced from the specialization selected when
is_json_pointer_of is false, so the decltype is never written when A/B are
a json_pointer/string pair, regardless of compiler.

Signed-off-by: Niels Lohmann <niels.lohmann@gmail.com>

include/nlohmann/detail/input/binary_reader.hpp

@@ -1846,6 +1846,29 @@ class binary_reader
         return get_ubjson_value(get_char ? get_ignore_noop() : current);
     }
 
+    /*!
+    @brief reject a negative UBJSON/BJData string length
+
+    String and key lengths are written with signed integer markers (i, I, l,
+    L). A negative value is malformed; without this check get_string() would
+    silently treat it as an empty string and leave the following bytes to be
+    misread as the next value. This mirrors the non-negative check the
+    optimized-container count path already performs in get_ubjson_size_value.
+
+    @param[in] len  the string length read from the input
+    @return whether the length is valid (non-negative)
+    */
+    template<typename NumberType>
+    bool check_ubjson_string_length(const NumberType len)
+    {
+        if (JSON_HEDLEY_UNLIKELY(len < 0))
+        {
+            return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read,
+                                    exception_message(input_format, "string length must not be negative", "string"), nullptr));
+        }
+        return true;
+    }
+
     /*!
     @brief reads a UBJSON string
 
@@ -1883,25 +1906,25 @@ class binary_reader
             case 'i':
             {
                 std::int8_t len{};
-                return get_number(input_format, len) && get_string(input_format, len, result);
+                return get_number(input_format, len) && check_ubjson_string_length(len) && get_string(input_format, len, result);
             }
 
             case 'I':
             {
                 std::int16_t len{};
-                return get_number(input_format, len) && get_string(input_format, len, result);
+                return get_number(input_format, len) && check_ubjson_string_length(len) && get_string(input_format, len, result);
             }
 
             case 'l':
             {
                 std::int32_t len{};
-                return get_number(input_format, len) && get_string(input_format, len, result);
+                return get_number(input_format, len) && check_ubjson_string_length(len) && get_string(input_format, len, result);
             }
 
             case 'L':
             {
                 std::int64_t len{};
-                return get_number(input_format, len) && get_string(input_format, len, result);
+                return get_number(input_format, len) && check_ubjson_string_length(len) && get_string(input_format, len, result);
             }
 
             case 'u':

include/nlohmann/detail/meta/type_traits.hpp

@@ -699,21 +699,35 @@ struct is_json_pointer_of<A, ::nlohmann::json_pointer<A>> : std::true_type {};
 template <typename A>
 struct is_json_pointer_of<A, ::nlohmann::json_pointer<A>&> : std::true_type {};
 
-// checks if A and B are comparable using Compare functor
+// checks if A and B are comparable using Compare functor, assuming that
+// neither A nor B is a json_pointer type (that case is handled by
+// is_comparable below, which never instantiates this helper otherwise)
 template<typename Compare, typename A, typename B, typename = void>
-struct is_comparable : std::false_type {};
+struct is_comparable_no_json_pointer : std::false_type {};
 
-// We exclude json_pointer here, because the checks using Compare(A, B) will
-// use json_pointer::operator string_t() which triggers a deprecation warning
-// for GCC. See https://github.com/nlohmann/json/issues/4621. The call to
-// is_json_pointer_of can be removed once the deprecated function has been
-// removed.
 template<typename Compare, typename A, typename B>
-struct is_comparable < Compare, A, B, enable_if_t < !is_json_pointer_of<A, B>::value
-&& std::is_constructible <decltype(std::declval<Compare>()(std::declval<A>(), std::declval<B>()))>::value
+struct is_comparable_no_json_pointer < Compare, A, B, enable_if_t <
+std::is_constructible <decltype(std::declval<Compare>()(std::declval<A>(), std::declval<B>()))>::value
 && std::is_constructible <decltype(std::declval<Compare>()(std::declval<B>(), std::declval<A>()))>::value
 >> : std::true_type {};
 
+// checks if A and B are comparable using Compare functor
+// We dispatch on is_json_pointer_of as a plain bool (rather than folding it
+// into a single enable_if_t condition together with the checks below) so
+// that the Compare(A, B) checks are only ever written - and thus only ever
+// instantiated - when A/B are not a json_pointer/string pair. Those checks
+// use json_pointer::operator string_t() (GCC, see #4621) resp. the
+// deprecated json_pointer/string operator== (Clang, see #5288), and merely
+// naming them as later operands of a plain && chain is not sufficient to
+// avoid their instantiation on all compilers, even when the first operand
+// is false. The dispatch on is_json_pointer_of can be removed once the
+// deprecated json_pointer comparison operators have been removed.
+template<typename Compare, typename A, typename B, bool = is_json_pointer_of<A, B>::value>
+struct is_comparable : std::false_type {};
+
+template<typename Compare, typename A, typename B>
+struct is_comparable<Compare, A, B, false> : is_comparable_no_json_pointer<Compare, A, B> {};
+
 template<typename T>
 using detect_is_transparent = typename T::is_transparent;
 

single_include/nlohmann/json.hpp

@@ -4462,21 +4462,35 @@ struct is_json_pointer_of<A, ::nlohmann::json_pointer<A>> : std::true_type {};
 template <typename A>
 struct is_json_pointer_of<A, ::nlohmann::json_pointer<A>&> : std::true_type {};
 
-// checks if A and B are comparable using Compare functor
+// checks if A and B are comparable using Compare functor, assuming that
+// neither A nor B is a json_pointer type (that case is handled by
+// is_comparable below, which never instantiates this helper otherwise)
 template<typename Compare, typename A, typename B, typename = void>
-struct is_comparable : std::false_type {};
+struct is_comparable_no_json_pointer : std::false_type {};
 
-// We exclude json_pointer here, because the checks using Compare(A, B) will
-// use json_pointer::operator string_t() which triggers a deprecation warning
-// for GCC. See https://github.com/nlohmann/json/issues/4621. The call to
-// is_json_pointer_of can be removed once the deprecated function has been
-// removed.
 template<typename Compare, typename A, typename B>
-struct is_comparable < Compare, A, B, enable_if_t < !is_json_pointer_of<A, B>::value
-&& std::is_constructible <decltype(std::declval<Compare>()(std::declval<A>(), std::declval<B>()))>::value
+struct is_comparable_no_json_pointer < Compare, A, B, enable_if_t <
+std::is_constructible <decltype(std::declval<Compare>()(std::declval<A>(), std::declval<B>()))>::value
 && std::is_constructible <decltype(std::declval<Compare>()(std::declval<B>(), std::declval<A>()))>::value
 >> : std::true_type {};
 
+// checks if A and B are comparable using Compare functor
+// We dispatch on is_json_pointer_of as a plain bool (rather than folding it
+// into a single enable_if_t condition together with the checks below) so
+// that the Compare(A, B) checks are only ever written - and thus only ever
+// instantiated - when A/B are not a json_pointer/string pair. Those checks
+// use json_pointer::operator string_t() (GCC, see #4621) resp. the
+// deprecated json_pointer/string operator== (Clang, see #5288), and merely
+// naming them as later operands of a plain && chain is not sufficient to
+// avoid their instantiation on all compilers, even when the first operand
+// is false. The dispatch on is_json_pointer_of can be removed once the
+// deprecated json_pointer comparison operators have been removed.
+template<typename Compare, typename A, typename B, bool = is_json_pointer_of<A, B>::value>
+struct is_comparable : std::false_type {};
+
+template<typename Compare, typename A, typename B>
+struct is_comparable<Compare, A, B, false> : is_comparable_no_json_pointer<Compare, A, B> {};
+
 template<typename T>
 using detect_is_transparent = typename T::is_transparent;
 
@@ -12395,6 +12409,29 @@ class binary_reader
         return get_ubjson_value(get_char ? get_ignore_noop() : current);
     }
 
+    /*!
+    @brief reject a negative UBJSON/BJData string length
+
+    String and key lengths are written with signed integer markers (i, I, l,
+    L). A negative value is malformed; without this check get_string() would
+    silently treat it as an empty string and leave the following bytes to be
+    misread as the next value. This mirrors the non-negative check the
+    optimized-container count path already performs in get_ubjson_size_value.
+
+    @param[in] len  the string length read from the input
+    @return whether the length is valid (non-negative)
+    */
+    template<typename NumberType>
+    bool check_ubjson_string_length(const NumberType len)
+    {
+        if (JSON_HEDLEY_UNLIKELY(len < 0))
+        {
+            return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read,
+                                    exception_message(input_format, "string length must not be negative", "string"), nullptr));
+        }
+        return true;
+    }
+
     /*!
     @brief reads a UBJSON string
 
@@ -12432,25 +12469,25 @@ class binary_reader
             case 'i':
             {
                 std::int8_t len{};
-                return get_number(input_format, len) && get_string(input_format, len, result);
+                return get_number(input_format, len) && check_ubjson_string_length(len) && get_string(input_format, len, result);
             }
 
             case 'I':
             {
                 std::int16_t len{};
-                return get_number(input_format, len) && get_string(input_format, len, result);
+                return get_number(input_format, len) && check_ubjson_string_length(len) && get_string(input_format, len, result);
             }
 
             case 'l':
             {
                 std::int32_t len{};
-                return get_number(input_format, len) && get_string(input_format, len, result);
+                return get_number(input_format, len) && check_ubjson_string_length(len) && get_string(input_format, len, result);
             }
 
             case 'L':
             {
                 std::int64_t len{};
-                return get_number(input_format, len) && get_string(input_format, len, result);
+                return get_number(input_format, len) && check_ubjson_string_length(len) && get_string(input_format, len, result);
             }
 
             case 'u':

tests/src/unit-bjdata.cpp

@@ -2721,6 +2721,19 @@ TEST_CASE("BJData")
                 CHECK_THROWS_WITH_AS(_ = json::from_bjdata(v), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing BJData string: expected length type specification (U, i, u, I, m, l, M, L); last byte: 0x31", json::parse_error&);
             }
 
+            SECTION("negative length")
+            {
+                json _;
+
+                std::vector<uint8_t> const vi = {'S', 'i', 0xFF};
+                CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vi), "[json.exception.parse_error.113] parse error at byte 3: syntax error while parsing BJData string: string length must not be negative", json::parse_error&);
+                CHECK(json::from_bjdata(vi, true, false).is_discarded());
+
+                std::vector<uint8_t> const vl = {'S', 'l', 0xFF, 0xFF, 0xFF, 0xFF};
+                CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vl), "[json.exception.parse_error.113] parse error at byte 6: syntax error while parsing BJData string: string length must not be negative", json::parse_error&);
+                CHECK(json::from_bjdata(vl, true, false).is_discarded());
+            }
+
             SECTION("parse bjdata markers in ubjson")
             {
                 // create a single-character string for all number types

tests/src/unit-ubjson.cpp

@@ -1862,6 +1862,31 @@ TEST_CASE("UBJSON")
                 json _;
                 CHECK_THROWS_WITH_AS(_ = json::from_ubjson(v), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing UBJSON string: expected length type specification (U, i, I, l, L); last byte: 0x31", json::parse_error&);
             }
+
+            SECTION("negative length")
+            {
+                json _;
+
+                std::vector<uint8_t> const vi = {'S', 'i', 0xFF};
+                CHECK_THROWS_WITH_AS(_ = json::from_ubjson(vi), "[json.exception.parse_error.113] parse error at byte 3: syntax error while parsing UBJSON string: string length must not be negative", json::parse_error&);
+                CHECK(json::from_ubjson(vi, true, false).is_discarded());
+
+                std::vector<uint8_t> const vI = {'S', 'I', 0xFF, 0xFF};
+                CHECK_THROWS_WITH_AS(_ = json::from_ubjson(vI), "[json.exception.parse_error.113] parse error at byte 4: syntax error while parsing UBJSON string: string length must not be negative", json::parse_error&);
+                CHECK(json::from_ubjson(vI, true, false).is_discarded());
+
+                std::vector<uint8_t> const vl = {'S', 'l', 0xFF, 0xFF, 0xFF, 0xFF};
+                CHECK_THROWS_WITH_AS(_ = json::from_ubjson(vl), "[json.exception.parse_error.113] parse error at byte 6: syntax error while parsing UBJSON string: string length must not be negative", json::parse_error&);
+                CHECK(json::from_ubjson(vl, true, false).is_discarded());
+
+                std::vector<uint8_t> const vL = {'S', 'L', 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
+                CHECK_THROWS_WITH_AS(_ = json::from_ubjson(vL), "[json.exception.parse_error.113] parse error at byte 10: syntax error while parsing UBJSON string: string length must not be negative", json::parse_error&);
+                CHECK(json::from_ubjson(vL, true, false).is_discarded());
+
+                // a length of zero remains valid and yields an empty string
+                std::vector<uint8_t> const v0 = {'S', 'i', 0};
+                CHECK(json::from_ubjson(v0) == json(""));
+            }
         }
 
         SECTION("array")