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

time: document that RFC3339 accepts invalid formats when used with Parse #37616

Closed
xdg opened this issue Mar 3, 2020 · 6 comments
Closed

time: document that RFC3339 accepts invalid formats when used with Parse #37616

xdg opened this issue Mar 3, 2020 · 6 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@xdg
Copy link

xdg commented Mar 3, 2020

What version of Go are you using (go version)?

$ go version
go version go1.13 darwin/amd64

Does this issue reproduce with the latest release?

It reproduces on play.golang.org and the underlying issue is still present in the source.

What operating system and processor architecture are you using (go env)?

darwin/amd64

What did you do?

Parsed a faulty ISO 8601 string with a single-digit hour using time.RFC3339. E.g. 2020-02-02T2:02:02Z.

What did you expect to see?

I expected this parse to error because RFC-3339 sec 5.6 defines the following token, requiring two digits for the hour:

time-hour       = 2DIGIT  ; 00-23

What did you see instead?

The parse succeeded.

This could be fixed by changing time.RFC3339 to 2006-01-02T03:04:05Z07:00. If this is deemed a backward incompatible change, adding a new time.RFC3339Strict constant would also be an acceptable option.

See https://play.golang.org/p/6qyFYfx6pMr for example with one and two-digit years with time.Parse3339 and the stricter version.

@ianlancetaylor
Copy link
Contributor

As the documentation says, " RFC3339, RFC822, RFC822Z, RFC1123, and RFC1123Z are useful for formatting; when used with time.Parse they do not accept all the time formats permitted by the RFCs."

We can't change time.RFC3339 because that would break backward compatibility.

We aren't going to add a new constant just to use with time.Parse, because we already document that these constants don't do the right thing when used with time.Parse.

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 3, 2020
@xdg
Copy link
Author

xdg commented Mar 3, 2020

when used with time.Parse they do not accept all the time formats permitted by the RFCs

I hear your point, but the existing documentation says only that they don't accept all formats permitted. The docs don't say that they do accept formats prohibited by the RFCs, which is what I demonstrate.

If you don't want to change the existing constant (which I fully accept), or provide correct constants (which I question), what do you think about one or both of these:

  • adding documentation to caveat that they accept prohibited formats
  • providing an example of the stricter RFC3339 format (so that users can copy/paste it for use)

I'm happy to send a PR if we can agree in advance on what it should generally contain.

@antong
Copy link
Contributor

antong commented Apr 16, 2020

The title and description mention two digit "year". Is this intentional, or should it say "hour"?

@xdg xdg changed the title time.RFC3339 should require 2-digit year time.RFC3339 should require 2-digit hour Apr 16, 2020
@xdg
Copy link
Author

xdg commented Apr 16, 2020

Yes, thanks. Fixed the title.

@ianlancetaylor
Copy link
Contributor

I don't think it is possible to write a time format that works with time.Parse and that accepts exactly and only the time formats permitted by RFC 3339. The time.RFC3339 value produces a valid RFC 3339 time format when used with time.Time.Format. It does not and cannot work with time.Parse.

I'm fine with adding a clause saying that the various formats accept invalid strings when used with time.Parse. Maybe we can shorten the existing clause while we're at it.

@ianlancetaylor ianlancetaylor added Documentation NeedsFix The path to resolution is known, but the work has not been done. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Apr 17, 2020
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Apr 17, 2020
@ianlancetaylor ianlancetaylor changed the title time.RFC3339 should require 2-digit hour time: document that RFC3339 accepts invalid formats when used with Parse Apr 17, 2020
@gopherbot
Copy link

Change https://golang.org/cl/229460 mentions this issue: time: note that formats may parse invalid strings

@golang golang locked and limited conversation to collaborators Jun 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants