[release-branch.go1.15] time: use offset and isDST when caching zone from extend string

If the current time is computed from extend string
and the zone file contains multiple zones with the
same name, the lookup by name might find incorrect
zone.

This happens for example with the slim Europe/Dublin
time zone file in the embedded zip. This zone file
has last transition in 1996 and rest is covered by
extend string.
tzset returns IST as the zone name to use, but there
are two records with IST name. Lookup by name finds
the wrong one. We need to check offset and isDST too.

In case we can't find an existing zone, we allocate
a new zone so that we use correct offset and isDST.

I have renamed zone variable to zones as it shadowed
the zone type that we need to allocate the cached zone.

Backport note: this change also incorporates portions of
CL 264077.

For #45370
Fixes #45384

Change-Id: I43d416d009e20878261156c821a5784e2407ed1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/307212
Trust: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Emmanuel Odeke <[email protected]>

Author: ianlancetaylor

src/time/zoneinfo.go

@@ -178,7 +178,7 @@ func (l *Location) lookup(sec int64) (name string, offset int, start, end int64)
 	// If we're at the end of the known zone transitions,
 	// try the extend string.
 	if lo == len(tx)-1 && l.extend != "" {
-		if ename, eoffset, estart, eend, ok := tzset(l.extend, end, sec); ok {
+		if ename, eoffset, estart, eend, _, ok := tzset(l.extend, end, sec); ok {
 			return ename, eoffset, estart, eend
 		}
 	}
@@ -244,7 +244,7 @@ func (l *Location) firstZoneUsed() bool {
 // We call this a tzset string since in C the function tzset reads TZ.
 // The return values are as for lookup, plus ok which reports whether the
 // parse succeeded.
-func tzset(s string, initEnd, sec int64) (name string, offset int, start, end int64, ok bool) {
+func tzset(s string, initEnd, sec int64) (name string, offset int, start, end int64, isDST, ok bool) {
 	var (
 		stdName, dstName     string
 		stdOffset, dstOffset int
@@ -255,7 +255,7 @@ func tzset(s string, initEnd, sec int64) (name string, offset int, start, end in
 		stdOffset, s, ok = tzsetOffset(s)
 	}
 	if !ok {
-		return "", 0, 0, 0, false
+		return "", 0, 0, 0, false, false
 	}
 
 	// The numbers in the tzset string are added to local time to get UTC,
@@ -265,7 +265,7 @@ func tzset(s string, initEnd, sec int64) (name string, offset int, start, end in
 
 	if len(s) == 0 || s[0] == ',' {
 		// No daylight savings time.
-		return stdName, stdOffset, initEnd, omega, true
+		return stdName, stdOffset, initEnd, omega, false, true
 	}
 
 	dstName, s, ok = tzsetName(s)
@@ -278,7 +278,7 @@ func tzset(s string, initEnd, sec int64) (name string, offset int, start, end in
 		}
 	}
 	if !ok {
-		return "", 0, 0, 0, false
+		return "", 0, 0, 0, false, false
 	}
 
 	if len(s) == 0 {
@@ -287,19 +287,19 @@ func tzset(s string, initEnd, sec int64) (name string, offset int, start, end in
 	}
 	// The TZ definition does not mention ';' here but tzcode accepts it.
 	if s[0] != ',' && s[0] != ';' {
-		return "", 0, 0, 0, false
+		return "", 0, 0, 0, false, false
 	}
 	s = s[1:]
 
 	var startRule, endRule rule
 	startRule, s, ok = tzsetRule(s)
 	if !ok || len(s) == 0 || s[0] != ',' {
-		return "", 0, 0, 0, false
+		return "", 0, 0, 0, false, false
 	}
 	s = s[1:]
 	endRule, s, ok = tzsetRule(s)
 	if !ok || len(s) > 0 {
-		return "", 0, 0, 0, false
+		return "", 0, 0, 0, false, false
 	}
 
 	year, _, _, yday := absDate(uint64(sec+unixToInternal+internalToAbsolute), false)
@@ -313,22 +313,27 @@ func tzset(s string, initEnd, sec int64) (name string, offset int, start, end in
 
 	startSec := int64(tzruleTime(year, startRule, stdOffset))
 	endSec := int64(tzruleTime(year, endRule, dstOffset))
+	dstIsDST, stdIsDST := true, false
+	// Note: this is a flipping of "DST" and "STD" while retaining the labels
+	// This happens in southern hemispheres. The labelling here thus is a little
+	// inconsistent with the goal.
 	if endSec < startSec {
 		startSec, endSec = endSec, startSec
 		stdName, dstName = dstName, stdName
 		stdOffset, dstOffset = dstOffset, stdOffset
+		stdIsDST, dstIsDST = dstIsDST, stdIsDST
 	}
 
 	// The start and end values that we return are accurate
 	// close to a daylight savings transition, but are otherwise
 	// just the start and end of the year. That suffices for
 	// the only caller that cares, which is Date.
 	if ysec < startSec {
-		return stdName, stdOffset, abs, startSec + abs, true
+		return stdName, stdOffset, abs, startSec + abs, stdIsDST, true
 	} else if ysec >= endSec {
-		return stdName, stdOffset, endSec + abs, abs + 365*secondsPerDay, true
+		return stdName, stdOffset, endSec + abs, abs + 365*secondsPerDay, stdIsDST, true
 	} else {
-		return dstName, dstOffset, startSec + abs, endSec + abs, true
+		return dstName, dstOffset, startSec + abs, endSec + abs, dstIsDST, true
 	}
 }
 

src/time/zoneinfo_read.go

@@ -247,8 +247,8 @@ func LoadLocationFromTZData(name string, data []byte) (*Location, error) {
 		// This also avoids a panic later when we add and then use a fake transition (golang.org/issue/29437).
 		return nil, badData
 	}
-	zone := make([]zone, nzone)
-	for i := range zone {
+	zones := make([]zone, nzone)
+	for i := range zones {
 		var ok bool
 		var n uint32
 		if n, ok = zonedata.big4(); !ok {
@@ -257,22 +257,22 @@ func LoadLocationFromTZData(name string, data []byte) (*Location, error) {
 		if uint32(int(n)) != n {
 			return nil, badData
 		}
-		zone[i].offset = int(int32(n))
+		zones[i].offset = int(int32(n))
 		var b byte
 		if b, ok = zonedata.byte(); !ok {
 			return nil, badData
 		}
-		zone[i].isDST = b != 0
+		zones[i].isDST = b != 0
 		if b, ok = zonedata.byte(); !ok || int(b) >= len(abbrev) {
 			return nil, badData
 		}
-		zone[i].name = byteString(abbrev[b:])
+		zones[i].name = byteString(abbrev[b:])
 		if runtime.GOOS == "aix" && len(name) > 8 && (name[:8] == "Etc/GMT+" || name[:8] == "Etc/GMT-") {
 			// There is a bug with AIX 7.2 TL 0 with files in Etc,
 			// GMT+1 will return GMT-1 instead of GMT+1 or -01.
 			if name != "Etc/GMT+0" {
 				// GMT+0 is OK
-				zone[i].name = name[4:]
+				zones[i].name = name[4:]
 			}
 		}
 	}
@@ -295,7 +295,7 @@ func LoadLocationFromTZData(name string, data []byte) (*Location, error) {
 			}
 		}
 		tx[i].when = n
-		if int(txzones[i]) >= len(zone) {
+		if int(txzones[i]) >= len(zones) {
 			return nil, badData
 		}
 		tx[i].index = txzones[i]
@@ -314,7 +314,7 @@ func LoadLocationFromTZData(name string, data []byte) (*Location, error) {
 	}
 
 	// Committed to succeed.
-	l := &Location{zone: zone, tx: tx, name: name, extend: extend}
+	l := &Location{zone: zones, tx: tx, name: name, extend: extend}
 
 	// Fill in the cache with information about right now,
 	// since that will be the most common lookup.
@@ -323,33 +323,43 @@ func LoadLocationFromTZData(name string, data []byte) (*Location, error) {
 		if tx[i].when <= sec && (i+1 == len(tx) || sec < tx[i+1].when) {
 			l.cacheStart = tx[i].when
 			l.cacheEnd = omega
-			zoneIdx := tx[i].index
+			l.cacheZone = &l.zone[tx[i].index]
 			if i+1 < len(tx) {
 				l.cacheEnd = tx[i+1].when
 			} else if l.extend != "" {
 				// If we're at the end of the known zone transitions,
 				// try the extend string.
-				if name, _, estart, eend, ok := tzset(l.extend, l.cacheEnd, sec); ok {
+				if name, offset, estart, eend, isDST, ok := tzset(l.extend, l.cacheEnd, sec); ok {
 					l.cacheStart = estart
 					l.cacheEnd = eend
-					// Find the zone that is returned by tzset,
-					// the last transition is not always the correct zone.
-					for i, z := range l.zone {
-						if z.name == name {
-							zoneIdx = uint8(i)
-							break
+					// Find the zone that is returned by tzset to avoid allocation if possible.
+					if zoneIdx := findZone(l.zone, name, offset, isDST); zoneIdx != -1 {
+						l.cacheZone = &l.zone[zoneIdx]
+					} else {
+						l.cacheZone = &zone{
+							name:   name,
+							offset: offset,
+							isDST:  isDST,
 						}
 					}
 				}
 			}
-			l.cacheZone = &l.zone[zoneIdx]
 			break
 		}
 	}
 
 	return l, nil
 }
 
+func findZone(zones []zone, name string, offset int, isDST bool) int {
+	for i, z := range zones {
+		if z.name == name && z.offset == offset && z.isDST == isDST {
+			return i
+		}
+	}
+	return -1
+}
+
 // loadTzinfoFromDirOrZip returns the contents of the file with the given name
 // in dir. dir can either be an uncompressed zip file, or a directory.
 func loadTzinfoFromDirOrZip(dir, name string) ([]byte, error) {