From: Jonathan Amsterdam <jba@google.com>
Date: Thu, 16 May 2024 12:14:52 +0000 (-0400)
Subject: log/slog: handle times with undefined UnixNanos
X-Git-Tag: go1.23rc1~313
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=db7bb2742ce01601842e277e7808d225ff8390cd;p=gostls13.git

log/slog: handle times with undefined UnixNanos

slog tries to represent a time.Time without allocations, which involves
storing its UnixNanos value. But UnixNanos is undefined for some valid
times. Provide a fallback representation for those times by storing them
in the `any` field of `Value`.

Fixes #65902.

Change-Id: I736c739a92f77d7b1122ea0831524acdd2c4703f
Reviewed-on: https://go-review.googlesource.com/c/go/+/585519
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
---

diff --git a/src/log/slog/value.go b/src/log/slog/value.go
index d278d9b923..6b0768eb1d 100644
--- a/src/log/slog/value.go
+++ b/src/log/slog/value.go
@@ -90,7 +90,7 @@ func (v Value) Kind() Kind {
 		return x
 	case stringptr:
 		return KindString
-	case timeLocation:
+	case timeLocation, timeTime:
 		return KindTime
 	case groupptr:
 		return KindGroup
@@ -139,9 +139,14 @@ func BoolValue(v bool) Value {
 	return Value{num: u, any: KindBool}
 }
 
-// Unexported version of *time.Location, just so we can store *time.Locations in
-// Values. (No user-provided value has this type.)
-type timeLocation *time.Location
+type (
+	// Unexported version of *time.Location, just so we can store *time.Locations in
+	// Values. (No user-provided value has this type.)
+	timeLocation *time.Location
+
+	// timeTime is for times where UnixNano is undefined.
+	timeTime time.Time
+)
 
 // TimeValue returns a [Value] for a [time.Time].
 // It discards the monotonic portion.
@@ -153,7 +158,15 @@ func TimeValue(v time.Time) Value {
 		// mistaken for any other Value, time.Time or otherwise.
 		return Value{any: timeLocation(nil)}
 	}
-	return Value{num: uint64(v.UnixNano()), any: timeLocation(v.Location())}
+	nsec := v.UnixNano()
+	t := time.Unix(0, nsec)
+	if v.Equal(t) {
+		// UnixNano correctly represents the time, so use a zero-alloc representation.
+		return Value{num: uint64(nsec), any: timeLocation(v.Location())}
+	}
+	// Fall back to the general form.
+	// Strip the monotonic portion to match the other representation.
+	return Value{any: timeTime(v.Round(0))}
 }
 
 // DurationValue returns a [Value] for a [time.Duration].
@@ -368,12 +381,19 @@ func (v Value) Time() time.Time {
 	return v.time()
 }
 
+// See TimeValue to understand how times are represented.
 func (v Value) time() time.Time {
-	loc := v.any.(timeLocation)
-	if loc == nil {
-		return time.Time{}
+	switch a := v.any.(type) {
+	case timeLocation:
+		if a == nil {
+			return time.Time{}
+		}
+		return time.Unix(0, int64(v.num)).In(a)
+	case timeTime:
+		return time.Time(a)
+	default:
+		panic(fmt.Sprintf("bad time type %T", v.any))
 	}
-	return time.Unix(0, int64(v.num)).In(loc)
 }
 
 // LogValuer returns v's value as a LogValuer. It panics
diff --git a/src/log/slog/value_test.go b/src/log/slog/value_test.go
index 033f945407..3e191589c5 100644
--- a/src/log/slog/value_test.go
+++ b/src/log/slog/value_test.go
@@ -30,7 +30,10 @@ func TestValueEqual(t *testing.T) {
 		BoolValue(true),
 		BoolValue(false),
 		TimeValue(testTime),
+		TimeValue(time.Time{}),
 		TimeValue(time.Date(2001, 1, 2, 3, 4, 5, 0, time.UTC)),
+		TimeValue(time.Date(2300, 1, 1, 0, 0, 0, 0, time.UTC)),            // overflows nanoseconds
+		TimeValue(time.Date(1715, 6, 13, 0, 25, 26, 290448384, time.UTC)), // overflowed value
 		AnyValue(&x),
 		AnyValue(&y),
 		GroupValue(Bool("b", true), Int("i", 3)),
@@ -229,11 +232,20 @@ func TestLogValue(t *testing.T) {
 	}
 }
 
-func TestZeroTime(t *testing.T) {
-	z := time.Time{}
-	got := TimeValue(z).Time()
-	if !got.IsZero() {
-		t.Errorf("got %s (%#[1]v), not zero time (%#v)", got, z)
+func TestValueTime(t *testing.T) {
+	// Validate that all representations of times work correctly.
+	for _, tm := range []time.Time{
+		time.Time{},
+		time.Unix(0, 1e15), // UnixNanos is defined
+		time.Date(2300, 1, 1, 0, 0, 0, 0, time.UTC), // overflows UnixNanos
+	} {
+		got := TimeValue(tm).Time()
+		if !got.Equal(tm) {
+			t.Errorf("got %s (%#[1]v), want %s (%#[2]v)", got, tm)
+		}
+		if g, w := got.Location(), tm.Location(); g != w {
+			t.Errorf("%s: location: got %v, want %v", tm, g, w)
+		}
 	}
 }