search

brick / date-time

Date and time library for PHP
MIT License
321 stars 29 forks source link

Instant::toISOString() is not RFC 3339 compliant #110

Open JanTvrdik opened 6 days ago

JanTvrdik commented 6 days ago

Instant::toISOString() / ZonedDateTime::toISOString returns value that is not RFC 3339 compliant, because it will omit seconds, when its value is zero.

This is causing portability issues, for example, such value is not accepted by java.time.Instant.parse.

Java sandbox

RFC 3339 format description

BenMorel commented 4 days ago

Good point, thanks for reporting it @JanTvrdik!

I'm surprised to see that the behaviour is different for LocalDateTime and LocalTime: sandbox.

This is probably the source of confusion when I first implemented ZonedDateTime::__toString(), where I just concatenated the output of the LocalDateTime with the timezone.

I'll work on a fix, however this will target version 0.8.0 as this is technically a BC break.

What about parsing, should we force seconds as well? I'm leaning towards yes.

JanTvrdik commented 4 days ago

What about parsing, should we force seconds as well?

Java allows missing seconds when parsing ZonedDateTime, LocalDateTime and LocalTime. The missing seconds are allowed by ISO 8601. You also have the problem, that you would be unable to parse string that were generated by brick/date-time < 0.8. So overall I think it's better to keep the parser unchanged.