From 094ed8f2993d826710c04f5767cfa45f2b43d19b Mon Sep 17 00:00:00 2001 From: cyan317 <120398799+cindyyan317@users.noreply.github.com> Date: Tue, 9 Jul 2024 13:14:22 +0100 Subject: [PATCH] fix: Fix the ledger_index timezone issue (#1522) Fix unittest failures --- src/rpc/common/Validators.cpp | 7 ++- src/rpc/handlers/LedgerIndex.cpp | 15 ++---- src/util/CMakeLists.txt | 1 + src/util/TimeUtils.cpp | 42 ++++++++++++++++ src/util/TimeUtils.hpp | 46 ++++++++++++++++++ tests/unit/CMakeLists.txt | 1 + tests/unit/rpc/handlers/LedgerIndexTests.cpp | 18 +++++++ tests/unit/util/TimeUtilsTests.cpp | 51 ++++++++++++++++++++ 8 files changed, 167 insertions(+), 14 deletions(-) create mode 100644 src/util/TimeUtils.cpp create mode 100644 src/util/TimeUtils.hpp create mode 100644 tests/unit/util/TimeUtilsTests.cpp diff --git a/src/rpc/common/Validators.cpp b/src/rpc/common/Validators.cpp index 5488259a..4eb4ab86 100644 --- a/src/rpc/common/Validators.cpp +++ b/src/rpc/common/Validators.cpp @@ -22,6 +22,7 @@ #include "rpc/Errors.hpp" #include "rpc/RPCHelpers.hpp" #include "rpc/common/Types.hpp" +#include "util/TimeUtils.hpp" #include #include @@ -65,10 +66,8 @@ TimeFormatValidator::verify(boost::json::value const& value, std::string_view ke if (not value.as_object().at(key).is_string()) return Error{Status{RippledError::rpcINVALID_PARAMS}}; - std::tm time = {}; - std::stringstream stream(value_to(value.as_object().at(key))); - stream >> std::get_time(&time, format_.c_str()); - if (stream.fail()) + auto const ret = util::SystemTpFromUTCStr(value_to(value.as_object().at(key)), format_); + if (!ret) return Error{Status{RippledError::rpcINVALID_PARAMS}}; return {}; diff --git a/src/rpc/handlers/LedgerIndex.cpp b/src/rpc/handlers/LedgerIndex.cpp index 57ca7d10..b306ab08 100644 --- a/src/rpc/handlers/LedgerIndex.cpp +++ b/src/rpc/handlers/LedgerIndex.cpp @@ -22,19 +22,16 @@ #include "rpc/Errors.hpp" #include "rpc/JS.hpp" #include "rpc/common/Types.hpp" +#include "util/TimeUtils.hpp" #include #include #include -#include #include #include #include -#include #include -#include -#include #include #include #include @@ -61,18 +58,16 @@ LedgerIndexHandler::process(LedgerIndexHandler::Input input, Context const& ctx) return fillOutputByIndex(maxIndex); auto const convertISOTimeStrToTicks = [](std::string const& isoTimeStr) { - std::tm time = {}; - std::stringstream ss(isoTimeStr); - ss >> std::get_time(&time, DATE_FORMAT); - return std::chrono::system_clock::from_time_t(std::mktime(&time)).time_since_epoch().count(); + auto const systemTime = util::SystemTpFromUTCStr(isoTimeStr, DATE_FORMAT); + // systemTime must be valid after validation passed + return systemTime->time_since_epoch().count(); }; auto const ticks = convertISOTimeStrToTicks(*input.date); auto const earlierThan = [&](std::uint32_t ledgerIndex) { auto const header = sharedPtrBackend_->fetchLedgerBySequence(ledgerIndex, ctx.yield); - auto const ledgerTime = - std::chrono::system_clock::time_point{header->closeTime.time_since_epoch() + ripple::epoch_offset}; + auto const ledgerTime = util::SystemTpFromLedgerCloseTime(header->closeTime); return ticks < ledgerTime.time_since_epoch().count(); }; diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 9a9092df..f32af5c0 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -20,6 +20,7 @@ target_sources( requests/impl/SslContext.cpp Taggable.cpp TerminationHandler.cpp + TimeUtils.cpp TxUtils.cpp LedgerUtils.cpp ) diff --git a/src/util/TimeUtils.cpp b/src/util/TimeUtils.cpp new file mode 100644 index 00000000..b2e7b547 --- /dev/null +++ b/src/util/TimeUtils.cpp @@ -0,0 +1,42 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include "util/TimeUtils.hpp" + +#include + +namespace util { +[[nodiscard]] std::optional +SystemTpFromUTCStr(std::string const& dateStr, std::string const& format) +{ + std::tm timeStruct; + auto const ret = strptime(dateStr.c_str(), format.c_str(), &timeStruct); + if (ret == nullptr) { + return std::nullopt; + } + return std::chrono::system_clock::from_time_t(timegm(&timeStruct)); +} + +[[nodiscard]] std::chrono::system_clock::time_point +SystemTpFromLedgerCloseTime(ripple::NetClock::time_point closeTime) +{ + return std::chrono::system_clock::time_point{closeTime.time_since_epoch() + ripple::epoch_offset}; +} + +} // namespace util diff --git a/src/util/TimeUtils.hpp b/src/util/TimeUtils.hpp new file mode 100644 index 00000000..b6320a89 --- /dev/null +++ b/src/util/TimeUtils.hpp @@ -0,0 +1,46 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#pragma once + +#include + +#include +#include + +namespace util { + +/** + * @brief Convert a UTC date string to a system_clock::time_point if possible. + * @param dateStr The UTC date string to convert. + * @param format The format of the date string. + * @return The system_clock::time_point if the conversion was successful, otherwise std::nullopt. + */ +[[nodiscard]] std::optional +SystemTpFromUTCStr(std::string const& dateStr, std::string const& format); + +/** + * @brief Convert a ledger close time which is XRPL network clock to a system_clock::time_point. + * @param closeTime The ledger close time to convert. + * @return The system_clock::time_point. + */ +[[nodiscard]] std::chrono::system_clock::time_point +SystemTpFromLedgerCloseTime(ripple::NetClock::time_point closeTime); + +} // namespace util diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 38babe30..349537e4 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -119,6 +119,7 @@ target_sources( util/RandomTests.cpp util/RetryTests.cpp util/SignalsHandlerTests.cpp + util/TimeUtilsTests.cpp util/TxUtilTests.cpp # Webserver web/AdminVerificationTests.cpp diff --git a/tests/unit/rpc/handlers/LedgerIndexTests.cpp b/tests/unit/rpc/handlers/LedgerIndexTests.cpp index 128a870f..96a3f20a 100644 --- a/tests/unit/rpc/handlers/LedgerIndexTests.cpp +++ b/tests/unit/rpc/handlers/LedgerIndexTests.cpp @@ -90,6 +90,24 @@ TEST_F(RPCLedgerIndexTest, EarlierThanMinLedger) }); } +TEST_F(RPCLedgerIndexTest, ChangeTimeZone) +{ + setenv("TZ", "EST+5", 1); + backend->setRange(RANGEMIN, RANGEMAX); + auto const handler = AnyHandler{LedgerIndexHandler{backend}}; + auto const req = json::parse(R"({"date": "2024-06-25T12:23:05Z"})"); + auto const ledgerHeader = + CreateLedgerHeaderWithUnixTime(LEDGERHASH, RANGEMIN, 1719318190); //"2024-06-25T12:23:10Z" + EXPECT_CALL(*backend, fetchLedgerBySequence(RANGEMIN, _)).WillOnce(Return(ledgerHeader)); + runSpawn([&](auto yield) { + auto const output = handler.process(req, Context{yield}); + ASSERT_FALSE(output); + auto const err = rpc::makeError(output.result.error()); + EXPECT_EQ(err.at("error").as_string(), "lgrNotFound"); + }); + unsetenv("TZ"); +} + struct LedgerIndexTestsCaseBundle { std::string testName; std::string json; diff --git a/tests/unit/util/TimeUtilsTests.cpp b/tests/unit/util/TimeUtilsTests.cpp new file mode 100644 index 00000000..64df3f1d --- /dev/null +++ b/tests/unit/util/TimeUtilsTests.cpp @@ -0,0 +1,51 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include "util/TimeUtils.hpp" + +#include + +TEST(TimeUtilTests, SystemTpFromUTCStrSuccess) +{ + auto const tp = util::SystemTpFromUTCStr("2024-01-01T10:50:40Z", "%Y-%m-%dT%H:%M:%SZ"); + ASSERT_TRUE(tp.has_value()); + auto const time = std::chrono::system_clock::to_time_t(tp.value()); + std::tm timeStruct; + gmtime_r(&time, &timeStruct); + EXPECT_EQ(timeStruct.tm_year + 1900, 2024); + EXPECT_EQ(timeStruct.tm_mon, 0); + EXPECT_EQ(timeStruct.tm_mday, 1); + EXPECT_EQ(timeStruct.tm_hour, 10); + EXPECT_EQ(timeStruct.tm_min, 50); + EXPECT_EQ(timeStruct.tm_sec, 40); +} + +TEST(TimeUtilTests, SystemTpFromUTCStrFail) +{ + auto const tp = util::SystemTpFromUTCStr("2024-01-01T", "%Y-%m-%dT%H:%M:%SZ"); + ASSERT_FALSE(tp.has_value()); +} + +TEST(TimeUtilTests, SystemTpFromLedgerCloseTime) +{ + using namespace std::chrono; + + auto const tp = util::SystemTpFromLedgerCloseTime(ripple::NetClock::time_point{seconds{0}}); + EXPECT_EQ(tp.time_since_epoch(), ripple::epoch_offset); +}