Skip to content

Commit 4b766da

Browse files
committed
fix(time): address Copilot review nits
- safari: guard float64 seconds against out-of-range conversion to int64 (Go spec makes those implementation-dependent); bound by maxCoreDataSeconds so year clamp at the end becomes unnecessary. - firefox: correct clampJSON's documented range to [1, 9999] so the comment matches the predicate. - tests: swap assert.Equal for assert.Same on UTC location checks, so any leaked time.Local is caught via pointer identity regardless of the runner's TZ (t.Setenv was ineffective since time.Local is cached at process init).
1 parent 1c44b56 commit 4b766da

5 files changed

Lines changed: 21 additions & 15 deletions

File tree

browser/chromium/chromium_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -743,9 +743,11 @@ func TestTimeEpoch_NegativeReturnsZeroTime(t *testing.T) {
743743
}
744744

745745
func TestTimeEpoch_AlwaysUTC(t *testing.T) {
746-
t.Setenv("TZ", "Asia/Shanghai")
746+
// assert.Same checks pointer equality: time.UTC and time.Local are
747+
// distinct *Location globals, so this catches any regression that
748+
// drops .UTC() even when the runner's TZ happens to be UTC.
747749
got := timeEpoch(anchorChromiumMicros)
748-
assert.Equal(t, time.UTC, got.Location())
750+
assert.Same(t, time.UTC, got.Location())
749751
}
750752

751753
func TestTimeEpoch_MicrosecondPrecisionPreserved(t *testing.T) {

browser/firefox/firefox.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ func firefoxSeconds(s int64) time.Time {
315315
return clampJSON(time.Unix(s, 0).UTC())
316316
}
317317

318-
// clampJSON maps years outside time.Time.MarshalJSON's [0, 9999] window
318+
// clampJSON maps years outside time.Time.MarshalJSON's [1, 9999] window
319319
// to the zero time, so JSON export can't crash on sentinel inputs.
320320
func clampJSON(t time.Time) time.Time {
321321
if t.Year() < 1 || t.Year() > 9999 {

browser/firefox/firefox_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,10 +361,11 @@ func TestFirefoxHelpers_NegativeReturnsZeroTime(t *testing.T) {
361361
}
362362

363363
func TestFirefoxHelpers_AlwaysUTC(t *testing.T) {
364-
t.Setenv("TZ", "Asia/Shanghai")
365-
assert.Equal(t, time.UTC, firefoxMicros(anchorUnixSeconds*1_000_000).Location())
366-
assert.Equal(t, time.UTC, firefoxMillis(anchorUnixSeconds*1_000).Location())
367-
assert.Equal(t, time.UTC, firefoxSeconds(anchorUnixSeconds).Location())
364+
// assert.Same: pointer equality reliably catches any helper that
365+
// leaks time.Local, independent of the runner's configured TZ.
366+
assert.Same(t, time.UTC, firefoxMicros(anchorUnixSeconds*1_000_000).Location())
367+
assert.Same(t, time.UTC, firefoxMillis(anchorUnixSeconds*1_000).Location())
368+
assert.Same(t, time.UTC, firefoxSeconds(anchorUnixSeconds).Location())
368369
}
369370

370371
func TestFirefoxHelpers_SameMomentAcrossUnits(t *testing.T) {

browser/safari/safari.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,18 +215,20 @@ func resolveSourcePaths(sources map[types.Category][]sourcePath) map[types.Categ
215215
// Offset from the Core Data epoch (2001-01-01 UTC) to the Unix epoch.
216216
const coreDataEpochOffset = 978307200
217217

218+
// maxCoreDataSeconds is the largest CFAbsoluteTime that still lands inside
219+
// time.Time.MarshalJSON's [1, 9999] year window. Also bounds the float →
220+
// int64 conversion below; Go's spec makes out-of-range conversions return
221+
// an implementation-dependent int64, which could silently corrupt results.
222+
const maxCoreDataSeconds = 252423993600
223+
218224
// coredataTimestamp converts Core Data seconds (CFAbsoluteTime) to UTC.
219225
// Returns zero for non-positive input or out-of-JSON-range values.
220226
func coredataTimestamp(seconds float64) time.Time {
221-
if seconds <= 0 {
227+
if seconds <= 0 || seconds > maxCoreDataSeconds {
222228
return time.Time{}
223229
}
224230
whole := int64(seconds)
225231
frac := seconds - float64(whole)
226232
nanos := int64(frac * 1e9)
227-
t := time.Unix(whole+coreDataEpochOffset, nanos).UTC()
228-
if t.Year() < 1 || t.Year() > 9999 {
229-
return time.Time{}
230-
}
231-
return t
233+
return time.Unix(whole+coreDataEpochOffset, nanos).UTC()
232234
}

browser/safari/safari_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,8 @@ func TestCoredataTimestamp_FractionalSecondsPreserved(t *testing.T) {
359359
}
360360

361361
func TestCoredataTimestamp_AlwaysUTC(t *testing.T) {
362-
t.Setenv("TZ", "Asia/Shanghai")
362+
// assert.Same: pointer equality reliably catches any regression that
363+
// leaks time.Local, independent of the runner's configured TZ.
363364
got := coredataTimestamp(float64(anchorCoreDataSeconds))
364-
assert.Equal(t, time.UTC, got.Location())
365+
assert.Same(t, time.UTC, got.Location())
365366
}

0 commit comments

Comments
 (0)