Fix handling of Time values out of UnixNano range (uber-go#804)

Fixes uber-go#737, uber-go#803.

Time values are encoded by storing the UnixNano representation, but this
fails when the value of UnixNano is out of int64 range, see docs:

https://golang.org/pkg/time/#Time.UnixNano
> The result is undefined if the Unix time in nanoseconds cannot be
represented by an int64 (a date before the year 1678 or after 2262)

Fix this by storing values outside of UnixNano range as-is rather
than using UnixNano with a new Field type.

Author: prashantv

field.go

@@ -32,6 +32,11 @@ import (
 // improves the navigability of this package's API documentation.
 type Field = zapcore.Field
 
+var (
+	_minTimeInt64 = time.Unix(0, math.MinInt64)
+	_maxTimeInt64 = time.Unix(0, math.MaxInt64)
+)
+
 // Skip constructs a no-op field, which is often useful when handling invalid
 // inputs in other Field constructors.
 func Skip() Field {
@@ -339,6 +344,9 @@ func Stringer(key string, val fmt.Stringer) Field {
 // Time constructs a Field with the given key and value. The encoder
 // controls how the time is serialized.
 func Time(key string, val time.Time) Field {
+	if val.Before(_minTimeInt64) || val.After(_maxTimeInt64) {
+		return Field{Key: key, Type: zapcore.TimeFullType, Interface: val}
+	}
 	return Field{Key: key, Type: zapcore.TimeType, Integer: val.UnixNano(), Interface: val.Location()}
 }
 

field_test.go

@@ -21,6 +21,7 @@
 package zap
 
 import (
+	"math"
 	"net"
 	"sync"
 	"testing"
@@ -107,6 +108,10 @@ func TestFieldConstructors(t *testing.T) {
 		{"String", Field{Key: "k", Type: zapcore.StringType, String: "foo"}, String("k", "foo")},
 		{"Time", Field{Key: "k", Type: zapcore.TimeType, Integer: 0, Interface: time.UTC}, Time("k", time.Unix(0, 0).In(time.UTC))},
 		{"Time", Field{Key: "k", Type: zapcore.TimeType, Integer: 1000, Interface: time.UTC}, Time("k", time.Unix(0, 1000).In(time.UTC))},
+		{"Time", Field{Key: "k", Type: zapcore.TimeType, Integer: math.MinInt64, Interface: time.UTC}, Time("k", time.Unix(0, math.MinInt64).In(time.UTC))},
+		{"Time", Field{Key: "k", Type: zapcore.TimeType, Integer: math.MaxInt64, Interface: time.UTC}, Time("k", time.Unix(0, math.MaxInt64).In(time.UTC))},
+		{"Time", Field{Key: "k", Type: zapcore.TimeFullType, Interface: time.Time{}}, Time("k", time.Time{})},
+		{"Time", Field{Key: "k", Type: zapcore.TimeFullType, Interface: time.Unix(math.MaxInt64, 0)}, Time("k", time.Unix(math.MaxInt64, 0))},
 		{"Uint", Field{Key: "k", Type: zapcore.Uint64Type, Integer: 1}, Uint("k", 1)},
 		{"Uint64", Field{Key: "k", Type: zapcore.Uint64Type, Integer: 1}, Uint64("k", 1)},
 		{"Uint32", Field{Key: "k", Type: zapcore.Uint32Type, Integer: 1}, Uint32("k", 1)},

zapcore/field.go

@@ -65,8 +65,11 @@ const (
 	Int8Type
 	// StringType indicates that the field carries a string.
 	StringType
-	// TimeType indicates that the field carries a time.Time.
+	// TimeType indicates that the field carries a time.Time that is
+	// representable by a UnixNano() stored as an int64.
 	TimeType
+	// TimeFullType indicates that the field carries a time.Time stored as-is.
+	TimeFullType
 	// Uint64Type indicates that the field carries a uint64.
 	Uint64Type
 	// Uint32Type indicates that the field carries a uint32.
@@ -145,6 +148,8 @@ func (f Field) AddTo(enc ObjectEncoder) {
 			// Fall back to UTC if location is nil.
 			enc.AddTime(f.Key, time.Unix(0, f.Integer))
 		}
+	case TimeFullType:
+		enc.AddTime(f.Key, f.Interface.(time.Time))
 	case Uint64Type:
 		enc.AddUint64(f.Key, uint64(f.Integer))
 	case Uint32Type:

zapcore/field_test.go

@@ -29,6 +29,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	. "go.uber.org/zap/zapcore"
 )
@@ -164,6 +165,14 @@ func TestFields(t *testing.T) {
 }
 
 func TestEquals(t *testing.T) {
+	// Values outside the UnixNano range were encoded incorrectly (#737, #803).
+	timeOutOfRangeHigh := time.Unix(0, math.MaxInt64).Add(time.Nanosecond)
+	timeOutOfRangeLow := time.Unix(0, math.MinInt64).Add(-time.Nanosecond)
+	timeOutOfRangeHighNano := time.Unix(0, timeOutOfRangeHigh.UnixNano())
+	timeOutOfRangeLowNano := time.Unix(0, timeOutOfRangeLow.UnixNano())
+	require.False(t, timeOutOfRangeHigh.Equal(timeOutOfRangeHighNano), "should be different as value is >  UnixNano range")
+	require.False(t, timeOutOfRangeHigh.Equal(timeOutOfRangeHighNano), "should be different as value is <  UnixNano range")
+
 	tests := []struct {
 		a, b Field
 		want bool
@@ -198,6 +207,16 @@ func TestEquals(t *testing.T) {
 			b:    zap.Time("k", time.Unix(1000, 1000).In(time.FixedZone("TEST", -8))),
 			want: false,
 		},
+		{
+			a:    zap.Time("k", timeOutOfRangeLow),
+			b:    zap.Time("k", timeOutOfRangeLowNano),
+			want: false,
+		},
+		{
+			a:    zap.Time("k", timeOutOfRangeHigh),
+			b:    zap.Time("k", timeOutOfRangeHighNano),
+			want: false,
+		},
 		{
 			a:    zap.Time("k", time.Unix(1000, 1000)),
 			b:    zap.Time("k", time.Unix(1000, 2000)),