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: IST is incorrectly parsed in Europe/Dublin #36529

Open
nigelbabu opened this issue Jan 13, 2020 · 8 comments
Open

time: IST is incorrectly parsed in Europe/Dublin #36529

nigelbabu opened this issue Jan 13, 2020 · 8 comments
Milestone

Comments

@nigelbabu
Copy link

@nigelbabu nigelbabu commented Jan 13, 2020

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

$ go version
go version go1.13.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env
GOARCH="amd64" GOHOSTARCH="amd64" GOHOSTOS="linux" GOOS="linux" GOPROXY="https://proxy.golang.org,direct" GOSUMDB="sum.golang.org" GCCGO="gccgo" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="" 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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build876647815=/tmp/go-build -gno-record-gcc-switches"
$ go env

What did you do?

I wrote a parser to parse time with 3-letter timezones in Europe/Dublin. I tried variations of IST/GMT/UTC/BST. All of those work fine except for IST when Daylight Savings is off. See example code: https://play.golang.org/p/3iNUKCLFC2l

What did you expect to see?

IST should be parsed similar to BST or at the very least not be partially parsed into an offset that doesn't exist.

2020-01-12 09:30 IST should be equal to 2020-01-12 08:30:00 +0000 GMT

What did you see instead?

2020-01-12 09:30 IST is parsed as 2020-01-12 08:55:21 +0000 GMT

@nigelbabu
Copy link
Author

@nigelbabu nigelbabu commented Jan 13, 2020

This does work as expected when I pick a time when Daylight savings is actually on. For example, changing the month to June.

@nigelbabu
Copy link
Author

@nigelbabu nigelbabu commented Jan 13, 2020

A colleague pointed this out to me. From tzdata2019c, europe, line 529+:

# The following is like GB-Eire and EU, except with standard time in
# summer and negative daylight saving time in winter.  It is for when
# negative SAVE values are used.
# Rule	NAME	FROM	TO	TYPE	IN	ON	AT	SAVE	LETTER/S
Rule	Eire	1971	only	-	Oct	31	 2:00u	-1:00	-
Rule	Eire	1972	1980	-	Mar	Sun>=16	 2:00u	0	-
Rule	Eire	1972	1980	-	Oct	Sun>=23	 2:00u	-1:00	-
Rule	Eire	1981	max	-	Mar	lastSun	 1:00u	0	-
Rule	Eire	1981	1989	-	Oct	Sun>=23	 1:00u	-1:00	-
Rule	Eire	1990	1995	-	Oct	Sun>=22	 1:00u	-1:00	-
Rule	Eire	1996	max	-	Oct	lastSun	 1:00u	-1:00	-

# Zone	NAME		STDOFF	RULES	FORMAT	[UNTIL]
Zone	Europe/Dublin	-0:25:00 -	LMT	1880 Aug  2
			-0:25:21 -	DMT	1916 May 21  2:00s
			-0:25:21 1:00	IST	1916 Oct  1  2:00s
			 0:00	GB-Eire	%s	1921 Dec  6 # independence
			 0:00	GB-Eire	GMT/IST	1940 Feb 25  2:00s
			 0:00	1:00	IST	1946 Oct  6  2:00s
			 0:00	-	GMT	1947 Mar 16  2:00s
			 0:00	1:00	IST	1947 Nov  2  2:00s
			 0:00	-	GMT	1948 Apr 18  2:00s
			 0:00	GB-Eire	GMT/IST	1968 Oct 27
# The next line is for when negative SAVE values are used.
			 1:00	Eire	IST/GMT
# These three lines are for when SAVE values are always nonnegative.
#			 1:00	-	IST	1971 Oct 31  2:00u
#			 0:00	GB-Eire	GMT/IST	1996
#			 0:00	EU	GMT/IST

Looks like the offset is coming from when there used to be a DMT.

@cagedmantis cagedmantis changed the title time.Parse parses IST incorrectly in Europe/Dublin time: IST is incorrectly parsed in Europe/Dublin Jan 13, 2020
@cagedmantis cagedmantis added this to the Backlog milestone Jan 13, 2020
@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented Jan 13, 2020

/cc @rsc

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 13, 2020

Go's time package simply reflects the local system's timezone data. As far as I can tell, that is what is happening here, and there is no bug in Go. The data needs to be changed in tzdata instead. If you think there is a bug in Go, could you describe it more precisely? Thanks.

@nigelbabu
Copy link
Author

@nigelbabu nigelbabu commented Jan 14, 2020

I'm unsure whether there is a problem in Golang parser or not. Here's an example with the date command:

 $ TZ=Europe/Dublin date -d '2019-01-12 09:30 IST'
date: invalid date ‘2019-01-12 09:30 IST’
$ TZ=Europe/Dublin date -d '2019-06-12 09:30 IST'
Wed 12 Jun 2019 09:30:00 AM IST

The date command "correctly" rejects that when Daylight Savings is not "on", IST is an invalid timezone. The Golang parser on the other hand, converts it into Dublin Mean Time. Is that expected behavior? My quick read of the tzdata rules isn't comprehensive enough to understand whether this is expected.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 14, 2020

I also don't know what is correct. Currently what Go does is look up the name "IST" in the location, and check whether it is valid for the given date. In this case it is not, so Go simply looks for the first occurrence of "IST" in the time zone, which is the one with the 25:21 offset. Usually that algorithm seems like a decent guess at what the user meant. In this case it's not so good.

Perhaps we should use the last occurrence of the name in the time zone.

@nigelbabu
Copy link
Author

@nigelbabu nigelbabu commented Jan 14, 2020

That would still be a bad idea if you tried to use the Parser for times in the past. It may need more careful parsing based on the actual time. Is that supported or in scope?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 14, 2020

We don't currently have an API for asking whether a time is supported in a given timezone. And we don't reject dates because their timezone seems invalid, we just try to handle them as best we can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.