Skip to content

Commit

Permalink
Fail metrics parsing on unescaped quotes (#3409)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
faye-sama authored and danielnelson committed Nov 13, 2017
1 parent 176064c commit ccd2175
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
20 changes: 13 additions & 7 deletions metric/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
14 changes: 14 additions & 0 deletions plugins/parsers/influx/parser_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package influx

import (
"fmt"
"io/ioutil"
"testing"
"time"
Expand All @@ -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 = `
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit ccd2175

Please sign in to comment.