Skip to content

Commit

Permalink
[release-branch.go1.20] time: revert strict parsing of RFC 3339
Browse files Browse the repository at this point in the history
CL 444277 fixed Time.UnmarshalText and Time.UnmarshalJSON to properly
unmarshal timestamps according to RFC 3339 instead of according
to Go's bespoke time syntax that is a superset of RFC 3339.

However, this change seems to have broken an AWS S3 unit test
that relies on parsing timestamps with single digit hours.
It is unclear whether S3 emits these timestamps in production or
whether this is simply a testing artifact that has been cargo culted
across many code bases. Either way, disable strict parsing for now
and re-enable later with better GODEBUG support.

Updates #54580

Change-Id: Icced2c7f9a6b2fc06bbd9c7e90f90edce24c2306
Reviewed-on: https://go-review.googlesource.com/c/go/+/462286
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Joseph Tsai <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Russ Cox <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/462675
Reviewed-by: Joseph Tsai <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
  • Loading branch information
dsnet authored and gopherbot committed Jan 18, 2023
1 parent 8b34676 commit 5adb0ca
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 10 deletions.
5 changes: 2 additions & 3 deletions doc/go1.20.html
Original file line number Diff line number Diff line change
Expand Up @@ -1200,9 +1200,8 @@ <h3 id="minor_library_changes">Minor changes to the library</h3>
</p>

<p><!-- CL 444277 -->
The <a href="/pkg/time/#Time.MarshalJSON"><code>Time.MarshalJSON</code></a> and
<a href="/pkg/time/#Time.UnmarshalJSON"><code>Time.UnmarshalJSON</code></a> methods
are now more strict about adherence to RFC 3339.
The <a href="/pkg/time/#Time.MarshalJSON"><code>Time.MarshalJSON</code></a> method
is now more strict about adherence to RFC 3339.
</p>
</dd>
</dl><!-- time -->
Expand Down
7 changes: 6 additions & 1 deletion src/time/format_rfc3339.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ func parseRFC3339[bytes []byte | string](s bytes, local *Location) (Time, bool)
func parseStrictRFC3339(b []byte) (Time, error) {
t, ok := parseRFC3339(b, Local)
if !ok {
if _, err := Parse(RFC3339, string(b)); err != nil {
t, err := Parse(RFC3339, string(b))
if err != nil {
return Time{}, err
}

Expand All @@ -164,6 +165,10 @@ func parseStrictRFC3339(b []byte) (Time, error) {
// See https://go.dev/issue/54580.
num2 := func(b []byte) byte { return 10*(b[0]-'0') + (b[1] - '0') }
switch {
// TODO(https://go.dev/issue/54580): Strict parsing is disabled for now.
// Enable this again with a GODEBUG opt-out.
case true:
return t, nil
case b[len("2006-01-02T")+1] == ':': // hour must be two digits
return Time{}, &ParseError{RFC3339, string(b), "15", string(b[len("2006-01-02T"):][:1]), ""}
case b[len("2006-01-02T15:04:05")] == ',': // sub-second separator must be a period
Expand Down
12 changes: 6 additions & 6 deletions src/time/time_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -830,10 +830,10 @@ func TestUnmarshalInvalidTimes(t *testing.T) {
}{
{`{}`, "Time.UnmarshalJSON: input is not a JSON string"},
{`[]`, "Time.UnmarshalJSON: input is not a JSON string"},
{`"2000-01-01T1:12:34Z"`, `parsing time "2000-01-01T1:12:34Z" as "2006-01-02T15:04:05Z07:00": cannot parse "1" as "15"`},
{`"2000-01-01T00:00:00,000Z"`, `parsing time "2000-01-01T00:00:00,000Z" as "2006-01-02T15:04:05Z07:00": cannot parse "," as "."`},
{`"2000-01-01T00:00:00+24:00"`, `parsing time "2000-01-01T00:00:00+24:00": timezone hour out of range`},
{`"2000-01-01T00:00:00+00:60"`, `parsing time "2000-01-01T00:00:00+00:60": timezone minute out of range`},
{`"2000-01-01T1:12:34Z"`, `<nil>`},
{`"2000-01-01T00:00:00,000Z"`, `<nil>`},
{`"2000-01-01T00:00:00+24:00"`, `<nil>`},
{`"2000-01-01T00:00:00+00:60"`, `<nil>`},
{`"2000-01-01T00:00:00+123:45"`, `parsing time "2000-01-01T00:00:00+123:45" as "2006-01-02T15:04:05Z07:00": cannot parse "+123:45" as "Z07:00"`},
}

Expand All @@ -842,13 +842,13 @@ func TestUnmarshalInvalidTimes(t *testing.T) {

want := tt.want
err := json.Unmarshal([]byte(tt.in), &ts)
if err == nil || err.Error() != want {
if fmt.Sprint(err) != want {
t.Errorf("Time.UnmarshalJSON(%s) = %v, want %v", tt.in, err, want)
}

if strings.HasPrefix(tt.in, `"`) && strings.HasSuffix(tt.in, `"`) {
err = ts.UnmarshalText([]byte(strings.Trim(tt.in, `"`)))
if err == nil || err.Error() != want {
if fmt.Sprint(err) != want {
t.Errorf("Time.UnmarshalText(%s) = %v, want %v", tt.in, err, want)
}
}
Expand Down

0 comments on commit 5adb0ca

Please sign in to comment.