Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
JanTvrdik opened this issue Jul 4, 2024 · 2 comments
Open

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

JanTvrdik opened this issue Jul 4, 2024 · 2 comments
Milestone

Comments

@JanTvrdik
Copy link

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 BenMorel added this to the 0.8.0 milestone Jul 6, 2024
@BenMorel
Copy link
Member

BenMorel commented Jul 6, 2024

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants