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: Parse not setting Location to Local when TZ=UTC #45960

Open
colin-sitehost opened this issue May 5, 2021 · 12 comments
Open

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

colin-sitehost opened this issue May 5, 2021 · 12 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@colin-sitehost
Copy link

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

This comment has been minimized.

@colin-sitehost

This comment has been minimized.

@seankhliao
Copy link
Member

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

@seankhliao seankhliao reopened this May 5, 2021
@seankhliao seankhliao changed the title time: unmarshaling location different based on timezone time: Parse not setting Location to Local when TZ=UTC May 5, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 5, 2021
@ianlancetaylor
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

colin-sitehost commented May 20, 2021

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
Copy link

daenney commented May 23, 2021

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
Copy link
Author

@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
Copy link

daenney commented May 24, 2021

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
Copy link

slowaner commented Jul 6, 2021

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.

go/src/time/time.go

Lines 1073 to 1081 in 912f075

func Now() Time {
sec, nsec, mono := now()
mono -= startNano
sec += unixToInternal - minWall
if uint64(sec)>>33 != 0 {
return Time{uint64(nsec), sec + minWall, Local}
}
return Time{hasMonotonic | uint64(sec)<<nsecShift | uint64(nsec), mono, Local}
}

go/src/time/time.go

Lines 1083 to 1085 in 912f075

func unixTime(sec int64, nsec int32) Time {
return Time{uint64(nsec), sec + unixToInternal, Local}
}

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 jackc/pgx#863

@antong
Copy link
Contributor

antong commented Aug 20, 2021

Relevant, I think: #30114 (comment)

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().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants