time: JSON marshaling/unmarshaling does not work reliably for zero Time value

[ShawnHsiung]

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

$ go version go1.18.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/xxx/Library/Caches/go-build"
GOENV="/Users/xxx/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE="git.xxx.cn"
GOMODCACHE="/Users/xxx/Workspace/golib-common/pkg/mod"
GONOPROXY="git.xxx.cn"
GONOSUMDB="git.xxx.cn"
GOOS="darwin"
GOPATH="/Users/xxx/Workspace/golib-common"
GOPRIVATE="git.xxx.cn"
GOPROXY="https://goproxy.cn,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18.5"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/xxx/Workspace/xxx/src/git.xxx.cn/xxx/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/mh/05pw0l8d64s30wl_hl55bbc40000gn/T/go-build4224931359=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://go.dev/play/p/Xsu1BBKI26i

package main

import (
	"fmt"
	"time"
)

func main() {

	loc, _ := time.LoadLocation("Asia/Shanghai")
	time.Local = loc
	tlocal := time.Time{}.Local()

        fmt.Println("----- Before MarshalJSON -----")
	fmt.Println(tlocal)
	fmt.Println(tlocal.Zone())
	fmt.Println(tlocal.IsZero())

	mal, _ := tlocal.MarshalJSON()
	recv := time.Time{}
	recv.UnmarshalJSON(mal)

	fmt.Println("----- After UnmarshalJSON -----")
	fmt.Println(recv)
	fmt.Println(recv.Zone())
	fmt.Println(recv.IsZero())
}

What did you expect to see?

----- Before MarshalJSON -----
0001-01-01 08:05:43 +0805 LMT
LMT 29143
true
----- After UnmarshalJSON -----
0001-01-01 08:05:43 +0805 +0805
29143 // should be 29143
true // IsZero() should return true

What did you see instead?

----- Before MarshalJSON -----
0001-01-01 08:05:43 +0805 LMT
LMT 29143
true
----- After UnmarshalJSON -----
0001-01-01 08:05:43 +0805 +0805
29100 // lost 43 seconds
false // then IsZero() == false

[ianlancetaylor]

Thanks. This is failing because we use RFC 3339 format to marshal and unmarshal times, and that format can't represent the unusual sub-minute timezones used for Asia/Shanghai before 1901. I don't think we can fix this in general, but it may be worth handling the zero time.Time value specially.

CC @dsnet

[dsnet]

I see at least four defensible behaviors:

1. Keep the status quo.
2. Report an error when using any timezone with sub-minute offsets
3. Always serialize a time.Time where IsZero reports true as "0001-01-01T00:00:00Z" regardless of timezone
4. Support an json:",omitzero" tag option that elides a field if the IsZero reports true (see encoding/json: add omitzero option #45669).

Of the options, I like 2 and 4 the most.

[bcmills]

The doc for time.Time currently says:

The zero value of type Time is January 1, year 1, 00:00:00.000000000 UTC. As this time is unlikely to come up in practice, the IsZero method gives a simple way of detecting a time that has not been initialized explicitly.

And the implementation of (time.Time).Location seems to agree with that: it returns UTC if t.loc is nil.

So this issue is not about the zero time.Time, but rather the localized time representing the same instant as the zero time.Time. That seems like a much less severe problem.

[bcmills]

I would argue that there are two clear bugs here:

1. time.Time.IsZero() is reporting true for non-UTC values, which are not entirely equivalent to the zero time.Time.
2. MarshalJSON is marshaling an approximate time for a non-representable value instead of returning an error.

[dsnet]

time.Time.IsZero() is reporting true for non-UTC values, which are not entirely equivalent to the zero time.Time.

I'm not sure I see that as a bug. My interpretation of IsZero is that it reports whether the time is the same "time instant" as 0001-01-01T00:00:00Z, which is agnostic to the specified time zone. Requiring that it also check that the timezone be UTC is reasonable behavior, but that's not how it operated for the past 10 years.

MarshalJSON is marshaling an approximate time for a non-representable value instead of returning an error.

I agree with this.

[seankhliao]

MarshalJSON is marshaling an approximate time for a non-representable value instead of returning an error.

Is this really an issue?

RFC 3339 Section 4.2 says this about time zones with second offsets

NOTE: Following ISO 8601, numeric offsets represent only time
zones that differ from UTC by an integral number of minutes.
However, many historical time zones differ from UTC by a non-
integral number of minutes. To represent such historical time
stamps exactly, applications must convert them to a representable
time zone.

RFC 3339 Section 5.8 goes on to give an example of how it should be represented.

  1937-01-01T12:00:27.87+00:20

This represents the same instant of time as noon, January 1, 1937,
Netherlands time. Standard time in the Netherlands was exactly 19
minutes and 32.13 seconds ahead of UTC by law from 1909-05-01 through
1937-06-30. This time zone cannot be represented exactly using the
HH:MM format, and this timestamp uses the closest representable UTC
offset.

[bcmills]

Is this really an issue?

Maybe? I guess that depends on whether you expect time.Time to round-trip through JSON with or without preserving the exact timezone. (It explicitly does not preserve monotonic clock readings, but doesn't say anything about losing time zone information.)

[yonderblue]

round-trip through JSON

Most developers likely expect roundtrip by default to work. For example https://play.golang.com/p/YpuQWEmy62S. Getting an LMT location as in that example is normal depending what machine it is running as far as I know.

At the least some documentation in the time and/or json pkg would be very helpful.