golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.72k stars 17.5k forks source link

time: RFC3339 time.Parse can not parse string that come from time.Format #20555

Closed bronze1man closed 1 month ago

bronze1man commented 7 years ago

Please answer these questions before submitting your issue. Thanks!

What did you do?

ttps://play.golang.org/p/BItq172M-_ The bad time string "0007-05-31T3:50:00+99:80"

What did you expect to see?

0001-01-01 00:00:00 +0000 UTC xxx some error

What did you see instead?

0007-05-31 03:50:00 +10020 +10020 <nil>
0007-05-31T03:50:00+100:20
0001-01-01 00:00:00 +0000 UTC parsing time "0007-05-31T03:50:00+100:20" as "2006-01-02T15:04:05Z07:00": cannot parse "+100:20" as "Z07:00"

Because the RFC3339 parse/format is using in json. This may curse some crashes with special user input and developer assumes that json.Unmarshal is not return an error equals to json.Unmarshal/json.Marshal/json.Unmarshal will not return an error

This string is found by using https://github.com/dvyukov/go-fuzz

ianlancetaylor commented 7 years ago

This is not a bug in the time package. It's true that you can format a time using time.RFC3339 that you can not parse with time.RFC3339. That is perhaps unfortunate but it follows from the documentation.

Can you express this as an issue about encoding/json?

bronze1man commented 7 years ago

"Can you express this as an issue about encoding/json?" https://play.golang.org/p/r6XY9LyWqu

I assumes that I can json.Unmarshal A to B successfully,so I can json.Marshal B to C,and I can json.Unmarshal C to D successfully. But the package encoding/json and time do not behavior like what i assumes with this special user input:

[]byte(`"0007-05-31T3:50:00+99:80"`)
kennygrant commented 6 years ago

The RFC defines time-numoffset in terms of time-hour and time-minute which are limited to 0-24 and 0-59. From a quick look I can't see any checking on zone offsets (other fields are checked) in the source for the parse() function, could this be added? Otherwise should it just be noted in docs that zone offsets are not range checked?

This does seem rather theoretical as the offset is invalid (I think, unless I'm reading the RFC wrong), but it might be worth documenting or returning an out of range error.

https://tools.ietf.org/html/rfc3339#section-5.6

Time:

   time-hour         = 2DIGIT ; 00-24
   time-minute       = 2DIGIT ; 00-59
   time-second       = 2DIGIT ; 00-58, 00-59, 00-60 based on
                              ; leap-second rules
   time-fraction     = ("," / ".") 1*DIGIT
   time-numoffset    = ("+" / "-") time-hour [[":"] time-minute]
   time-zone         = "Z" / time-numoffset
ippoippo commented 6 years ago

I've just been hit by this issue.

I would expect "2018-01-16T05:15:37Z" to be valid input and handled by time.RFC3339, but "2018-01-16T5:15:37Z" is not valid due to missing leading zero on the Hour.

It seems rather inconsistent....

andybons commented 6 years ago

It seems that Parse and Format should be symmetric in their behavior. Marking for consideration in Go2 at least.

ianlancetaylor commented 6 years ago

Making them fully symmetric seems reasonable, but it also seems quite hard to do it in a backward compatible manner. And even if we want to automatically fix existing uses, it seems quite hard to do so with complete reliability.

geocode commented 5 years ago

I would like to add my experience to this. Would really love to see a fix for this. Many systems (including fmt.Println(someTime) output the time as YYYY-MM-DDTHH:MM:SS.sss+HHMM (note no colon on offset).

The RFC 3339 spec notes that the colon is optional for the offset (see https://tools.ietf.org/html/rfc3339#appendix-A). Because the json.Unmarshal() automatically uses the RFC3339 as the format, this prevents Go from parsing many versions of the standard timestamp format in its acceptance of json.

All of these should be valid: "2006-01-02T15:04:05+0000" "2006-01-02T15:04:05-0000"

I'm wonder if supporting the colon in the offset being optional really requires waiting for Go 2?

flyinprogrammer commented 5 years ago

Just ran into issues parsing dates out of the Sonatype Nexus 3 API. This has been open for 697 days now, can we get this issue on a road map?

ianlancetaylor commented 5 years ago

I don't think we even know what should be done.

You can not use time.RFC3339 with time.Parse to parse all valid RFC 3339 time data strings. This is explicitly documented. I can't see any way to change it.

zedawg commented 5 years ago

Just ran into this as well trying to unmarshal a Bitbucket Webhook payload.

ippoippo commented 5 years ago

I'm going to put out perhaps a somewhat unpopular opinion, but perhaps the designers of Golang and the core libs to re-assess whether the "by example" format string that Go uses is actually fit for purpose.

I know we can have arguments about what is more 'understandable', and I don't contest that perhaps some people find this easier, and others find the approaches used in other languages (like C, Swift, Ruby etc).

BUT, it seems that the Go approach just is not able to parse all scenarios using the Time functionality. It forces you to perform direct string parsing instead. I know this isn't going to happen for Go2, but maybe future releases could consider deprecating and replacing the "by example" way of doing things???

ianlancetaylor commented 5 years ago

As I said just a few posts up, I don't think we even know what should be done.

I agree that while the format string works reasonably well for time.Time.Format, it can be harder to use with time.Parse. But that acknowledgement does not show us a way forward. Are there other libraries, perhaps in other languages, that do a better job?

brian-brazil commented 5 years ago

https://play.golang.org/p/hM_DWyv0JMK shows another issue, it appears that time.Parse can only handle 4 digit years for RFC3339.

johnaoss commented 4 years ago

@brian-brazil I think that is behaviour as expected. RFC3339 assumes that:

All dates and times are assumed to be in the "current era", somewhere between 0000AD and 9999AD.

FaisalHBTU commented 4 years ago

https://play.golang.org/p/Dflk-FGvm40. But the timezone must be within 24 hours ideally

yanhao-pro commented 4 years ago

https://play.golang.org/p/2xDQsXxYcxe

Even the layout itself is not a valid value,

time.Parse(time.RFC3339, time.RFC3339)
// parsing time "2006-01-02T15:04:05Z07:00": extra text: 07:00
chaudharisuresh997 commented 4 years ago

should it be that difficult ????it is so hard golang time package . how u think i should use wasted my 5 hours still no outcome seems like layout does exact string comparision do u really think production data will be this much accurate if layout is 2012-01-02 and time as input is 2020-07-27 18:14:58.1527977 +0530 IST then time.Parse shamelessly fails

Program==> t:="2020-07-27 18:14:58.1527977 +0530 IST"
t1,_:=time.Parse("2006-01-02",t)

output ==> 0001-01-01 00:00:00 +0000 UTC playground link https://play.golang.org/p/G7qXFIJJabg what i expect my time should be converted to yyyy-mm-dd instead it gives this

except time package golang is excellant. Why you do exact string matching ????

mfreeman451 commented 3 years ago

Experiencing this same issue trying to unmarshal json from firebase

thaney071 commented 3 years ago

The bigger problem isn't just that the Format and Parse are not symmetric, instead, that none of the provided time methods can get back to the original time struct.

Even if the data that is lost between conversions is not relevant to being used in a time struct, it is still not acceptable that data is lost with the event the Marshal, Unmarshal methods.

https://play.golang.org/p/VdqFYrhNOGX

So even if we are not to expect Parse and Format to be used, there is still no other methods available to encode time for storage in some other way, and get back the original time struct.

seankhliao commented 3 years ago

@thaney071 that is something else and documented:

Because the monotonic clock reading has no meaning outside the current process, the serialized forms generated by t.GobEncode, t.MarshalBinary, t.MarshalJSON, and t.MarshalText omit the monotonic clock reading, and t.Format provides no format for it

thaney071 commented 3 years ago

@seankhliao That is unfortunate because it makes testing time structure difficult since any deep equals check of the time struct before and after the marshaling will not be equal.

Indy9000 commented 2 years ago

https://play.golang.org/p/2xDQsXxYcxe

Even the layout itself is not a valid value,

time.Parse(time.RFC3339, time.RFC3339)
// parsing time "2006-01-02T15:04:05Z07:00": extra text: 07:00

This is expected and documented: " // Some valid layouts are invalid time values, due to format specifiers // such as _ for space padding and Z for zone information. // For example the RFC3339 layout 2006-01-02T15:04:05Z07:00 // contains both Z and a time zone offset in order to handle both valid options: // 2006-01-02T15:04:05Z // 2006-01-02T15:04:05+07:00 " Ref https://pkg.go.dev/time#Parse

ianlancetaylor commented 1 month ago

I'm closing this issue because there is nothing to be done at this point.

This general idea can be revisited in the context of a time/v2 package. But that package would have many other differences with regard to formatting, and I don't think we are going to forget this issue.