diff --git a/field.go b/field.go index 83c1ea245..dd558fc23 100644 --- a/field.go +++ b/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()} } diff --git a/field_test.go b/field_test.go index d25c964fb..cc7f2e564 100644 --- a/field_test.go +++ b/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)}, diff --git a/zapcore/field.go b/zapcore/field.go index ae772e4a1..6e05f831f 100644 --- a/zapcore/field.go +++ b/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: diff --git a/zapcore/field_test.go b/zapcore/field_test.go index 8ec045fd7..a0c9c12b7 100644 --- a/zapcore/field_test.go +++ b/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)),