golang / go

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

time: Parse not setting Location to Local when TZ=UTC #45960

Open colin-sitehost opened 3 years ago

colin-sitehost commented 3 years ago

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

$ go version
go version go1.16.3 linux/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="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.3"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3471220289=/tmp/go-build -gno-record-gcc-switches"

What did you do?

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

What did you expect to see?

time.Time{wall:0xbab699fc00000000, ext:1, loc:(*time.Location)(0x5666c0)} "2009-11-10T23:00:00Z" time.Time{wall:0x0, ext:63393490800, loc:(*time.Location)(0x5666c0)}

What did you see instead?

time.Time{wall:0xbab699fc00000000, ext:1, loc:(*time.Location)(0x5666c0)} "2009-11-10T23:00:00Z" time.Time{wall:0x0, ext:63393490800, loc:(*time.Location)(nil)}

Summary

It looks like when time.Local reflects utc, unmarshaling is different than to when a timezone is present. play.golang.org may seem like a poor reproducer, since time and clocks are so heavily mocked, but my system does the same thing (either without /etc/timezone or with it pointed at /usr/share/zoneinfo/Etc/UTC). When I provide TZ=NZ or /etc/localtime, we get the desired output.

seankhliao commented 3 years ago

This works as intended, without zone info only the local time can be assumed

colin-sitehost commented 3 years ago

This works as intended, without zone info only the local time can be assumed

the issue is that code that works one one machine, breaks on another based on how the timezone is set. how are we able to assume zone info for NZ (or any non UTC zone) but not UTC?

this bug is more subtle than "only local time can be assumed". would it help to provide more samples?

seankhliao commented 3 years ago

I think I see the problem now

main » go run .
time.Date(2021, time.May, 5, 17, 29, 40, 618448319, time.Local) "2021-05-05T17:29:40.618448319+02:00" time.Date(2021, time.May, 5, 17, 29, 40, 618448319, time.Local)

main » TZ=NZ go run .
time.Date(2021, time.May, 6, 3, 29, 47, 269450883, time.Local) "2021-05-06T03:29:47.269450883+12:00" time.Date(2021, time.May, 6, 3, 29, 47, 269450883, time.Local)

main » TZ=UTC go run .
time.Date(2021, time.May, 5, 15, 29, 54, 909582162, time.Local) "2021-05-05T15:29:54.909582162Z" time.Date(2021, time.May, 5, 15, 29, 54, 909582162, time.UTC)

cc @rsc

ianlancetaylor commented 3 years ago

It's true that MarshalText stores a Z rather than a time offset when the timezone is UTC. And it's true that this means that UnmarshalText does not recover the same Location value. However, it does recover the same time offset.

Why does this matter?

colin-sitehost commented 3 years ago

the concern is that usually time.Time.MarshalJSON and time.Time.UnmarshalJSON are bidirectional, unless you are running with TZ=UTC. this caused an "it works on my machine" bug. if it was never or always, that would be fine, but it looks like some times we try and use time.Local when time.Time.UnmarshalJSONing:

package main

import (
        "fmt"
        "time"
)

func main() {
        fmt.Printf("%p\n", time.Local)
        t := time.Now().Round(0)
        b, err := t.MarshalJSON()
        fmt.Printf("%#v, %s\n", t, err)
        fmt.Println(string(b))
        t = time.Time{}
        err = t.UnmarshalJSON(b)
        fmt.Printf("%#v, %s\n", t, err)
}
[user@localhost ~]$ TZ=UTC go run .
0x5676a0
time.Time{wall:0x232e9915, ext:63757064330, loc:(*time.Location)(0x5676a0)}, %!s(<nil>)
"2021-05-19T23:38:50.590256405Z"
time.Time{wall:0x232e9915, ext:63757064330, loc:(*time.Location)(nil)}, %!s(<nil>)
[user@localhost ~]$ TZ=NZ go run .
0x5676a0
time.Time{wall:0x1d2f3de7, ext:63757064334, loc:(*time.Location)(0x5676a0)}, %!s(<nil>)
"2021-05-20T11:38:54.489635303+12:00"
time.Time{wall:0x1d2f3de7, ext:63757064334, loc:(*time.Location)(0x5676a0)}, %!s(<nil>)
ianlancetaylor commented 3 years ago

MarshalJSON and UnmarshalJSON are never going to be bidirectional with regard to the timezone, because the timezone is inherently dependent on what timezones are supported on the local machine. MarshalJSON maps from the exact timezone to a zone offset, and UnmarshalJSON returns a timezone with that offset. Nothing ensures that they are the same, and in principle it is impossible to ensure that they are the same. So although the unmarshaled result should always satisfy time.TIme.Equal with the original time, it won't always have the same timezone.

colin-sitehost commented 3 years ago

that makes sense, my request then is to never use time.Local during time.Time.UnmarshalJSON, since assuming that +12:00 means time.Local (for TZ=NZ) is as incorrect as assuming that Z is time.Local (for TZ=UTC).

EDIT: digging further into this, it looks like time.Parse is the root cause. note how different time.Locations are set based on the TZ env:

fmt.Printf("%p\n", time.Local)
t, _ := time.Parse(time.RFC3339Nano, "2021-05-20T16:03:45.330360905+12:00")
fmt.Printf("%#v\n", t)
[user@localhost ~]$ TZ=NZ go run .
0x5676a0
time.Time{wall:0x13b0e849, ext:63757080225, loc:(*time.Location)(0x5676a0)}
[user@localhost ~]$ TZ=UTC go run .
0x5676a0
time.Time{wall:0x13b0e849, ext:63757080225, loc:(*time.Location)(0xc000104000)}
daenney commented 3 years ago

I'm perhaps observing a related behaviour, which is also different between Go 1.16.4 on my local machine versus the same Go version on the playground.

tr, _ := time.Parse(time.RFC3339, "2021-05-21T12:21:25.954+00:00")
fmt.Printf("%#v\n", tr)
fmt.Printf("%#v\n", tr.Location())

Local machine:

$ go version
go version go1.16.4 linux/amd64

$ go run main.go
time.Time{wall:0x38dce280, ext:63757196485, loc:(*time.Location)(0xc0004e4e70)}
&time.Location{name:"", zone:[]time.zone{time.zone{name:"", offset:0, isDST:false}}, tx:[]time.zoneTrans{time.zoneTrans{when:-9223372036854775808, index:0x0, isstd:false, isutc:false}}, extend:"", cacheStart:-9223372036854775808, cacheEnd:9223372036854775807, cacheZone:(*time.zone)(0xc0004a67a0)}

Go playground:

time.Time{wall:0x38dce280, ext:63757196485, loc:(*time.Location)(0x5666c0)}
&time.Location{name:"Local", zone:[]time.zone{time.zone{name:"UTC", offset:0, isDST:false}}, tx:[]time.zoneTrans{time.zoneTrans{when:-576460752303423488, index:0x0, isstd:false, isutc:false}}, extend:"UTC0", cacheStart:9223372036854775807, cacheEnd:9223372036854775807, cacheZone:(*time.zone)(0xc0000c6000)

Local system has tzdata installed, importing time/tzdata doesn't change this either.

The difference in my case seems to be that when I run with env TZ=UTC on my local system, I get:

time.Time{wall:0x38dce280, ext:63757196485, loc:(*time.Location)(0x9044e0)}
&time.Location{name:"UTC", zone:[]time.zone(nil), tx:[]time.zoneTrans(nil), extend:"", cacheStart:0, cacheEnd:0, cacheZone:(*time.zone)(nil)}

I'm perhaps misunderstanding how time.Parse is meant to work, but given the time we're parsing is specified with offset +00:00 I would expect it to end up with a time.zone(nil) etc regardless of the value of TZ?

colin-sitehost commented 3 years ago

@daenney: caution time on the playground is screwey [recte deterministic], so that things are reproducable; but I think you are stumbling over the same thing I found.

daenney commented 3 years ago

I noticed the discrepancy with playground too and saw you mentioned the same thing. I updated my comment to include runs from my local system.

On further reflection, I suppose +00:00 could also be GMT, so I suppose always parsing +00:00 as UTC wouldn't be correct. But I do wonder why with env TZ=UTC the behaviour is altered.

It's worth noting though, that if I do strings.ReplaceAll(t, "+00:00", "Z") the result always get a UTC location, regardless of the value of the TZ environment variable. That seems inconsistent with +00:00 not being parsed as UTC, as Z just means "no offset from UTC", not "in UTC" according to the RFC

Z A suffix which, when applied to a time, denotes a UTC offset of 00:00; often spoken "Zulu" from the ICAO phonetic alphabet representation of the letter "Z".

slowaner commented 3 years ago

I have related problems with Location. In my case the reason is that functions Now() and unixTime(sec int64, nsec int32) don't use t.setLoc(Local) but just binds it to Time.loc. Other functions uses t.setLoc(Local) and when Local == utcLoc it sets nil to Time.loc. https://github.com/golang/go/blob/912f0750472dd4f674b69ca1616bfaf377af1805/src/time/time.go#L1073-L1081 https://github.com/golang/go/blob/912f0750472dd4f674b69ca1616bfaf377af1805/src/time/time.go#L1083-L1085

I faced this issue when migrating from lib/pq to pgx. This behavior ruined my tests. I had to forcely set Local to custom timezone (not utc) to make it work. This problem is mentioned in https://github.com/jackc/pgx/issues/863

antong commented 2 years ago

Relevant, I think: https://github.com/golang/go/issues/30114#issuecomment-461372130

time: Parse not setting Location to Local when TZ=UTC

I think this is not an issue, because when TZ=UTC, then time.Local and time.UTC have the same contents even though they are separate variables (GoString() shows different names, but the behavior is identical unless you actually fmt the time with "%#v"). I think it works as intended for TZ=UTC. However, there is a potential issue with real locations (not UTC) that have a zero offset to UTC. They get serialized with an appended "Z" instead of the numeric zone offset, and deserializing always gives a time with the location UTC. So a serialize/deserialize roundtrip of a local time (like time.Now()) gives back the original location everywhere else, but for locations with a zero UTC offset at the time, you will instead get back UTC. This can cause peculiar results, especially for locations such as London that part of the time has a zero offset to UTC, and otherwise not due to daylight saving time.

Example: https://play.golang.org/p/ema0DpRVUsu (note the last time for London)

A workaround for timestamps and other use cases where it is possible to do so, is to always handle everything as UTC, so instead of writing time.Now(), write time.Now().UTC().