RFC 3339 Z suffix no longer works with date-time validation in 2.3

[oschwald]

Before 2.3, this code using \DateTime::RFC3339 accepted RFC 3339 timestamps with the Zulu (Z) suffix, e.g., 2014-04-12T23:20:50.052Z. As of 2.3, an exception with the message time must be a valid date/time in the format "2005-12-30T01:02:03+00:00" is thrown.

I previously mentioned this in a comment. I probably should have created a separate issue, but I didn't think much of it after initially testing out the pre-release version.

[henriquemoody]

Are you sure that it's working on version 2.2? I'm getting an exception when I runt the code below:

v::dateTime(\DateTime::RFC3339)->assert('2014-04-12T23:20:50.052Z')

[henriquemoody]

v::dateTime(\DateTime::RFC3339)->assert('2005-12-30T01:02:03+00:00')

This works fine on 2.3

[henriquemoody]

If you use PHP's DateTime constant as a format, you must check its value. See DateTimeInterface

RFC3339 = "Y-m-d\\TH:i:sP"
RFC3339_EXTENDED = "Y-m-d\\TH:i:s.vP"

It is possible to validate that the way you want, but you must use a different format: Y-m-d\TH:i:s.vp. It's very similar to RFC3339_EXTENDED, but it used p instead of P. You can check DateTime.format for more details.

I wouldn't like to overwrite PHP's default behaviour because that could cause more confusion. If I misunderstood your request, feel free to reopen the issue. I'm here to help!

Cheers 🐼

[oschwald]

Yes, this was working on 2.2. The tests for the above repo pass with 2.2 and fail with 2.3. We have weekly automated builds that were passing until last week. I also just tested this downgrading to 2.2.

DateTime can parse this fine:

echo  DateTime::createFromFormat(DateTime::RFC3339_EXTENDED, '2014-04-12T23:20:50.052Z')->format("H:i:s.v");

outputs 23:20:50.052.

[oschwald]

I bisected the issue to 5fe4b96.

[oschwald]

The issue is $value !== $formattedDate->format($format) as 2014-04-12T23:20:50.052Z will be turned into 2014-04-12T23:20:50.052+00:00.

[henriquemoody]

I wouldn't call this an "issue", although I understand that that can be problematic for your use case.

The commit you mentioned aimed to fix an issue with the date-related rules. See #1274.

Our date-related rules aim to respect the format that we pass to them. The value of DateTimeInterface::RFC3339 is Y-m-d\\TH:i:sP, while the input in your example matches the Y-m-d\TH:i:s.vp format.

[oschwald]

This is a breaking change. It accepted valid RFC 3339 timestamps before the change with the PHP RFC3339 format and now it rejects them.

…

On Fri, Feb 2, 2024, 3:27 AM Henrique Moody ***@***.***> wrote: I wouldn't call this an "issue", although I understand that that can be problematic for your use case. The commit you mentioned aimed to fix an issue with the date-related rules. See #1274 <#1274>. The date-related aim is to respect the format that we pass to them. The value of DateTimeInterface::RFC3339 is Y-m-d\\TH:i:sP, while the input in your example matches the Y-m-d\TH:i:s.vp format. — Reply to this email directly, view it on GitHub <#1442 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACECMYKAIIEPK3NBV7RTB3YRTESTAVCNFSM6AAAAABCPVUGWOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRTGYYTINZQGE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[henriquemoody]

I understand, and I'm sorry you are frustrated because your code doesn't work as expected. After all, that happened because we changed things, not you.

I insist that when you pass a format to the DateTime rule, it expects the input to match the format you pass to it. From the perspective of the rule, the value you're passing is Y-m-d\\TH:i:sP and not RFC3339.

In fact, PHP itself cannot parse all dates using that constant, even the ones mentioned in the RFC itself. See https://onlinephp.io/c/4609f

Reverting that commit will defeat the purpose of our date-related rules, and I will not do that. That change makes the rules more predictable and accurate.

[henriquemoody]

I suggest you add another rule to your chain, so it will accept yet another format v::dateTime('Y-m-d\TH:i:s.vp'). That will fix the issue you're having.

[oschwald]

If you look at the commit that mentions this issue, I did that. However, users of my library that happen to have a newer version of Respect/Validation with an older version of the library are still experience the breakage. The RFC3339 format intentionally allows Z while parsing as it is a valid representation

How do you suggest I deal with the other cases where this breaking change caused valid timestamps to fail, e.g., fractional seconds? For instance, 2014-04-12T23:20:50.05+00:00. It is a mistake to assume that a valid timestamp will round-trip to exactly the same string.

[henriquemoody]

You need to include formats you intend your rule to pass. If Y-m-d\\TH:i:sP (DateTime::RFC3339) and Y-m-d\\TH:i:s.vP (DateTime::RFC3339_EXTENDED) are not sufficient, you might want to add the formats you want to allow.

diff --git a/src/MinFraud/Validation/Rules/Event.php b/src/MinFraud/Validation/Rules/Event.php
index ea2a33c..3639b50 100644
--- a/src/MinFraud/Validation/Rules/Event.php
+++ b/src/MinFraud/Validation/Rules/Event.php
@@ -20,7 +20,9 @@ class Event extends AbstractWrapper
                 'time',
                 v::anyOf(
                     v::dateTime(\DateTime::RFC3339),
-                    v::dateTime(\DateTime::RFC3339_EXTENDED)
+                    v::dateTime(\DateTime::RFC3339_EXTENDED),
+                    v::dateTime('Y-m-d\TH:i:s.vp'),
+                    v::dateTime('Y-m-d\TH:i:sp'),
                 ),
                 false
             ),

[henriquemoody]

Perhaps I didn't understand you. I saw you commit maxmind/minfraud-api-php@a23246f, and you did exactly what I did.

However, users of my library that happen to have a newer version of Respect/Validation with an older version of the library are still experience the breakage.

I understand that this may be upsetting, but the change introduced was, in fact, a bug fix. The problem was that your code used the bug in its favour, as it was more flexible. If I revert the change, there could be other people experiencing a "real" bug.

How do you suggest I deal with the other cases where this breaking change caused valid timestamps to fail, e.g., fractional seconds? For instance, 2014-04-12T23:20:50.05+00:00. It is a mistake to assume that a valid timestamp will round-trip to exactly the same string.

That's what I don't think I understood. 2014-04-12T23:20:50.05+00:00 matches Y-m-d\TH:i:s.vp perfectly.

The RFC3339 format intentionally allows Z while parsing as it is a valid representation

It does, and I understand that. However, neither DateTime::RFC3339 nor DateTime::RFC3339_EXTENDED match that format.

[oschwald]

I understand that this may be upsetting, but the change introduced was, in fact, a bug fix. The problem was that your code used the bug in its favour, as it was more flexible. If I revert the change, there could be other people experiencing a "real" bug.

I think this is confusing parsing with formatting.

That's what I don't think I understood. 2014-04-12T23:20:50.05+00:00 matches Y-m-d\TH:i:s.vp perfectly.

Yes, like Z with P, it matches the format for parsing and it does not match the string produced with that format. Given:

<?php

require 'vendor/autoload.php';

use Respect\Validation\Validator as v;

v::dateTime(\DateTime::RFC3339_EXTENDED)->assert('2014-04-12T23:20:50.05+00:00');

I receive the following error on 2.3:

PHP Fatal error:  Uncaught All of the required rules must pass for "2014-04-12T23:20:50.05+00:00"
  thrown in /home/greg/MaxMind/minfraud-api-php/vendor/respect/validation/library/Factory.php on line 233

As mentioned above, this is due to the use of the fractional second. PHP formats it as three digits, i.e., 2014-04-12T23:20:50.050+00:00, which does not match the original string.

[henriquemoody]

I see a distinction between parsing and formatting, but remember that the date-related rules validate whether the input matches a given format and NOT if the input can be parsed with DateTime::createFromFormat() with a given format.

The DateTime::format()'s documentation states that

P => Difference to Greenwich time (GMT) with colon between hours and minutes
p => The same as P, but returns Z instead of +00:00 (available as of PHP 8.0.0)

If we break down the last date/time you provided, 2014-04-12T23:20:50.05+00:00, we have the following:

Value	Format	Description
2014	Y	A full numeric representation of a year
04	m	Numeric representation of a month, with leading zeros
12	d	Day of the month, two digits with leading zeros
23	H	24-hour format of an hour with leading zeros
20	i	Minutes with leading zeros
50	s	Seconds with leading zeros
05	v	Milliseconds
+00:00	P	Difference to Greenwich time (GMT) with a colon between hours and minutes NOT using Z instead of +00:00

The DateTimeImmutable::createFromFormat()'s documentation states:

The formatting constants as used in this example consist of a string of characters for formatting a DateTimeImmutable object. In most cases, these letters match with the same elements of date/time information as the ones defined in the parameters section above, but they tend to be more lenient.

I think the value of DateTimeInterface::RFC3339_EXTENDED should have been Y-m-d\TH:i:s.vp to account for Zulu (Z) suffix.

It is good that DateTimeImmutable::createFromFormat() tends to be more lenient, but if someone uses v::dateTime(), they expect an input to match a specific format. They might not use DateTime::createFromFormat() to parse that input but expect that Validation has validated it accordingly. That would mean they'll have a false positive.

That said, I insist that this is not a bug but the expected behaviour of Validation. I will not revert the changes we made on version 2.3

Alternatively, you might use v::callback(fn($input) => is_string($input) && DateTime::createFromFormat(DateTime::RFC3339_EXTENDED, $input) !== false)

[henriquemoody]

Hi @oschwald!

It took me a while to come around, but I can see now how this is a breaking change. I might need your opinion about this, because it's been a month since I released version 2.3. I could revert this change, but that means it will also be a breaking change for people using version 2.3. Do you have any suggestions?