mariusbancila / croncpp

A C++11/14/17 header-only cross-platform library for handling CRON expressions
MIT License
232 stars 68 forks source link

cron_next with time_t parameter produces next time in the past #23

Open enrico-samknows opened 2 years ago

enrico-samknows commented 2 years ago

Hi, I'm trying to use croncpp and it looks very good. The only issue I see is with time_t input and timezone. For example if I call setenv to use TZ as Europe/Rome, then I call the time_t function with current timestamp, a cron string like "/5 " gives a next_time in the past. The problem is that the function takes a UTC time_t as input and then converts it as if it was a local time.

mariusbancila commented 2 years ago

Can you please provide a minimal example with what you have, and result you get and what you expect?

enrico-samknows commented 2 years ago

Sure. Here is my test function:

    ....
    setenv("TZ", timezone.c_str(), 1); // POSIX-specific
    std::cout << "TZ set to " << timezone << std::endl;
    std::cout << "Cron string: " << cron_string << std::endl;

    auto cron_timing = cron::make_cron(cron_string);
    time_t next = cron::cron_next(cron_timing, time);

    if (next > time) {
        return next;
    }
    throw std::runtime_error(std::string("Cron evaluation error: result not positive; input [") + cron_string + "] [" +
                             timezone + "] [" + std::to_string(time) + "]; output [" + std::to_string(next) + "]");

and I added some debug into cron_next:


template<typename Traits = cron_standard_traits>
static std::time_t
cron_next(cronexpr const& cex, std::time_t const& date)
{
    std::tm val;

    memset(&val, 0, sizeof(val));

    std::cout << "Input time: " << date << std::endl << "Initial val after memset with 0: " << std::asctime(&val);

    std::tm* dt = localtime_r(&date, &val);
    if (dt == nullptr)
        return INVALID_TIME;

    std::cout << "Converted with localtime_r: " << std::asctime(dt);

    time_t original = std::mktime(dt);

    std::cout << "Original: " << original << std::endl;

    if (INVALID_TIME == original)
        return INVALID_TIME;

    if (!detail::find_next<Traits>(cex, *dt, dt->tm_year))
        return INVALID_TIME;

    time_t calculated = std::mktime(dt);
    if (INVALID_TIME == calculated)
        return calculated;

    if (calculated == original) {
        add_to_field(*dt, detail::cron_field::second, 1);
        if (!detail::find_next<Traits>(cex, *dt, dt->tm_year))
            return INVALID_TIME;
    }

    std::cout << "dt: " << std::asctime(dt);

    auto res = std::mktime(dt);

    std::cout << "time_t result: " << res << std::endl;
    return res;
}

when running, I get:


TZ set to CET-1CEST,M3.5.0,M10.5.0/3
Cron string: 5 * * * * *
Input time: 1640099471
Initial val after memset with 0: Sun Jan  0 00:00:00 1900
Converted with localtime_r: Tue Dec 21 15:11:11 2021
Original: 1640095871
dt: Tue Dec 21 15:12:05 2021
time_t result: 1640095925
-------------------------------------------------------------------------------
task_runner cron
  Cron test: */5 CET
-------------------------------------------------------------------------------
test.cpp:171
...............................................................................

test:179: FAILED:
  CATCH_REQUIRE( expected_next_time == cron_eval_next_time_time_t(cron_string, timezone, test_time) )
due to unexpected exception with message:
  Cron evaluation error: result not positive; input [5 * * * * *] [CET-1CEST,
  M3.5.0,M10.5.0/3] [1640099471]; output [1640095925]

my input and expected output are:

auto test_time = 1640099471;                                 // Tue Dec 21 2021 15:11:11 GMT+0000
constexpr auto expected_next_time = 1640099525;              // Tue Dec 21 2021 15:12:05 GMT+0000

Is this clear enough?

To me it looks like a wrong conversion is made between unixtime and localized time. 1640099471 is 15:11 GMT. It is converted to a 15:11 std::tm structure, then the cron evaluation is made on that time. Then the std::tm result is converted back from 15:12:05 (assuming that it's a "CEST" localized time) to GMT and the final output is 1640095925 (14:12 GMT), which is earlier than the 15:11 GMT starting time. Thanks!

enrico-samknows commented 2 years ago

Well, everything is fixed if I add a call to tzset(); right after setenv("TZ", timezone.c_str(), 1); Probably the explanation is here: https://pubs.opengroup.org/onlinepubs/9699919799/functions/tzset.html

APPLICATION USAGE Since the ctime(), localtime(), mktime(), strftime(), and strftime_l() functions are required to set timezone information as if by calling tzset(), there is no need for an explicit tzset() call before using these functions. However, portable applications should call tzset() explicitly before using ctime_r() or localtime_r() because setting timezone information is optional for those functions.