PHPC-2429: Fix UTCDateTime with negative timestamps

[alcaeus]

PHPC-2429

When using negative milliseconds, the DateTime instance created by toDateTime is broken with negative milliseconds. Directly initialising the instance with a decimal timestamp fixes this issue.

[jmikola]

Opened alcaeus#5 to suggest using int math here so we don't have to consider edge cases from floating point division. I'll defer to you if you'd like to accept/modify it.

+/* Initialize a DateTime using "Unix Timestamp with microseconds" notation.
+ * PHP 7.4 expects exactly six points of precision to denote microseconds.
+ * PHP 8.0+ accepts between zero and six points of precision. */
+sec_len = spprintf(&sec, 0, "@%" PRId64 ".%.6d", intern->milliseconds / 1000, abs((int) (intern->milliseconds % 1000) * 1000));
-sec_len = spprintf(&sec, 0, "@%.6f", (double) intern->milliseconds / 1000);

[jmikola]

@alcaeus: I just discovered that dates smaller than one second before the epoch cause both the original PR and my patch to fail. Consider the following test:

// One millisecond before the epoch
$datetime = new \DateTimeImmutable('1969-12-31 23:59:59.999');
echo $datetime->format(DateTimeInterface::RFC3339_EXTENDED), PHP_EOL;

$utcdatetime = new MongoDB\BSON\UTCDateTime($datetime);
echo $utcdatetime, PHP_EOL;

echo $utcdatetime->toDateTime()->format(DateTimeInterface::RFC3339_EXTENDED), PHP_EOL;

This yields:

1969-12-31T23:59:59.999+00:00
-1
1970-01-01T00:00:00.001+00:00

I'll continue playing with this.

[jmikola]

The failure appears to be present in PHP versions before 8.1.7, where timelib is unable to correctly parse negative, fractional timestamps. See php/php-src#7758 and derickr/timelib#123.

I'll try to come up with a conditional workaround for the affected versions, since we're some time away from requiring PHP 8.2+.

[alcaeus]

We'll be dropping support for PHP 7.4 and 8.1 after the release of 1.20.0. It wouldn't be a problem to require PHP 8.1.7 as a minimum to completely avoid the issue.

I have no problem with a partial bug fix in this case since it affects a very narrow use case on old PHP versions that we're dropping support for anyways. Let me know what you think.

[alcaeus]

So, after investigating the issue a little more, I've decided that it's probably best to avoid any printf logic until we're requiring PHP 8.1.6+ as it's just not worth the hassle. Even then, if we want to handle seconds and microseconds separately as your patch to this PR suggested, we'll always have to account for this edge case of negative timestamps less than one second.

So, I've decided to do away with microseconds in the call to spprintf, and instead decrement the number of seconds myself for negative timestamps and ensure that the number of microseconds get adjusted accordingly (i.e. usec_new = 1000000 - usec). I've also added the date from your example above to ensure we're protected against this regression.

[jmikola]

I know this code has worked for some time, but it does differ from the timestampms pattern case, which sets relative.us instead of us as we do here.

[jmikola]

Do you need to include <inttypes.h> for this (see: cppreference). I imagine it may be included already through some sequence of libmongoc/php includes.

[alcaeus]

Good point. It worked without, but I've added an explicit include so we don't have to rely on other headers including this for us.