Skip to content

time: time#Format using time.RFC3339 using time.Time with time.Location Europe/London should use +00:00 instead of Z #68164

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

Closed
jufemaiz opened this issue Jun 25, 2024 · 10 comments
Labels
Documentation Issues describing a change to documentation.

Comments

@jufemaiz
Copy link
Contributor

Go version

go version go1.22.4 darwin/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN='/path/to/repo/directory/.hermit/go/bin'
GOCACHE='/Users/USERNAME/Library/Caches/go-build'
GOENV='/Users/USERNAME/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/USERNAME/go/pkg/mod'
GONOPROXY='github.com/USERNAME'
GONOSUMDB='github.com/USERNAME'
GOOS='darwin'
GOPATH='/Users/USERNAME/go'
GOPRIVATE='github.com/enosi'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/USERNAME/Library/Caches/hermit/pkg/[email protected]'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/Users/USERNAME/Library/Caches/hermit/pkg/[email protected]/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.22.4'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/path/to/repo/directory/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/9q/v1dfygx14czghp71cfmb68r00000gn/T/go-build1533160705=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Ref: https://go.dev/play/p/QLy4ID0CwiU

// You can edit this code!
// Click here and start typing.
package main

import (
	"fmt"
	"time"
)

func main() {
	tz, err := time.LoadLocation("Europe/London")
	if err != nil {
		panic(err)
	}

	tsDST := time.Date(2024, 6, 1, 0, 0, 0, 0, tz)
	tsST := time.Date(2024, 12, 1, 0, 0, 0, 0, tz)

	fmt.Println(tsDST.Format(time.RFC3339))
	fmt.Println(tsST.Format(time.RFC3339))
}

What did you see happen?

2024-06-01T00:00:00+01:00
2024-12-01T00:00:00Z

This is due to the following portion of code:

if offset == 0 {
return append(b, 'Z')
}

Now this is understandable for timezone like time.UTC, however as seen above, we've loaded a timezone that has variable.

What did you expect to see?

It would be great to see the following output for consistency:

2024-06-01T00:00:00+01:00
2024-12-01T00:00:00+00:00

However that may have breaking changes in the codebase.

An alternative would be to explicitly document time.RFC3339 to indicate that any timezone where the offset for a particular time is zero will be returned as Z not as +00:00 regardless of how the timezone may treat other points in time.

In effect, in the absence of being able to resolve a principle of least surprise issue, it would be useful to document the early exit of the time#appendFormatRFC3339 method with a Z instead of an [+-]HH:MM offset where the offset value is 0.

Alternatively, a breaking change would be to validate if the time's location is time.UTC, and use Z, otherwise use +00:00.

@gopherbot gopherbot added the Documentation Issues describing a change to documentation. label Jun 25, 2024
@jufemaiz jufemaiz changed the title time: time.Format using RFC3339 in timezone Europe/London uses Z not +00:00 in standard time but is not documented as such time: time#Format using time.RFC3339 in timezone Europe/London uses Z not +00:00 in standard time but is not documented as such Jun 25, 2024
@seankhliao seankhliao changed the title time: time#Format using time.RFC3339 in timezone Europe/London uses Z not +00:00 in standard time but is not documented as such time: RFC3339 outputs offset +00:00 as Z Jun 25, 2024
@seankhliao
Copy link
Member

RFC 3339 Section 2: https://www.rfc-editor.org/rfc/rfc3339#section-2

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".

Z means zero offset, not UTC itself.

@jufemaiz
Copy link
Contributor Author

RFC3339 paraphrases the source material – ISO8601.

image Ref: ISO 8601-1:2019(en)

@cooler-SAI
Copy link

cooler-SAI commented Jun 25, 2024

Ok, guys, checked all month, here we go:

package main

import (
	"fmt"
	"time"
)

func main() {
	tz, err := time.LoadLocation("Europe/London")
	if err != nil {
		panic(err)
	}

	dates := []time.Time{
		time.Date(2024, 1, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 2, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 3, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 4, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 5, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 6, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 7, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 8, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 9, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 10, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 11, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 12, 1, 0, 0, 0, 0, tz),
	}

	for _, date := range dates {
		fmt.Println(date.Format(time.RFC3339))
	}
}

Result :

2024-01-01T00:00:00Z
2024-02-01T00:00:00Z
2024-03-01T00:00:00Z
2024-04-01T00:00:00+01:00
2024-05-01T00:00:00+01:00
2024-06-01T00:00:00+01:00
2024-07-01T00:00:00+01:00
2024-08-01T00:00:00+01:00
2024-09-01T00:00:00+01:00
2024-10-01T00:00:00+01:00
2024-11-01T00:00:00Z
2024-12-01T00:00:00Z

Hacked Fixed:

package main

import (
	"fmt"
	"time"
)

func main() {
	tz, err := time.LoadLocation("Europe/London")
	if err != nil {
		panic(err)
	}
	dates := []time.Time{
		time.Date(2024, 1, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 2, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 3, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 4, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 5, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 6, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 7, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 8, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 9, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 10, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 11, 1, 0, 0, 0, 0, tz),
		time.Date(2024, 12, 1, 0, 0, 0, 0, tz),
	}

	for _, date := range dates {
		fmt.Println(date.Format("2006-01-02T15:04:05-07:00"))
	}
}

Result 👍 , but not good

2024-01-01T00:00:00+00:00
2024-02-01T00:00:00+00:00
2024-03-01T00:00:00+00:00
2024-04-01T00:00:00+01:00
2024-05-01T00:00:00+01:00
2024-06-01T00:00:00+01:00
2024-07-01T00:00:00+01:00
2024-08-01T00:00:00+01:00
2024-09-01T00:00:00+01:00
2024-10-01T00:00:00+01:00
2024-11-01T00:00:00+00:00
2024-12-01T00:00:00+00:00

strange is : not all months have this issue

RFC 3339 Section 2: https://www.rfc-editor.org/rfc/rfc3339#section-2

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".

Z means zero offset, not UTC itself.

yeah, it's +00:00

@ianlancetaylor
Copy link
Contributor

I don't think it's feasible for us to change this now. If you don't like the Z, use a format string "2006-01-02T15:04:05-07:00" instead of time.RFC3339.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Jun 25, 2024
@jufemaiz
Copy link
Contributor Author

@ianlancetaylor to be clear, as I've mentioned:

An alternative would be to explicitly document time.RFC3339 to indicate that any timezone where the offset for a particular time is zero will be returned as Z not as +00:00 regardless of how the timezone may treat other points in time.

I'm disappointed that it appears the Golang team have failed to take into account the considerations indicated, or a wider discussion on the implicit use of "Z" and early return in the codebase if offset is zero, as opposed to being an explicit UTC timezone. As noted I've been explicit in one option being better documentation that any offset of zero IS ASSUMED TO BE UTC regardless of the timezone location being used.

Further, discussion points above indicate an assumption that "Z means zero offset, not UTC itself", yet the documentary evidence does not IMHO support this assertion (which is then implicit in the codebase).

Finally, RFC3339 is currently in review for an update, with further implicit and explicit examples highlighting my comments in this issue.

Ref: https://www.ietf.org/archive/id/draft-ietf-sedate-datetime-extended-11.html#name-optional-generation-electiv

@jufemaiz
Copy link
Contributor Author

jufemaiz commented Jun 25, 2024

Also, the change in the title by @seankhliao was unwarranted and completely at odds with the actual issue. I'm not happy that was made without a note of the change of scope, or a discussion over the topic.

@jufemaiz jufemaiz changed the title time: RFC3339 outputs offset +00:00 as Z time: time#Format using time.RFC3339 using time.Time with time.Location Europe/London should use +00:00 instead of Z Jun 25, 2024
@ianlancetaylor
Copy link
Contributor

Changing the current behavior is sure to break existing working code. We aren't going to do that.

We are always happy to improve the documentation. In this case I'm concerned that the documentation is already lengthy and complex and that adding more information won't actually help people. But I would be happy to look at a patch.

@jufemaiz
Copy link
Contributor Author

I've drafted a change which I know has no hope of going through, however, is indicative of what I understand of IS8601 (see https://github.com/golang/go/blob/master/src/time/format.go#L787-L817 where RFC3339 is broken down into ISO8601 parts, and, as I've noted, IMHO, incorrectly assumes a corollary of "UTC is offset of zero" to be "zero offset is UTC" even where location information is available).

https://github.com/ace-teknologi/go/blob/d9b5ea30ac12e78c65ae0e55c78f7cf88ff6a434/src/time/format.go#L787-L817
https://github.com/ace-teknologi/go/blob/d9b5ea30ac12e78c65ae0e55c78f7cf88ff6a434/src/time/format_rfc3339.go#L44-L46
https://github.com/ace-teknologi/go/blob/d9b5ea30ac12e78c65ae0e55c78f7cf88ff6a434/src/time/format.go#L666-L672

@jufemaiz
Copy link
Contributor Author

jufemaiz commented Jun 26, 2024

Changing the current behavior is sure to break existing working code. We aren't going to do that.

We are always happy to improve the documentation. In this case I'm concerned that the documentation is already lengthy and complex and that adding more information won't actually help people. But I would be happy to look at a patch.

Lengthy documentation in the time package is probably par for the course given the innate complexity of it, and its fundamental role in the language IMHO.

Happy to do so if that's the proposed path forward from the Golang team, but would thus prefer the issue to be reopened as a reference point for those changes.

Further, the proposed changes to be made (immediate comment above) should IMHO be considered as part of a Golang v2 release where breaking changes would be allowed, given a fundamental assumption is not necessarily in keeping with the specs it is built on (that is, a location that is not nil or time.UTC but coincidently has an offset of +00:00 is not UTC and should not use the UTC designator).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues describing a change to documentation.
Projects
None yet
Development

No branches or pull requests

6 participants