From df16feaa3687e8d4ee9b920a375c75b604d23cfd Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 9 Feb 2026 21:04:18 -0600 Subject: [PATCH] feat: add serializer support for std::chrono::sys_seconds, std::u8string, std::filesystem::path (#8364) * chore: rename display-mode-tests.cc as converter-tests.cc * feat: support std::chrono::sys_seconds in serializers * feat: support std::u8string, std::filesystem::path in serializer * build: address review feedback * chore: remove unnecessary helper function * Revert "chore: remove unnecessary helper function" This reverts commit 69ea9078367993bc8a00d33ce14275ecb0908a34. std::to_chars() unavailable on macOS < 13.3 We can remove this hack if/when we drop support for macOS < 13.3 --- libtransmission-app/converters.cc | 218 ++++++++++++++++++ libtransmission/serializer.cc | 44 ++++ libtransmission/utils.cc | 12 + libtransmission/utils.h | 2 + tests/libtransmission-app/CMakeLists.txt | 2 +- ...splay-mode-tests.cc => converter-tests.cc} | 65 +++++- tests/libtransmission/serializer-tests.cc | 62 +++++ tests/libtransmission/utils-test.cc | 12 + tests/qt/session-test.cc | 3 + 9 files changed, 415 insertions(+), 5 deletions(-) rename tests/libtransmission-app/{display-mode-tests.cc => converter-tests.cc} (51%) diff --git a/libtransmission-app/converters.cc b/libtransmission-app/converters.cc index 8e36b2568..e98efa4fe 100644 --- a/libtransmission-app/converters.cc +++ b/libtransmission-app/converters.cc @@ -4,11 +4,19 @@ // License text can be found in the licenses/ folder. #include +#include +#include #include +#include +#include #include #include +#include +#include + #include "libtransmission/serializer.h" +#include "libtransmission/utils.h" #include "libtransmission/variant.h" #include "libtransmission-app/display-modes.h" @@ -18,11 +26,90 @@ namespace tr::app::detail { namespace { +template +inline constexpr bool HasTmGmtoffV = requires(T t) { t.tm_gmtoff; }; + template using Lookup = std::array, N>; // --- +struct TrYearMonthDay +{ + int year = 0; + unsigned month = 0; + unsigned day = 0; + bool valid = false; + + [[nodiscard]] constexpr bool ok() const noexcept + { + return valid; + } +}; + +// c++20 (P0355) replace with std::chrono::year_month_day() after Debian 11 is EOL +[[nodiscard]] constexpr TrYearMonthDay tr_year_month_day(int year, unsigned month, unsigned day) +{ + auto const is_leap_year = [](int y) constexpr + { + return (y % 4 == 0) && ((y % 100) != 0 || (y % 400) == 0); + }; + + auto const days_in_month = [&](int y, unsigned m) constexpr + { + switch (m) + { + case 1: + case 3: + case 5: + case 7: + case 8: + case 10: + case 12: + return 31; + case 4: + case 6: + case 9: + case 11: + return 30; + case 2: + return is_leap_year(y) ? 29 : 28; + default: + return 0; + } + }; + + auto const is_valid_ymd = [&](int y, unsigned m, unsigned d) constexpr + { + if (m < 1 || m > 12) + { + return false; + } + + auto const dim = days_in_month(y, m); + return d >= 1 && d <= static_cast(dim); + }; + + return TrYearMonthDay{ year, month, day, is_valid_ymd(year, month, day) }; +} + +// c++20 (P0355) replace with std::chrono::sys_days{} after Debian 11 is EOL +// Returns days since 1970-01-01. Based on Howard Hinnant's civil calendar algorithms. +[[nodiscard]] constexpr std::chrono::sys_days tr_sys_days(TrYearMonthDay const& ymd) +{ + auto const days_from_civil = [](int year, unsigned month, unsigned day) constexpr + { + year -= static_cast(month <= 2); + auto const era = (year >= 0 ? year : year - 399) / 400; + auto const yoe = static_cast(year - (era * 400)); + auto const doy = ((153 * (month + (month > 2 ? -3 : 9)) + 2) / 5) + day - 1; + auto const doe = (yoe * 365) + (yoe / 4) - (yoe / 100) + doy; + return (static_cast(era) * 146097) + static_cast(doe) - 719468; + }; + + return std::chrono::sys_days{ std::chrono::days{ days_from_civil(ymd.year, ymd.month, ymd.day) } }; +} + auto constexpr ShowKeys = std::array, ShowModeCount>{ { { "show_active", ShowMode::ShowActive }, { "show_all", ShowMode::ShowAll }, @@ -159,6 +246,136 @@ tr_variant from_stats_mode(StatsMode const& src) return from_stats_mode(DefaultStatsMode); } + +// --- + +// c++20(P0355): use std::chrono::parse if/when it's ever available +[[nodiscard]] std::optional parse_sys_seconds(std::string_view str) +{ + auto const sv = tr_strv_strip(str); + if ((std::size(sv) != 20U && std::size(sv) != 24U && std::size(sv) != 25U) || sv[4] != '-' || sv[7] != '-' || + sv[10] != 'T' || sv[13] != ':' || sv[16] != ':') + { + return {}; + } + + auto parse_int = [](std::string_view token, int min, int max, int* out) -> bool + { + if (auto const parsed = tr_num_parse(token); parsed && *parsed >= min && *parsed <= max) + { + *out = *parsed; + return true; + } + + return false; + }; + + auto year = int{}; + auto month = int{}; + auto day = int{}; + auto hour = int{}; + auto minute = int{}; + auto second = int{}; + + if (!parse_int(sv.substr(0, 4), 0, 9999, &year) || !parse_int(sv.substr(5, 2), 1, 12, &month) || + !parse_int(sv.substr(8, 2), 1, 31, &day) || !parse_int(sv.substr(11, 2), 0, 23, &hour) || + !parse_int(sv.substr(14, 2), 0, 59, &minute) || !parse_int(sv.substr(17, 2), 0, 59, &second)) + { + return {}; + } + + auto const ymd = tr_year_month_day(year, static_cast(month), static_cast(day)); + if (!ymd.ok()) + { + return {}; + } + + auto const day_point = std::chrono::time_point_cast(tr_sys_days(ymd)); + auto const local_tp = day_point + std::chrono::hours{ hour } + std::chrono::minutes{ minute } + + std::chrono::seconds{ second }; + + if (std::size(sv) == 20U) + { + if (sv[19] != 'Z') + { + return {}; + } + + return std::chrono::sys_seconds{ local_tp }; + } + + auto const sign = sv[19]; + if (sign != '+' && sign != '-') + { + return {}; + } + + auto off_hours = int{}; + auto off_minutes = int{}; + + if (std::size(sv) == 24U) + { + if (!parse_int(sv.substr(20, 2), 0, 23, &off_hours) || !parse_int(sv.substr(22, 2), 0, 59, &off_minutes)) + { + return {}; + } + } + else + { + if (sv[22] != ':' || !parse_int(sv.substr(20, 2), 0, 23, &off_hours) || + !parse_int(sv.substr(23, 2), 0, 59, &off_minutes)) + { + return {}; + } + } + + auto const offset = std::chrono::minutes{ (off_hours * 60) + off_minutes } * (sign == '-' ? -1 : 1); + return std::chrono::sys_seconds{ local_tp - offset }; +} + +[[nodiscard]] std::string format_sys_seconds(std::chrono::sys_seconds const& src) +{ + auto const tp = std::chrono::time_point_cast(src); + auto const tt = std::chrono::system_clock::to_time_t(tp); + + // prefer localtime with TZ offset data when we can get it. + if constexpr (HasTmGmtoffV) + { + if (auto const* local = std::localtime(&tt)) + { + return fmt::format(FMT_STRING("{:%FT%T%z}"), *local); + } + } + + return fmt::format(FMT_STRING("{:%FT%TZ}"), src); +} + +bool to_sys_seconds(tr_variant const& src, std::chrono::sys_seconds* tgt) +{ + if (auto const val = src.value_if()) + { + if (auto const parsed = parse_sys_seconds(*val); parsed) + { + *tgt = *parsed; + return true; + } + } + + if (auto const val = src.value_if()) + { + auto const tp = std::chrono::system_clock::from_time_t(static_cast(*val)); + *tgt = std::chrono::time_point_cast(tp); + return true; + } + + return false; +} + +tr_variant from_sys_seconds(std::chrono::sys_seconds const& src) +{ + auto const formatted = format_sys_seconds(src); + return tr_variant{ formatted }; +} } // unnamed namespace void register_app_converters() @@ -172,6 +389,7 @@ void register_app_converters() Converters::add(to_show_mode, from_show_mode); Converters::add(to_sort_mode, from_sort_mode); Converters::add(to_stats_mode, from_stats_mode); + Converters::add(to_sys_seconds, from_sys_seconds); }); } diff --git a/libtransmission/serializer.cc b/libtransmission/serializer.cc index 0c4fbec6a..31c0bfc0a 100644 --- a/libtransmission/serializer.cc +++ b/libtransmission/serializer.cc @@ -8,6 +8,7 @@ #include #include // size_t #include // int64_t, uint32_t, uint64_t +#include #include #include #include @@ -463,6 +464,47 @@ tr_variant from_verify_added_mode(tr_verify_added_mode const& val) { return from_enum_or_integral_with_lookup(VerifyModeKeys, val); } + +// --- + +bool to_u8string(tr_variant const& src, std::u8string* tgt) +{ + if (auto const val = src.value_if()) + { + if (tr_strv_find_invalid_utf8(*val) != std::string_view::npos) + { + tr_logAddWarn(fmt::format(fmt::runtime(_("String '{string}' contains invalid UTF-8")), fmt::arg("string", *val))); + } + + *tgt = tr_strv_to_u8string(tr_strv_replace_invalid(*val)); + return true; + } + + return false; +} + +tr_variant from_u8string(std::u8string const& val) +{ + return std::string{ reinterpret_cast(std::data(val)), std::size(val) }; +} + +// --- + +bool to_fs_path(tr_variant const& src, std::filesystem::path* tgt) +{ + if (auto u8str = std::u8string{}; to_u8string(src, &u8str)) + { + *tgt = std::filesystem::path{ u8str }; + return true; + } + + return false; +} + +tr_variant from_fs_path(std::filesystem::path const& path) +{ + return from_u8string(path.u8string()); +} } // unnamed namespace void Converters::ensure_default_converters() @@ -476,6 +518,7 @@ void Converters::ensure_default_converters() Converters::add(to_diffserv_t, from_diffserv_t); Converters::add(to_double, from_double); Converters::add(to_encryption_mode, from_encryption_mode); + Converters::add(to_fs_path, from_fs_path); Converters::add(to_int64, from_int64); Converters::add(to_log_level, from_log_level); Converters::add(to_mode_t, from_mode_t); @@ -485,6 +528,7 @@ void Converters::ensure_default_converters() Converters::add(to_preferred_transport, from_preferred_transport); Converters::add(to_size_t, from_size_t); Converters::add(to_string, from_string); + Converters::add(to_u8string, from_u8string); Converters::add(to_uint64, from_uint64); Converters::add(to_verify_added_mode, from_verify_added_mode); }); diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc index 76c418d10..ad73d7e70 100644 --- a/libtransmission/utils.cc +++ b/libtransmission/utils.cc @@ -305,6 +305,11 @@ std::string tr_strv_to_utf8_string(std::string_view sv) #endif +std::string_view::size_type tr_strv_find_invalid_utf8(std::string_view const sv) +{ + return utf8::find_invalid(sv); +} + std::string tr_strv_replace_invalid(std::string_view sv, uint32_t replacement) { // stripping characters after first \0 @@ -318,6 +323,13 @@ std::string tr_strv_replace_invalid(std::string_view sv, uint32_t replacement) return out; } +std::u8string tr_strv_to_u8string(std::string_view const sv) +{ + auto u8str = tr_strv_to_utf8_string(sv); + auto const view = std::views::transform(u8str, [](char c) -> char8_t { return c; }); + return { view.begin(), view.end() }; +} + #ifdef _WIN32 std::string tr_win32_native_to_utf8(std::wstring_view in) diff --git a/libtransmission/utils.h b/libtransmission/utils.h index 62406394e..faa3279cf 100644 --- a/libtransmission/utils.h +++ b/libtransmission/utils.h @@ -193,6 +193,7 @@ constexpr bool tr_strv_sep(std::string_view* sv, std::string_view* token, Args&& [[nodiscard]] std::string_view tr_strv_strip(std::string_view str); [[nodiscard]] std::string tr_strv_to_utf8_string(std::string_view sv); +[[nodiscard]] std::u8string tr_strv_to_u8string(std::string_view sv); #ifdef __APPLE__ #ifdef __OBJC__ @@ -203,6 +204,7 @@ constexpr bool tr_strv_sep(std::string_view* sv, std::string_view* token, Args&& #endif #endif +[[nodiscard]] std::string_view::size_type tr_strv_find_invalid_utf8(std::string_view sv); [[nodiscard]] std::string tr_strv_replace_invalid(std::string_view sv, uint32_t replacement = 0xFFFD /*�*/); // --- diff --git a/tests/libtransmission-app/CMakeLists.txt b/tests/libtransmission-app/CMakeLists.txt index f143bc4a6..4791bec30 100644 --- a/tests/libtransmission-app/CMakeLists.txt +++ b/tests/libtransmission-app/CMakeLists.txt @@ -4,7 +4,7 @@ add_executable(libtransmission-app-test) target_sources(libtransmission-app-test PRIVATE - display-mode-tests.cc + converter-tests.cc test-fixtures.h) set_property( diff --git a/tests/libtransmission-app/display-mode-tests.cc b/tests/libtransmission-app/converter-tests.cc similarity index 51% rename from tests/libtransmission-app/display-mode-tests.cc rename to tests/libtransmission-app/converter-tests.cc index 57621f594..6a3ddd587 100644 --- a/tests/libtransmission-app/display-mode-tests.cc +++ b/tests/libtransmission-app/converter-tests.cc @@ -4,6 +4,9 @@ // License text can be found in the licenses/ folder. #include +#include +#include +#include #include #include @@ -15,12 +18,36 @@ #include "test-fixtures.h" -using DisplayModeTest = TransmissionTest; +using ConverterTest = TransmissionTest; using namespace std::literals; using tr::serializer::Converters; namespace { +constexpr int64_t days_from_civil(int year, unsigned month, unsigned day) +{ + year -= month <= 2; + auto const era = (year >= 0 ? year : year - 399) / 400; + auto const yoe = static_cast(year - era * 400); + auto const doy = (153 * (month + (month > 2 ? -3 : 9)) + 2) / 5 + day - 1; + auto const doe = yoe * 365 + yoe / 4 - yoe / 100 + doy; + return static_cast(era) * 146097 + static_cast(doe) - 719468; +} + +constexpr std::chrono::sys_seconds make_sys_seconds(int year, int month, int day, int hour, int minute, int second) +{ + return std::chrono::sys_seconds{ std::chrono::seconds{ days_from_civil(year, month, day) * 86400 } } + + std::chrono::hours{ hour } + std::chrono::minutes{ minute } + std::chrono::seconds{ second }; +} + +void expect_sys_seconds_eq(std::optional const& actual, std::chrono::sys_seconds expected) +{ + EXPECT_TRUE(actual.has_value()); + if (actual.has_value()) + { + EXPECT_EQ(actual->time_since_epoch().count(), expected.time_since_epoch().count()); + } +} template void testModeRoundtrip(std::array, N> const& items) @@ -39,7 +66,7 @@ void testModeRoundtrip(std::array, N> const& item } // namespace -TEST_F(DisplayModeTest, showModeStringsRoundtrip) +TEST_F(ConverterTest, showModeStringsRoundtrip) { auto constexpr Items = std::array, tr::app::ShowModeCount>{ { { "show_active", tr::app::ShowMode::ShowActive }, @@ -55,7 +82,7 @@ TEST_F(DisplayModeTest, showModeStringsRoundtrip) testModeRoundtrip(Items); } -TEST_F(DisplayModeTest, sortModeStringsRoundtrip) +TEST_F(ConverterTest, sortModeStringsRoundtrip) { auto constexpr Items = std::array, tr::app::SortModeCount>{ { { "sort_by_activity", tr::app::SortMode::SortByActivity }, @@ -73,7 +100,7 @@ TEST_F(DisplayModeTest, sortModeStringsRoundtrip) testModeRoundtrip(Items); } -TEST_F(DisplayModeTest, statsModeStringsRoundtrip) +TEST_F(ConverterTest, statsModeStringsRoundtrip) { auto constexpr Items = std::array, tr::app::StatsModeCount>{ { { "total_ratio", tr::app::StatsMode::TotalRatio }, @@ -84,3 +111,33 @@ TEST_F(DisplayModeTest, statsModeStringsRoundtrip) testModeRoundtrip(Items); } + +TEST_F(ConverterTest, sysSecondsRoundtrip) +{ + using namespace std::chrono; + using namespace tr::serializer; + + auto constexpr Expected = make_sys_seconds(2024, 2, 3, 4, 5, 6); + auto const var = Converters::serialize(Expected); + EXPECT_TRUE(var.holds_alternative()); + auto const serialized = var.value_if().value_or(""sv); + static auto const re = std::regex(R"(^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:Z|[+-]\d{4})$)"); + EXPECT_TRUE(std::regex_match(std::string{ serialized }, re)); + + auto actual = to_value(tr_variant{ serialized }); + expect_sys_seconds_eq(actual, Expected); + + actual = to_value(tr_variant{ "2024-02-03T04:05:06Z"sv }); + expect_sys_seconds_eq(actual, Expected); + + actual = to_value(tr_variant{ "2024-02-03T04:05:06+00:00"sv }); + expect_sys_seconds_eq(actual, Expected); + + actual = to_value(tr_variant{ "2024-02-03T04:05:06+02:30"sv }); + expect_sys_seconds_eq(actual, Expected - (hours{ 2 } + minutes{ 30 })); + + auto constexpr Epoch = int64_t{ 1700000000 }; + auto const epoch_seconds = time_point_cast(system_clock::from_time_t(static_cast(Epoch))); + actual = to_value(tr_variant{ Epoch }); + expect_sys_seconds_eq(actual, epoch_seconds); +} diff --git a/tests/libtransmission/serializer-tests.cc b/tests/libtransmission/serializer-tests.cc index 746c27aca..6a6b81d6d 100644 --- a/tests/libtransmission/serializer-tests.cc +++ b/tests/libtransmission/serializer-tests.cc @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -13,6 +14,7 @@ #include #include +#include #include #include #include @@ -26,6 +28,11 @@ using tr::serializer::Converters; namespace { +[[nodiscard]] std::string toString(std::u8string const& value) +{ + return { reinterpret_cast(std::data(value)), std::size(value) }; +} + struct Rect { int x = 0; @@ -147,6 +154,61 @@ TEST_F(SerializerTest, usesBuiltins) } } +TEST_F(SerializerTest, usesU8String) +{ + auto const expected = std::u8string{ u8"hello" }; + auto const var = Converters::serialize(expected); + EXPECT_TRUE(var.holds_alternative()); + EXPECT_EQ(var.value_if().value_or(""sv), "hello"sv); + + auto actual = std::u8string{}; + EXPECT_TRUE(Converters::deserialize(var, &actual)); + EXPECT_EQ(toString(actual), toString(expected)); +} + +TEST_F(SerializerTest, usesFsPath) +{ + auto const expected = std::filesystem::path{ std::u8string{ u8"foo/βar" } }; + auto const var = Converters::serialize(expected); + EXPECT_TRUE(var.holds_alternative()); + EXPECT_EQ(var.value_if().value_or(""sv), "foo/βar"sv); + + auto actual = std::filesystem::path{}; + EXPECT_TRUE(Converters::deserialize(var, &actual)); + EXPECT_EQ(toString(actual.u8string()), toString(expected.u8string())); +} + +TEST_F(SerializerTest, u8StringWarnsOnInvalidUtf8) +{ + auto const bad = std::string{ static_cast(0xC3), static_cast(0x28) }; + auto const var = tr_variant{ std::string_view{ bad } }; + + auto const old_level = tr_logGetLevel(); + tr_logSetLevel(TR_LOG_WARN); + tr_logSetQueueEnabled(true); + tr_logFreeQueue(tr_logGetQueue()); + + auto actual = std::u8string{}; + EXPECT_TRUE(Converters::deserialize(var, &actual)); + + auto* const msgs = tr_logGetQueue(); + auto warned = false; + for (auto* msg = msgs; msg != nullptr; msg = msg->next) + { + if (msg->level == TR_LOG_WARN && msg->message.find("contains invalid UTF-8") != std::string::npos) + { + warned = true; + break; + } + } + + tr_logFreeQueue(msgs); + tr_logSetQueueEnabled(false); + tr_logSetLevel(old_level); + + EXPECT_TRUE(warned); +} + TEST_F(SerializerTest, usesCustomTypes) { registerRectConverter(); diff --git a/tests/libtransmission/utils-test.cc b/tests/libtransmission/utils-test.cc index 5becce87f..4d06bad34 100644 --- a/tests/libtransmission/utils-test.cc +++ b/tests/libtransmission/utils-test.cc @@ -204,6 +204,18 @@ TEST_F(UtilsTest, strvReplaceInvalid) EXPECT_EQ(out, tr_strv_replace_invalid(out)); } +TEST_F(UtilsTest, strvFindInvalidUtf8) +{ + EXPECT_EQ(std::string_view::npos, tr_strv_find_invalid_utf8("hello"sv)); + EXPECT_EQ(std::string_view::npos, tr_strv_find_invalid_utf8("Трудно быть Богом"sv)); + + auto const invalid = std::string{ static_cast(0xC3), static_cast(0x28) }; + EXPECT_EQ(0U, tr_strv_find_invalid_utf8(invalid)); + + auto const mixed = std::string{ "ok " } + invalid + " end"; + EXPECT_EQ(3U, tr_strv_find_invalid_utf8(mixed)); +} + TEST_F(UtilsTest, strvConvertUtf8Fuzz) { auto buf = std::vector{}; diff --git a/tests/qt/session-test.cc b/tests/qt/session-test.cc index eea8891dd..70c923b74 100644 --- a/tests/qt/session-test.cc +++ b/tests/qt/session-test.cc @@ -45,6 +45,9 @@ Q_DECLARE_METATYPE(Style) .arg(dir) }; } + + abort(); + return {}; } class SessionTest