From a411306fba0dc87778e64392b57c844e0477df2b Mon Sep 17 00:00:00 2001 From: faye-sama <32477052+faye-sama@users.noreply.github.com> Date: Mon, 13 Nov 2017 23:06:47 +0000 Subject: [PATCH] Fail metrics parsing on unescaped quotes (#3409) Before this change Fields() method on a metric parsed from a line with unescaped quotes could panic. This change makes such line unparseable. Fixes #3326 --- metric/parse.go | 20 +++++++++++++------- plugins/parsers/influx/parser_test.go | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/metric/parse.go b/metric/parse.go index 1acf30094..80fe2ef3f 100644 --- a/metric/parse.go +++ b/metric/parse.go @@ -326,7 +326,9 @@ func scanTagsValue(buf []byte, i int) (int, int, error) { func scanFields(buf []byte, i int) (int, []byte, error) { start := skipWhitespace(buf, i) i = start - quoted := false + + // track how many '"" we've seen since last '=' + quotes := 0 // tracks how many '=' we've seen equals := 0 @@ -350,13 +352,17 @@ func scanFields(buf []byte, i int) (int, []byte, error) { // Only quote values in the field value since quotes are not significant // in the field key if buf[i] == '"' && equals > commas { - quoted = !quoted i++ + quotes++ + if quotes > 2 { + break + } continue } // If we see an =, ensure that there is at least on char before and after it - if buf[i] == '=' && !quoted { + if buf[i] == '=' && quotes != 1 { + quotes = 0 equals++ // check for "... =123" but allow "a\ =123" @@ -398,19 +404,19 @@ func scanFields(buf []byte, i int) (int, []byte, error) { } } - if buf[i] == ',' && !quoted { + if buf[i] == ',' && quotes != 1 { commas++ } // reached end of block? - if buf[i] == ' ' && !quoted { + if buf[i] == ' ' && quotes != 1 { break } i++ } - if quoted { - return i, buf[start:i], makeError("unbalanced quotes", buf, i) + if quotes != 0 && quotes != 2 { + return i, buf[start:i], makeError("unescaped/ublanaced quotes", buf, i) } // check that all field sections had key and values (e.g. prevent "a=1,b" diff --git a/plugins/parsers/influx/parser_test.go b/plugins/parsers/influx/parser_test.go index 6b2ba8d56..67833fc56 100644 --- a/plugins/parsers/influx/parser_test.go +++ b/plugins/parsers/influx/parser_test.go @@ -1,6 +1,7 @@ package influx import ( + "fmt" "io/ioutil" "testing" "time" @@ -24,6 +25,8 @@ const ( validInfluxNoNewline = "cpu_load_short,cpu=cpu0 value=10 1257894000000000000" invalidInflux = "I don't think this is line protocol\n" invalidInflux2 = "{\"a\": 5, \"b\": {\"c\": 6}}\n" + invalidInflux3 = `name text="unescaped "quote" ",value=1 1498077493081000000` + invalidInflux4 = `name text="unbalanced "quote" 1498077493081000000` ) const influxMulti = ` @@ -221,10 +224,21 @@ func TestParseInvalidInflux(t *testing.T) { assert.Error(t, err) _, err = parser.Parse([]byte(invalidInflux2)) assert.Error(t, err) + _, err = parser.Parse([]byte(invalidInflux3)) + assert.Error(t, err) + fmt.Printf("%+v\n", err) // output for debug + _, err = parser.Parse([]byte(invalidInflux4)) + assert.Error(t, err) + _, err = parser.ParseLine(invalidInflux) assert.Error(t, err) _, err = parser.ParseLine(invalidInflux2) assert.Error(t, err) + _, err = parser.ParseLine(invalidInflux3) + assert.Error(t, err) + _, err = parser.ParseLine(invalidInflux4) + assert.Error(t, err) + } func BenchmarkParse(b *testing.B) {