From cf3d74ed69016b789775dbd27e9073cc6f43ab4a Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Thu, 10 Oct 2019 16:09:08 +0200 Subject: [PATCH] promql: fix potential panic in the query logger (#6094) Signed-off-by: Simon Pasquier --- promql/query_logger.go | 10 ++++++---- promql/query_logger_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/promql/query_logger.go b/promql/query_logger.go index 256c3d775c..1014ade407 100644 --- a/promql/query_logger.go +++ b/promql/query_logger.go @@ -41,12 +41,14 @@ const ( entrySize int = 1000 ) -func parseBrokenJson(brokenJson []byte, logger log.Logger) (bool, string) { +func parseBrokenJson(brokenJson []byte) (bool, string) { queries := strings.ReplaceAll(string(brokenJson), "\x00", "") - queries = queries[:len(queries)-1] + "]" + if len(queries) > 0 { + queries = queries[:len(queries)-1] + "]" + } // Conditional because of implementation detail: len() = 1 implies file consisted of a single char: '['. - if len(queries) == 1 { + if len(queries) <= 1 { return false, "[]" } @@ -68,7 +70,7 @@ func logUnfinishedQueries(filename string, filesize int, logger log.Logger) { return } - queriesExist, queries := parseBrokenJson(brokenJson, logger) + queriesExist, queries := parseBrokenJson(brokenJson) if !queriesExist { return } diff --git a/promql/query_logger_test.go b/promql/query_logger_test.go index 27e5b66446..164c09c0e9 100644 --- a/promql/query_logger_test.go +++ b/promql/query_logger_test.go @@ -136,3 +136,43 @@ func TestMMapFile(t *testing.T) { t.Fatalf("Mmap failed") } } + +func TestParseBrokenJson(t *testing.T) { + for _, tc := range []struct { + b []byte + + ok bool + out string + }{ + { + b: []byte(""), + }, + { + b: []byte("\x00\x00"), + }, + { + b: []byte("\x00[\x00"), + }, + { + b: []byte("\x00[]\x00"), + ok: true, + out: "[]", + }, + { + b: []byte("[\"up == 0\",\"rate(http_requests[2w]\"]\x00\x00\x00"), + ok: true, + out: "[\"up == 0\",\"rate(http_requests[2w]\"]", + }, + } { + t.Run("", func(t *testing.T) { + ok, out := parseBrokenJson(tc.b) + if tc.ok != ok { + t.Fatalf("expected %t, got %t", tc.ok, ok) + return + } + if ok && tc.out != out { + t.Fatalf("expected %s, got %s", tc.out, out) + } + }) + } +}