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: handle >4-digit or negative years in MarshalJSON, UnmarshalJSON #4556

Closed
agoode opened this issue Dec 16, 2012 · 15 comments

Comments

Projects
None yet
4 participants
@agoode
Copy link

commented Dec 16, 2012

RFC3339 format for time.Time JSON translation is insufficient. The Google APIs will
happily output >4 digit years for such dates in the far future. For dates before year
0, negative values are produced. Dates in the far past also have more than 4 digits.

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
http://play.golang.org/p/QkymcrE2sX

1. Try to marshal or unmarshal a date outside of the year range [0,9999]


What is the expected output?
Valid interchange.


What do you see instead?
Errors.

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
Linux

Which version are you using?  (run 'go version')
go1.0.3
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2012

Comment 1:

Really? When do the Google APIs generate such bogus years?
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 17, 2012

Comment 2:

Quoting RFC 3339:
   There are many ways in which date and time values might appear in
   Internet protocols:  this document focuses on just one common usage,
   viz. timestamps for Internet protocol events.  This limited
   consideration has the following consequences:
   o  All dates and times are assumed to be in the "current era",
      somewhere between 0000AD and 9999AD.
@agoode

This comment has been minimized.

Copy link
Author

commented Dec 17, 2012

Comment 3:

Correct, RFC3339 is limited to 4 digit non-negative years. But dates as encoded in JSON
often are not.
The easiest way to get a Google API to give you a non-RFC3339 date is to insert a file
using the Drive API with a modifiedDate outside of the RFC3339 range. Perhaps the API
should have been written to disallow such dates, but it does not, and will happily
return the values that you entered.
It would not be such a big deal to fail non-RFC3339 dates with a parse error, except
that through various means, you'll get giant JSON objects containing a listing of
hundreds of files, and one invalid date will invalidate the entire listing. Worse, files
with such dates can be caused to appear in your JSON responses by arbitrary third
parties. (I can share a file with a crazy date with you, and cause you to get a parse
error if you cannot understand it.) This is a kind of denial of service.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 17, 2012

Comment 4:

Maybe we should get Google Drive fixed. The docs clearly say it uses
an RFC 3339 date, but you're saying it doesn't.
@agoode

This comment has been minimized.

Copy link
Author

commented Dec 17, 2012

Comment 5:

Yes, I sent feedback to the Drive team regarding this documentation problem.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 17, 2012

Comment 6:

I am still unconvinced that Google Drive breaking RFC 3339 means that
Time's UnmarshalJSON needs to. If this becomes a real problem the
Google API wrappers can use their own time type that accepts the
out-of-range years.
@agoode

This comment has been minimized.

Copy link
Author

commented Dec 17, 2012

Comment 7:

Fair enough. Is the only way to create a new time type? It would be nice to use native
time.Time in a JSON-encodable struct, with a custom MarshalJSON/UnmarshalJSON, but it
looks like that is not possible in Go?
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 17, 2012

Comment 8:

Probably the easiest thing to do would be to define
type Time struct {
    time.Time
}
func (t *Time) MarshalJSON ...
func (t *Time) UnmarshalJSON ...
and then users of the Google API Go package can get at the actual time
by using .WhateverField.Time instead of .WhateverField.
However, let's see if we can't get Google Drive to fix this first.
It's going to break more than just Go.
Russ
@agoode

This comment has been minimized.

Copy link
Author

commented Dec 17, 2012

Comment 9:

It's too bad that Time.Format doesn't return err. It would have been nice to limit
Format to the values that Parse can read. (Part of my confusion was that Format will
emit >4 digit and negative years, but Parse cannot read them.)
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2012

Comment 10:

I agree. Unfortunately, we can't change the Format API now.
MarshalJSON does refuse to format such years, for what it's worth.
Russ
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2012

Comment 11:

Leaving this open but I don't think we're going to do this without a more compelling
reason. One broken API, even one created by Google, does not seem like enough.

Labels changed: added priority-later, go1.1maybe, removed priority-triage, go1.1.

Status changed to Thinking.

@robpike

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2013

Comment 12:

Labels changed: removed go1.1maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2013

Comment 13:

The Google APIs are broken. They should be fixed.

Status changed to WontFix.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2014

Comment 14:

Issue #7566 has been merged into this issue.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2014

Comment 15:

Since this has come up again, more information:
http://tools.ietf.org/html/rfc3339#section-1
   o  All dates and times are assumed to be in the "current era",
      somewhere between 0000AD and 9999AD.
http://tools.ietf.org/html/rfc3339#section-5.6
   date-fullyear   = 4DIGIT
   full-date       = date-fullyear "-" date-month "-" date-mday
   date-time       = full-date "T" full-time
That's very clear: years are 4 digits, not 5, and no minus sign.
Returning an error from MarshalJSON and now MarshalText makes
sure that the guarantee of RFC 3339-compliant output is met.
time.Format does not return an error, and it cannot, because the
input to time.Format is not some kind of enumerated constant but
instead is a format specifier, a tiny program in a tiny language for
printing times. The time.RFC3339 and time.RFC3339Nano constants
are tiny programs in this tiny language that generate the closest
approximation to RFC 3339 output that they can, but since the tiny
language of time.Format has no concept of failing, the best they can
do is generate these non-compliant results. This should not be
construed to imply anything about what RFC 3339 requires.
Again, RFC 3339 is very clear.

@agoode agoode added wontfix labels Mar 24, 2014

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.