From 0e6798dfb25ef3a6c994b391ef7ac51778613948 Mon Sep 17 00:00:00 2001 From: itchyny Date: Mon, 14 Mar 2022 10:22:01 +0900 Subject: [PATCH 1/4] Fix JSONFormat to ensure outputting JSON on NaN and Infinities --- json.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/json.go b/json.go index 7206665..c11ec9c 100644 --- a/json.go +++ b/json.go @@ -4,6 +4,7 @@ import ( "encoding" "encoding/json" "fmt" + "math" "reflect" "strconv" "strings" @@ -182,12 +183,12 @@ func appendJSON(buf []byte, v interface{}) ([]byte, error) { if cap(buf) < 256 { return nil, ErrTooLarge } - return strconv.AppendFloat(buf, float64(t), 'f', -1, 32), nil + return appendFloat(buf, float64(t), 32), nil case float64: if cap(buf) < 256 { return nil, ErrTooLarge } - return strconv.AppendFloat(buf, t, 'f', -1, 64), nil + return appendFloat(buf, t, 64), nil case string: if !utf8.ValidString(t) { // the next line replaces invalid characters. @@ -303,3 +304,15 @@ func appendJSON(buf []byte, v interface{}) ([]byte, error) { // other types are just formatted as a string with "%#v". return appendJSON(buf, fmt.Sprintf("%#v", v)) } + +func appendFloat(buf []byte, v float64, bitSize int) []byte { + if math.IsNaN(v) { + return append(buf, `"NaN"`...) + } else if math.IsInf(v, 1) { + return append(buf, `"+Inf"`...) + } else if math.IsInf(v, -1) { + return append(buf, `"-Inf"`...) + } else { + return strconv.AppendFloat(buf, v, 'f', -1, bitSize) + } +} From 091c1513029bda12dfe1f3a4601b55d316e03ba9 Mon Sep 17 00:00:00 2001 From: itchyny Date: Mon, 14 Mar 2022 10:27:11 +0900 Subject: [PATCH 2/4] Update JSONFormat test to cover floating-point numbers --- json_test.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/json_test.go b/json_test.go index 0c7b8ed..e8661c6 100644 --- a/json_test.go +++ b/json_test.go @@ -3,6 +3,7 @@ package log import ( "bytes" "encoding/json" + "math" "reflect" "testing" "time" @@ -111,6 +112,8 @@ func TestJSONFormat(t *testing.T) { b, err = f.Format(buf, l, ts, LvDebug, "fuga fuga", map[string]interface{}{ "abc": []int{1, 2, 3}, + "float32": []float32{3.14159, float32(math.NaN()), float32(math.Inf(1)), float32(math.Inf(-1))}, + "float64": []float64{3.14159, math.NaN(), math.Inf(1), math.Inf(-1)}, "invalid": "12\xc534", "tm": testTextMarshal{}, "jm": testJSONMarshal{}, @@ -153,7 +156,25 @@ func TestJSONFormat(t *testing.T) { } else { if !reflect.DeepEqual(v.([]interface{}), []interface{}{1.0, 2.0, 3.0}) { t.Error(`!reflect.DeepEqual(v.([]interface{}), []interface{}{1, 2, 3})`) - t.Logf("%#v", v.([]interface{})) + t.Logf("%#v", v) + } + } + + if v, ok := j["float32"]; !ok { + t.Error(`v, ok := j["float32"]; !ok`) + } else { + if !reflect.DeepEqual(v.([]interface{}), []interface{}{3.14159, "NaN", "+Inf", "-Inf"}) { + t.Error(`!reflect.DeepEqual(v.([]interface{}), []interface{}{3.14159, "NaN", "+Inf", "-Inf"})`) + t.Logf("%#v", v) + } + } + + if v, ok := j["float64"]; !ok { + t.Error(`v, ok := j["float64"]; !ok`) + } else { + if !reflect.DeepEqual(v.([]interface{}), []interface{}{3.14159, "NaN", "+Inf", "-Inf"}) { + t.Error(`!reflect.DeepEqual(v.([]interface{}), []interface{}{3.14159, "NaN", "+Inf", "-Inf"})`) + t.Logf("%#v", v) } } From 0e5bdc917b998d493f2e5258c2f00874628c1f44 Mon Sep 17 00:00:00 2001 From: itchyny Date: Mon, 14 Mar 2022 10:28:16 +0900 Subject: [PATCH 3/4] Update Logfmt test to look like PlainFormat test --- logfmt_test.go | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/logfmt_test.go b/logfmt_test.go index 6955b59..336292c 100644 --- a/logfmt_test.go +++ b/logfmt_test.go @@ -17,56 +17,49 @@ const ( func TestAppendLogfmt(t *testing.T) { t.Parallel() - b := make([]byte, 0, 4096) - b, _ = appendLogfmt(b, nil) + buf := make([]byte, 0, 4096) + + b, _ := appendLogfmt(buf, nil) if string(b) != "null" { t.Error(string(b) + " != null") } - b = b[:0] - b, _ = appendLogfmt(b, 100) + b, _ = appendLogfmt(buf, 100) if string(b) != "100" { t.Error(string(b) + " != 100") } - b = b[:0] - b, _ = appendLogfmt(b, false) + b, _ = appendLogfmt(buf, false) if string(b) != "false" { t.Error(string(b) + " != false") } - b = b[:0] - b, _ = appendLogfmt(b, true) + b, _ = appendLogfmt(buf, true) if string(b) != "true" { t.Error(string(b) + " != true") } - b = b[:0] - b, _ = appendLogfmt(b, `"abc `) + b, _ = appendLogfmt(buf, `"abc `) if string(b) != `"\"abc "` { t.Error(string(b) + ` != "\"abc "`) } - b = b[:0] - b, _ = appendLogfmt(b, []int{-100, 100, 20000}) + b, _ = appendLogfmt(buf, []int{-100, 100, 20000}) if string(b) != `[-100 100 20000]` { t.Error("failed to format int list") } - b = b[:0] - b, _ = appendLogfmt(b, []int64{-100, 100, 20000}) + b, _ = appendLogfmt(buf, []int64{-100, 100, 20000}) if string(b) != `[-100 100 20000]` { t.Error("failed to format int64 list") } - b = b[:0] - b, _ = appendLogfmt(b, []string{"abc", "def"}) + b, _ = appendLogfmt(buf, []string{"abc", "def"}) if string(b) != `["abc" "def"]` { t.Error("failed to format string list") } - b = b[:0] - b, _ = appendLogfmt(b, map[string]interface{}{ + b, _ = appendLogfmt(buf, map[string]interface{}{ "abc": 123, "def ghi": nil, }) @@ -76,8 +69,7 @@ func TestAppendLogfmt(t *testing.T) { } invalidUtf8 := "hello" + string([]byte{0x80}) - b = b[:0] - b, err := appendLogfmt(b, invalidUtf8) + b, err := appendLogfmt(buf, invalidUtf8) if err != nil { t.Error(err) } else { @@ -92,8 +84,7 @@ func TestAppendLogfmt(t *testing.T) { } } - b = b[:0] - b, err = appendLogfmt(b, fmt.Errorf(invalidUtf8)) + b, err = appendLogfmt(buf, fmt.Errorf(invalidUtf8)) if err != nil { t.Error(err) } else { From 80e26a39d4ce09462236ebf5f8877ab5c51d152d Mon Sep 17 00:00:00 2001 From: itchyny Date: Mon, 14 Mar 2022 10:30:21 +0900 Subject: [PATCH 4/4] Update PlainFormat and Logfmt tests to cover floating-point numbers --- logfmt_test.go | 36 +++++++++++++++++++++++++++++++----- plain_test.go | 42 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/logfmt_test.go b/logfmt_test.go index 336292c..74738dd 100644 --- a/logfmt_test.go +++ b/logfmt_test.go @@ -3,6 +3,7 @@ package log import ( "bytes" "fmt" + "math" "testing" "time" "unicode/utf8" @@ -44,19 +45,44 @@ func TestAppendLogfmt(t *testing.T) { t.Error(string(b) + ` != "\"abc "`) } + b, _ = appendLogfmt(buf, float32(3.14159)) + if string(b) != "3.14159" { + t.Error(string(b) + ` != "3.14159"`) + } + + b, _ = appendLogfmt(buf, 3.14159) + if string(b) != "3.14159" { + t.Error(string(b) + ` != "3.14159"`) + } + + b, _ = appendLogfmt(buf, math.NaN()) + if string(b) != "NaN" { + t.Error(string(b) + ` != "NaN"`) + } + + b, _ = appendLogfmt(buf, math.Inf(1)) + if string(b) != "+Inf" { + t.Error(string(b) + ` != "+Inf"`) + } + + b, _ = appendLogfmt(buf, math.Inf(-1)) + if string(b) != "-Inf" { + t.Error(string(b) + ` != "-Inf"`) + } + b, _ = appendLogfmt(buf, []int{-100, 100, 20000}) - if string(b) != `[-100 100 20000]` { - t.Error("failed to format int list") + if string(b) != "[-100 100 20000]" { + t.Error(string(b) + ` != "[-100 100 20000]"`) } b, _ = appendLogfmt(buf, []int64{-100, 100, 20000}) - if string(b) != `[-100 100 20000]` { - t.Error("failed to format int64 list") + if string(b) != "[-100 100 20000]" { + t.Error(string(b) + ` != "[-100 100 20000]"`) } b, _ = appendLogfmt(buf, []string{"abc", "def"}) if string(b) != `["abc" "def"]` { - t.Error("failed to format string list") + t.Error(string(b) + ` != ["abc" "def"]`) } b, _ = appendLogfmt(buf, map[string]interface{}{ diff --git a/plain_test.go b/plain_test.go index 808f473..1f350c2 100644 --- a/plain_test.go +++ b/plain_test.go @@ -3,6 +3,7 @@ package log import ( "bytes" "fmt" + "math" "testing" "time" "unicode/utf8" @@ -15,17 +16,52 @@ func TestAppendPlain(t *testing.T) { b, _ := appendPlain(buf, nil) if string(b) != "null" { - t.Error(`string(b) != "null"`) + t.Error(string(b) + ` != "null"`) + } + + b, _ = appendLogfmt(buf, 100) + if string(b) != "100" { + t.Error(string(b) + " != 100") + } + + b, _ = appendPlain(buf, false) + if string(b) != "false" { + t.Error(string(b) + ` != "false"`) } b, _ = appendPlain(buf, true) if string(b) != "true" { - t.Error(`string(b) != "true"`) + t.Error(string(b) + ` != "true"`) } b, _ = appendPlain(buf, int16(-12345)) if string(b) != "-12345" { - t.Error(`string(b) != "-12345"`) + t.Error(string(b) + ` != "-12345"`) + } + + b, _ = appendPlain(buf, float32(3.14159)) + if string(b) != "3.14159" { + t.Error(string(b) + ` != "3.14159"`) + } + + b, _ = appendPlain(buf, 3.14159) + if string(b) != "3.14159" { + t.Error(string(b) + ` != "3.14159"`) + } + + b, _ = appendPlain(buf, math.NaN()) + if string(b) != "NaN" { + t.Error(string(b) + ` != "NaN"`) + } + + b, _ = appendPlain(buf, math.Inf(1)) + if string(b) != "+Inf" { + t.Error(string(b) + ` != "+Inf"`) + } + + b, _ = appendPlain(buf, math.Inf(-1)) + if string(b) != "-Inf" { + t.Error(string(b) + ` != "-Inf"`) } b, err := appendPlain(buf, []string{"abc", "def"})