From d3f442ccd002affbdb996e2481afd0d9a56a2760 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 27 Apr 2020 10:42:52 +0200 Subject: [PATCH 1/2] format: stricter quoting based on strconv --- format.go | 52 ++++++++++------------------------------------------ 1 file changed, 10 insertions(+), 42 deletions(-) diff --git a/format.go b/format.go index 4e8e160..18cf5e5 100644 --- a/format.go +++ b/format.go @@ -7,7 +7,6 @@ import ( "reflect" "strconv" "strings" - "sync" "time" ) @@ -208,10 +207,9 @@ func formatJSONValue(value interface{}) interface{} { // formatValue formats a value for serialization func formatLogfmtValue(value interface{}) string { - if value == nil { + if value == nil || (reflect.ValueOf(value).Kind() == reflect.Ptr && reflect.ValueOf(value).IsNil()) { return "nil" } - if t, ok := value.(time.Time); ok { // Performance optimization: No need for escaping since the provided // timeFormat doesn't have any escape characters, and escaping is @@ -235,49 +233,19 @@ func formatLogfmtValue(value interface{}) string { } } -var stringBufPool = sync.Pool{ - New: func() interface{} { return new(bytes.Buffer) }, -} - +// escapeString checks if the provided string needs escaping/quoting, and +// calls strconv.Quote if needed func escapeString(s string) string { - needsQuotes := false - needsEscape := false + needsQuoting := false for _, r := range s { - if r <= ' ' || r == '=' || r == '"' { - needsQuotes = true - } - if r == '\\' || r == '"' || r == '\n' || r == '\r' || r == '\t' { - needsEscape = true + // quote everything below " (0x34) and above~ (0x7E), plus equal-sign + if r <= '"' || r > '~' || r == '=' { + needsQuoting = true + break } } - if needsEscape == false && needsQuotes == false { + if !needsQuoting { return s } - e := stringBufPool.Get().(*bytes.Buffer) - e.WriteByte('"') - for _, r := range s { - switch r { - case '\\', '"': - e.WriteByte('\\') - e.WriteByte(byte(r)) - case '\n': - e.WriteString("\\n") - case '\r': - e.WriteString("\\r") - case '\t': - e.WriteString("\\t") - default: - e.WriteRune(r) - } - } - e.WriteByte('"') - var ret string - if needsQuotes { - ret = e.String() - } else { - ret = string(e.Bytes()[1 : e.Len()-1]) - } - e.Reset() - stringBufPool.Put(e) - return ret + return strconv.Quote(s) } From a6a1bb2925779ec7e9cdebcd941d7ffed17f1c8d Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 28 Apr 2020 12:33:46 +0200 Subject: [PATCH 2/2] format, test: test for nil-interface vs nil-value --- format.go | 2 +- log15_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/format.go b/format.go index 18cf5e5..cf364fc 100644 --- a/format.go +++ b/format.go @@ -207,7 +207,7 @@ func formatJSONValue(value interface{}) interface{} { // formatValue formats a value for serialization func formatLogfmtValue(value interface{}) string { - if value == nil || (reflect.ValueOf(value).Kind() == reflect.Ptr && reflect.ValueOf(value).IsNil()) { + if value == nil { return "nil" } if t, ok := value.(time.Time); ok { diff --git a/log15_test.go b/log15_test.go index fb85fc3..b9a5ba6 100644 --- a/log15_test.go +++ b/log15_test.go @@ -158,11 +158,11 @@ func TestLogfmt(t *testing.T) { l, buf := testFormatter(LogfmtFormat()) l.Error("some message", "x", 1, "y", 3.2, "equals", "=", "quote", "\"", - "nil", nilVal, "carriage_return", "bang"+string('\r')+"foo", "tab", "bar baz", "newline", "foo\nbar") + "nilval", nilVal, "nil", nil, "carriage_return", "bang"+string('\r')+"foo", "tab", "bar baz", "newline", "foo\nbar") // skip timestamp in comparison got := buf.Bytes()[27:buf.Len()] - expected := []byte(`lvl=eror msg="some message" x=1 y=3.200 equals="=" quote="\"" nil=nil carriage_return="bang\rfoo" tab="bar\tbaz" newline="foo\nbar"` + "\n") + expected := []byte(`lvl=eror msg="some message" x=1 y=3.200 equals="=" quote="\"" nilval=nil nil=nil carriage_return="bang\rfoo" tab="bar\tbaz" newline="foo\nbar"` + "\n") if !bytes.Equal(got, expected) { t.Fatalf("Got %s, expected %s", got, expected) }