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.

Sign up for GitHub

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

Jump to bottom

time: ParseInLocation does not adjust timezone offset when seconds are not explicitly set #53919

Closed
MarkRosemaker opened this issue Jul 16, 2022 · 6 comments
Closed

time: ParseInLocation does not adjust timezone offset when seconds are not explicitly set #53919

MarkRosemaker opened this issue Jul 16, 2022 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@MarkRosemaker
Copy link

MarkRosemaker commented Jul 16, 2022 •
edited

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

$ go version
go version go1.18.4 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="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/mark/Library/Caches/go-build"
GOENV="/Users/mark/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS="-mod=mod"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/mark/go/pkg/mod"
GONOPROXY="github.com/flypay/*,github.com/faetools/*"
GONOSUMDB="github.com/flypay/*,github.com/faetools/*"
GOOS="darwin"
GOPATH="/Users/mark/go"
GOPRIVATE="github.com/flypay/*,github.com/faetools/*"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.18.4/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.18.4/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/mark/go/src/github.com/golang/go/src/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/rw/2cbtm9v95bz86c0ktyjwj2_80000gn/T/go-build2898766309=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I ran into an issue where a time.Time object was seconds off after having been converted into a string using Format with RFC3339 and back using ParseInLocation.

https://go.dev/play/p/5t8wa3b_ahv

What did you expect to see?

The test above succeeding, i.e. when I explicitly state my desired location that go is smart enough to adjust the seconds.

What did you see instead?

The test failing.
This is because the parsed time zone differs mere seconds from the desired time zone and RFC3339 expresses a time zone only in hours and minutes.
@MarkRosemaker MarkRosemaker mentioned this issue Jul 16, 2022
Closed
MarkRosemaker added a commit to MarkRosemaker/go that referenced this issue Jul 16, 2022
@MarkRosemaker
time: adjust time zone seconds offset in parse
aa05238 
Fixes golang#53919
@MarkRosemaker MarkRosemaker mentioned this issue Jul 16, 2022
Closed
@gopherbot
Copy link

Change https://go.dev/cl/417934 mentions this issue: time: adjust time zone offset if desired location differs mere seconds when parsing string

@seankhliao
Copy link
Member

The Z0700 represents a fixed time offset, not an actual timezone location. You would either need a named timezone or a the timezone offset with seconds to accurately represent your time.

second zone offset: https://go.dev/play/p/ivhN_nsQ_qB
named zone: https://go.dev/play/p/pVPflGCeBDk

See also #37277

@seankhliao
Copy link
Member

cc @rsc

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 16, 2022
@MarkRosemaker
Copy link
Author

Thank you, @seankhliao. Yes, I realize that. The expectation was that ParseInLocation is smart enough to work with the standard format RFC3339.

BTW I encountered the error when writing custom JSON marshalling and unmarshalling of an object that has a field for the time and a timezone string. For the API client to work, times have to be marshalled using standard RFC3339.

@ianlancetaylor
Copy link
Contributor

It seems to me that this problem occurs because the tzdata database says that the timezone for Asia/Shanghai in 1900 is 8:05:43, with a note saying "Milne gives 8:05:43.2 for Xujiahui Observatory time; round to nearest." However, the time format defined by RFC 3339 does not support seconds in the timezone, only hours and minutes. So printing a time that has a timezone offset measured in seconds produces unavoidable data loss. You are suggesting that we try to compensate for that, but I don't see how to do that in a way that is always accurate.

So I don't think that there is anything to do here. Code that needs to worry about these old dates for which the timezone database uses second offsets can check the timezone offset via t.Zone, and compensate for unusual cases.

@seankhliao
Copy link
Member

Closing as there appears to be nothing to do

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Aug 6, 2022
@golang golang locked and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
4 participants
@ianlancetaylor @gopherbot @seankhliao @MarkRosemaker