Merge branch 'ximinez/number-round-maxrep-down' into ximinez/number-round-maxrep

.clang-tidy

@@ -50,13 +50,7 @@ Checks: "-*,
   performance-*,
   -performance-avoid-endl,
   -performance-enum-size,
-  -performance-inefficient-algorithm,
-  -performance-inefficient-string-concatenation,
-  -performance-no-int-to-ptr,
-  -performance-noexcept-destructor,
   -performance-noexcept-move-constructor,
-  -performance-noexcept-swap,
-  -performance-type-promotion-in-math-fn,
   -performance-unnecessary-copy-initialization,
   -performance-unnecessary-value-param,
 

include/xrpl/basics/IntrusivePointer.ipp

@@ -641,6 +641,9 @@ template <class T>
 T*
 SharedWeakUnion<T>::unsafeGetRawPtr() const
 {
+    // tp_ packs a raw pointer together with a strength bit; recovering the
+    // pointer inherently requires an integer-to-pointer cast.
+    // NOLINTNEXTLINE(performance-no-int-to-ptr)
     return reinterpret_cast<T*>(tp_ & kPtrMask);
 }
 

include/xrpl/shamap/detail/TaggedPointer.ipp

@@ -507,6 +507,9 @@ TaggedPointer::operator=(TaggedPointer&& other)
 [[nodiscard]] inline std::pair<std::uint8_t, void*>
 TaggedPointer::decode() const
 {
+    // tp_ packs a raw pointer together with the tag bits; recovering the
+    // pointer inherently requires an integer-to-pointer cast.
+    // NOLINTNEXTLINE(performance-no-int-to-ptr)
     return {tp_ & kTagMask, reinterpret_cast<void*>(tp_ & kPtrMask)};
 }
 
@@ -535,6 +538,9 @@ TaggedPointer::getHashesAndChildren() const
 [[nodiscard]] inline SHAMapHash*
 TaggedPointer::getHashes() const
 {
+    // tp_ packs a raw pointer together with the tag bits; recovering the
+    // pointer inherently requires an integer-to-pointer cast.
+    // NOLINTNEXTLINE(performance-no-int-to-ptr)
     return reinterpret_cast<SHAMapHash*>(tp_ & kPtrMask);
 };
 

src/libxrpl/json/json_reader.cpp

@@ -5,12 +5,13 @@
 
 #include <algorithm>
 #include <cctype>
+#include <charconv>
 #include <cstdint>
 #include <cstdio>
-#include <cstring>
 #include <istream>
 #include <stdexcept>
 #include <string>
+#include <system_error>
 
 namespace json {
 // Implementation of class Reader
@@ -605,36 +606,17 @@ bool
 Reader::decodeDouble(Token& token)
 {
     double value = 0;
-    int const bufferSize = 32;
-    int count = 0;
-    int const length = int(token.end - token.start);
-    // Sanity check to avoid buffer overflow exploits.
-    if (length < 0)
-    {
-        return addError("Unable to parse token length", token);
-    }
-    // Avoid using a string constant for the format control string given to
-    // sscanf, as this can cause hard to debug crashes on OS X. See here for
-    // more info:
-    //
-    // http://developer.apple.com/library/mac/#DOCUMENTATION/DeveloperTools/gcc-4.0.1/gcc/Incompatibilities.html
-    char format[] = "%lf";
-    if (length <= bufferSize)
-    {
-        Char buffer[bufferSize + 1];
-        memcpy(buffer, token.start, length);
-        buffer[length] = 0;
-        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
-        count = sscanf(buffer, format, &value);
-    }
-    else
-    {
-        std::string const buffer(token.start, token.end);
-        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
-        count = sscanf(buffer.c_str(), format, &value);
-    }
-    if (count != 1)
+    auto const [ptr, ec] = std::from_chars(token.start, token.end, value);
+
+    // Reject anything from_chars could not turn into a finite double:
+    //   - ec != std::errc{}: no valid conversion, or an out-of-range magnitude
+    //     (e.g. 1e400).
+    //   - ptr != token.end: readNumber() is permissive about which characters
+    //     it collects into a token (it will, for example, keep a '+' mid-token),
+    //     but from_chars() will stop at the first character it cannot parse.
+    if (ec != std::errc{} || ptr != token.end)
         return addError("'" + std::string(token.start, token.end) + "' is not a number.", token);
+
     currentValue() = value;
     return true;
 }

src/libxrpl/json/json_writer.cpp

@@ -4,13 +4,14 @@
 #include <xrpl/json/json_forwards.h>
 #include <xrpl/json/json_value.h>
 
-#include <cstdio>
+#include <charconv>
 #include <cstring>
 #include <iomanip>
 #include <ios>
 #include <ostream>
 #include <sstream>
 #include <string>
+#include <system_error>
 #include <utility>
 
 namespace json {
@@ -76,20 +77,18 @@ valueToString(UInt value)
 std::string
 valueToString(double value)
 {
-    // Allocate a buffer that is more than large enough to store the 16 digits
-    // of precision requested below.
+    // Format with 16 significant digits.
+    // We need not request the alternative representation that always has a
+    // decimal point because JSON doesn't distinguish the concepts of reals and integers.
+    // A double never needs more than 32 characters in this form,
+    // so to_chars cannot actually run out of room here.
     char buffer[32];
-    // Print into the buffer. We need not request the alternative representation
-    // that always has a decimal point because JSON doesn't distinguish the
-    // concepts of reals and integers.
-#if defined(_MSC_VER) && defined(__STDC_SECURE_LIB__)  // Use secure version with visual studio 2005
-                                                       // to avoid warning.
-    sprintf_s(buffer, sizeof(buffer), "%.16g", value);
-#else
-    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
-    snprintf(buffer, sizeof(buffer), "%.16g", value);
-#endif
-    return buffer;
+    auto const [ptr, ec] =
+        std::to_chars(buffer, buffer + sizeof(buffer), value, std::chars_format::general, 16);
+    XRPL_ASSERT(ec == std::errc{}, "json::valueToString(double) : conversion fits buffer");
+    if (ec != std::errc{})
+        return {};
+    return std::string(buffer, ptr);
 }
 
 std::string

src/libxrpl/protocol/STParsedJSON.cpp

@@ -69,6 +69,17 @@ toUnsigned(U2 value)
     return static_cast<U1>(value);
 }
 
+static std::string
+joinName(std::string const& jsonName, std::string const& fieldName)
+{
+    std::string result;
+    result.reserve(jsonName.size() + 1 + fieldName.size());
+    result += jsonName;
+    result += '.';
+    result += fieldName;
+    return result;
+}
+
 // LCOV_EXCL_START
 static inline std::string
 makeName(std::string const& object, std::string const& field)
@@ -76,7 +87,7 @@ makeName(std::string const& object, std::string const& field)
     if (field.empty())
         return object;
 
-    return object + "." + field;
+    return joinName(object, field);
 }
 
 static inline json::Value
@@ -1036,8 +1047,8 @@ parseObject(
 
                     try
                     {
-                        auto ret =
-                            parseObject(jsonName + "." + fieldName, value, field, depth + 1, error);
+                        auto ret = parseObject(
+                            joinName(jsonName, fieldName), value, field, depth + 1, error);
                         if (!ret)
                             return std::nullopt;
                         data.emplaceBack(std::move(*ret));
@@ -1054,8 +1065,8 @@ parseObject(
                 case STI_ARRAY:
                     try
                     {
-                        auto array =
-                            parseArray(jsonName + "." + fieldName, value, field, depth + 1, error);
+                        auto array = parseArray(
+                            joinName(jsonName, fieldName), value, field, depth + 1, error);
                         if (!array.has_value())
                             return std::nullopt;
                         data.emplaceBack(std::move(*array));

src/libxrpl/rdb/SociDB.cpp

@@ -213,6 +213,12 @@ public:
         if (auto [conn, keepAlive] = getConnection(); conn)
         {
             (void)keepAlive;
+            // The checkpointer is identified to the C callback by an integer id
+            // (resolved via checkpointerFromId) rather than a raw `this`, so it
+            // cannot dangle if the checkpointer is destroyed. Passing the id
+            // through sqlite's void* user-data requires an integer-to-pointer
+            // cast.
+            // NOLINTNEXTLINE(performance-no-int-to-ptr)
             sqlite_api::sqlite3_wal_hook(conn, &sqliteWALHook, reinterpret_cast<void*>(id_));
         }
     }

src/test/core/Config_test.cpp

@@ -1475,6 +1475,7 @@ r.ripple.com:51235
             std::string toLoad(R"xrpldConfig(
 [amendment_majority_time]
 )xrpldConfig");
+            // NOLINTNEXTLINE(performance-inefficient-string-concatenation)
             toLoad += std::to_string(val) + space + unit;
             space = space.empty() ? " " : "";
 

src/tests/libxrpl/json/Value.cpp

@@ -13,6 +13,7 @@
 #include <exception>
 #include <limits>
 #include <numbers>
+#include <optional>
 #include <regex>
 #include <sstream>
 #include <string>
@@ -592,6 +593,57 @@ TEST(json_value, bad_json)
     EXPECT_TRUE(r.parse(s, j));
 }
 
+namespace {
+
+std::optional<json::Value>
+parseValue(std::string const& doc)
+{
+    json::Value j;
+    json::Reader r;
+    if (!r.parse("{\"v\":" + doc + "}", j))
+        return std::nullopt;
+    return j["v"];
+}
+
+}  // namespace
+
+TEST(json_value, parse_double_valid)
+{
+    // 1e300 is large but still representable, so it parses (unlike the out-of-range cases below).
+    for (auto const& [text, expected] :
+         {std::pair{"2.5", 2.5},
+          std::pair{"-3.25e2", -325.0},
+          std::pair{"0.0", 0.0},
+          std::pair{"1E3", 1000.0},
+          std::pair{"-0.5e-1", -0.05},
+          std::pair{"1e300", 1e300}})
+    {
+        auto const v = parseValue(text);
+        ASSERT_TRUE(v.has_value()) << text;
+        // NOLINTBEGIN(bugprone-unchecked-optional-access)
+        EXPECT_TRUE(v->isDouble()) << text;
+        EXPECT_EQ(v->asDouble(), expected) << text;
+        // NOLINTEND(bugprone-unchecked-optional-access)
+    }
+}
+
+TEST(json_value, parse_double_out_of_range)
+{
+    // Magnitudes with no finite double representation are rejected.
+    for (char const* oor : {"1e400", "-1e400", "0.001e500", "1e-400", "-1e-400", "123e-500"})
+        EXPECT_FALSE(parseValue(oor).has_value()) << oor;
+}
+
+TEST(json_value, parse_double_malformed)
+{
+    // readNumber() collects any run of digits and '.eE+-' into a single Double
+    // token, so these malformed tokens reach decodeDouble. Each has a valid
+    // leading prefix that from_chars would accept on its own; requiring the
+    // entire token be consumed rejects them instead of silently truncating.
+    for (char const* bad : {"1+2", "1-2", "1.2.3", "1e5e6", "1..2", "++5", "1e", "1e+", ".", "-"})
+        EXPECT_FALSE(parseValue(bad).has_value()) << bad;
+}
+
 TEST(json_value, edge_cases)
 {
     std::uint32_t const maxUInt = std::numeric_limits<std::uint32_t>::max();