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

Should 2025-01-03T13:55-05[!-04:00] parse successfully into an Instant? #3068

Open
BurntSushi opened this issue Jan 3, 2025 · 3 comments
Open

Comments

@BurntSushi
Copy link

I was going over the error conditions for parsing a Temporal ISO 8601 datetime in my Rust library (Jiff) inspired by Temporal, and I stumbled on to this case and wondered if it was intended or not. Specifically, this seems to work right now:

>> ts = Temporal.Instant.from("2025-01-03T13:55-05[!-04:00]")
Object { … }
>> ts.toString()
"2025-01-03T18:55:00Z" 

I am wondering if this is intended. And more broadly, what Temporal does with the critcal flag (!) in a time zone annotation.

Depending on how you hold RFC 9557, it seems to suggest that the above should result in an error:

A suffix tag may also indicate that it is critical: The recipient is advised that it MUST NOT act on the IXDTF string unless it can process the suffix tag as specified. A critical suffix tag is indicated by following its opening bracket with an exclamation mark (see critical-flag in Section 4.1).

RFC 9557 also says this:

Note that applications MAY also perform additional processing on inconsistent or unrecognized elective suffix tags, such as asking the user how to resolve the inconsistency. While they are not required to do so with elective suffix tags, they are required to reject or perform some other error handling when encountering inconsistent or unrecognized suffix tags marked as critical.

Which seems to suggest further that the above should result in an error.

I suppose this is perhaps somewhat questionable since parsing an Instant can be done without looking at or caring about the time zone annotation at all. And if the above would return an error, then I suppose one might question whether this should return an error too:

>> ts = Temporal.Instant.from("2025-01-03T13:55-05[!NotATimeZone]")
Object { … }
>> ts.toString()
"2025-01-03T18:55:00Z" 

Perhaps one could make the argument that the first case above should be an error and this latter case shouldn't because, in the first case, there is a definitive inconsistency in the data.

In any case, Jiff has the same behavior as Temporal here currently, and I was wondering if it was intentional or not. In my case, I hadn't considered this at all, and the implementation of parsing an instant just completely ignores the time zone annotation (other than verifying that it parses correctly).

@Garbee
Copy link

Garbee commented Jan 31, 2025

Looks perfectly expected to me from RFC9557.

Essentially, while you say the time zone annotation is critical, if it conflicts with other data in the string it can be ignored. Your sample has a -05 offset already preceding the annotation. Thus, implementations can ignore that since they have another higher-precedent value to work from. So, regardless of the annotation being just another timezone or a completely arbitrary string, it isn't relevant. It can be ignored either way.

RFC9557 quote:

For example, IXDTF strings such as:

2022-07-08T00:14:07+01:00[Europe/Paris]

are internally inconsistent (see Section 3.4), because Europe/Paris did not use a time zone offset of +01:00 in July 2022. However, the time zone hint given in the suffix tag is elective, so the recipient is not required to act on the inconsistency; it can treat the Internet Date/Time Format string as if it were:

2022-07-08T00:14:07+01:00

@BurntSushi
Copy link
Author

Doesn't this part suggest it should be an error?

While they are not required to do so with elective suffix tags, they are required to reject or perform some other error handling when encountering inconsistent or unrecognized suffix tags marked as critical.

The offsets are in conflict with one another and the annotation is marked critical.

@Garbee
Copy link

Garbee commented Jan 31, 2025

Ah, good narrowing down on the spec wording. Then yea, I think it should error myself given the spec wording is pretty clear. We can't interrogate devs to figure out what they mean to adjust, error throwing makes sense.

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