]> git.ipfire.org Git - thirdparty/FORT-validator.git/commit
Fix date parsing from TAL cachefiles
authorAlberto Leiva Popper <ydahhrk@gmail.com>
Fri, 15 Mar 2024 22:44:07 +0000 (16:44 -0600)
committerAlberto Leiva Popper <ydahhrk@gmail.com>
Sat, 16 Mar 2024 00:24:06 +0000 (18:24 -0600)
commit7846f008a288c3913d7c30f22781544cde84b0b3
treea4a8acd38ee805a9fe803522ae561a7707586bab
parent6d7985b72fa3462d311aa9421e1342fcaa2deef6
Fix date parsing from TAL cachefiles

The code was writing dates in Zulu format, which was fine. But then, to
read them back, it was loading them with mktime(), which is a local
timezone function. The effects of this bug depend on the time zone.
Files would expire from the cache up to 12 hours too early (UTC+) or
late (UTC-), or be updated up to 12 hours late (UTC-). (There's no such
thing as "updating too early" in this context, since Fort cannot refresh
before the present.)

I fixed this by swapping mktime() for timegm(), which is not great,
because the latter is still a nonstandard function. But it'll have to
do, because the other options are worse.

Below is my thought process on timegm() and the other options:

==================================================

Problem:

1. I want to store timestamps as human-readable strings.
2. Converting a timestamp to Zulu string is trivial: time_t ->
   gmtime_r() -> struct tm (GMT) -> strftime() -> char*.
3. But converting a string to timestamp relies on timegm(), which is
   nonstandard: char* -> strptime() -> struct tm (GMT) -> timegm() ->
   time_t.

Brainstorm:

1. Store the dates in local time.

Hideous option, but not ENTIRELY insane.

Storing in local time will render the dates up to 24 hours off, I think.
But this only happens when the cache changes timezone, which isn't
really often.

But it's still pretty clumsy, and also not future-proof, as new date
fields would have to be constrained by the same limitation.

2/10 at best.

2. Ditch time_t, use struct tm in UTC for everything.

So I would rely on gmtime_r() only to get out of time_t, and never need
timegm() for anything.

Comparing field by field would be a pain, but it's interesting to note
that Fort is actually already doing it somewhere: tm_cmp(). I guess I
have to admire the audaciousness of past me.

What mainly scares me is that mktime() seems to be the only standard
function that is contractually obligated to normalize the tm, which
means I would have to keep mktime()ing every time I need to compare
them.

And mktime() is... a local time function. Probably wouldn't actually
work.

4/10. I hate this API.

3. Bite the bullet: Go modern POSIX; assume time_t is integer seconds
   since the 1970 epoch.

I'm fantasizing about an early environment assertion that checks the
gmtime_r() for a known time_t, that shows people the door if the
implementation isn't sane. Would work even in POSIX-adjacent systems
like most Linuxes.

This would have other benefits, like the ability to ditch difftime(),
and perform arithmetic operations on time_t's.

All the officially supported platforms get this aspect of POSIX right,
so what's stopping me?

Well, it would mean I would have to store the date as a seconds-since-
epoch integer, which is not as human-friendly and defeats the whole
point of the JSON format.

And yet... this feels so right, I might end up doing it even regardless
of the JSON data type and timegm().

But not today. 7/10.

4. Bite the bullet: Use timegm().

Interesting. timegm() might be added to C23:

> Changes integrated into the latest working draft of C23 are listed
> below.
> (...)
> - Add timegm() function in <time.h> to convert time structure into
> calendar time value - similar to function in glibc and musl libraries.

https://en.wikipedia.org/wiki/C23_(C_standard_revision)

So this has some probability of not needing future tweaking. Also, it's
very clean. (Except for the feature test macros.)

8/10.
src/Makefile.am
src/cache/local_cache.c
src/json_util.c
src/json_util.h
test/Makefile.am
test/json_util_test.c [new file with mode: 0644]