Fix panic when handling string fields with escapes (#3188)

This commit is contained in:
Daniel Nelson 2017-08-30 21:16:37 -07:00 committed by GitHub
parent 19f3264073
commit 28d16188b3
4 changed files with 56 additions and 19 deletions

View File

@ -150,12 +150,6 @@ func makemetric(
continue continue
} }
case string: case string:
if strings.HasSuffix(val, `\`) {
log.Printf("D! Measurement [%s] field [%s] has a value "+
"ending with a backslash, skipping", measurement, k)
delete(fields, k)
continue
}
fields[k] = v fields[k] = v
default: default:
fields[k] = v fields[k] = v

View File

@ -370,16 +370,17 @@ func TestMakeMetric_TrailingSlash(t *testing.T) {
expectedTags: map[string]string{}, expectedTags: map[string]string{},
}, },
{ {
name: "Field value with trailing slash dropped", name: "Field value with trailing slash okay",
measurement: `cpu`, measurement: `cpu`,
fields: map[string]interface{}{ fields: map[string]interface{}{
"value": int64(42), "value": int64(42),
"bad": `xyzzy\`, "ok": `xyzzy\`,
}, },
tags: map[string]string{}, tags: map[string]string{},
expectedMeasurement: `cpu`, expectedMeasurement: `cpu`,
expectedFields: map[string]interface{}{ expectedFields: map[string]interface{}{
"value": int64(42), "value": int64(42),
"ok": `xyzzy\`,
}, },
expectedTags: map[string]string{}, expectedTags: map[string]string{},
}, },
@ -387,7 +388,7 @@ func TestMakeMetric_TrailingSlash(t *testing.T) {
name: "Must have one field after dropped", name: "Must have one field after dropped",
measurement: `cpu`, measurement: `cpu`,
fields: map[string]interface{}{ fields: map[string]interface{}{
"bad": `xyzzy\`, "bad": math.NaN(),
}, },
tags: map[string]string{}, tags: map[string]string{},
expectedNil: true, expectedNil: true,

View File

@ -21,14 +21,14 @@ func New(
t time.Time, t time.Time,
mType ...telegraf.ValueType, mType ...telegraf.ValueType,
) (telegraf.Metric, error) { ) (telegraf.Metric, error) {
if len(fields) == 0 {
return nil, fmt.Errorf("Metric cannot be made without any fields")
}
if len(name) == 0 { if len(name) == 0 {
return nil, fmt.Errorf("Metric cannot be made with an empty name") return nil, fmt.Errorf("missing measurement name")
}
if len(fields) == 0 {
return nil, fmt.Errorf("%s: must have one or more fields", name)
} }
if strings.HasSuffix(name, `\`) { if strings.HasSuffix(name, `\`) {
return nil, fmt.Errorf("Metric cannot have measurement name ending with a backslash") return nil, fmt.Errorf("%s: measurement name cannot end with a backslash", name)
} }
var thisType telegraf.ValueType var thisType telegraf.ValueType
@ -49,10 +49,10 @@ func New(
taglen := 0 taglen := 0
for k, v := range tags { for k, v := range tags {
if strings.HasSuffix(k, `\`) { if strings.HasSuffix(k, `\`) {
return nil, fmt.Errorf("Metric cannot have tag key ending with a backslash") return nil, fmt.Errorf("%s: tag key cannot end with a backslash: %s", name, k)
} }
if strings.HasSuffix(v, `\`) { if strings.HasSuffix(v, `\`) {
return nil, fmt.Errorf("Metric cannot have tag value ending with a backslash") return nil, fmt.Errorf("%s: tag value cannot end with a backslash: %s", name, v)
} }
if len(k) == 0 || len(v) == 0 { if len(k) == 0 || len(v) == 0 {
@ -79,7 +79,7 @@ func New(
fieldlen := 0 fieldlen := 0
for k, _ := range fields { for k, _ := range fields {
if strings.HasSuffix(k, `\`) { if strings.HasSuffix(k, `\`) {
return nil, fmt.Errorf("Metric cannot have field key ending with a backslash") return nil, fmt.Errorf("%s: field key cannot end with a backslash: %s", name, k)
} }
// 10 bytes is completely arbitrary, but will at least prevent some // 10 bytes is completely arbitrary, but will at least prevent some
@ -102,7 +102,8 @@ func New(
} }
// indexUnescapedByte finds the index of the first byte equal to b in buf that // indexUnescapedByte finds the index of the first byte equal to b in buf that
// is not escaped. Returns -1 if not found. // is not escaped. Does not allow the escape char to be escaped. Returns -1 if
// not found.
func indexUnescapedByte(buf []byte, b byte) int { func indexUnescapedByte(buf []byte, b byte) int {
var keyi int var keyi int
for { for {
@ -122,6 +123,46 @@ func indexUnescapedByte(buf []byte, b byte) int {
return keyi return keyi
} }
// indexUnescapedByteBackslashEscaping finds the index of the first byte equal
// to b in buf that is not escaped. Allows for the escape char `\` to be
// escaped. Returns -1 if not found.
func indexUnescapedByteBackslashEscaping(buf []byte, b byte) int {
var keyi int
for {
i := bytes.IndexByte(buf[keyi:], b)
if i == -1 {
return -1
} else if i == 0 {
break
}
keyi += i
if countBackslashes(buf, keyi-1)%2 == 0 {
break
} else {
keyi++
}
}
return keyi
}
// countBackslashes counts the number of preceding backslashes starting at
// the 'start' index.
func countBackslashes(buf []byte, index int) int {
var count int
for {
if index < 0 {
return count
}
if buf[index] == '\\' {
count++
index--
} else {
break
}
}
return count
}
type metric struct { type metric struct {
name []byte name []byte
tags []byte tags []byte
@ -283,7 +324,7 @@ func (m *metric) Fields() map[string]interface{} {
// end index of field value // end index of field value
var i3 int var i3 int
if m.fields[i:][i2] == '"' { if m.fields[i:][i2] == '"' {
i3 = indexUnescapedByte(m.fields[i:][i2+1:], '"') i3 = indexUnescapedByteBackslashEscaping(m.fields[i:][i2+1:], '"')
if i3 == -1 { if i3 == -1 {
i3 = len(m.fields[i:]) i3 = len(m.fields[i:])
} }

View File

@ -258,6 +258,7 @@ func TestNewMetric_Fields(t *testing.T) {
"quote_string": `x"y`, "quote_string": `x"y`,
"backslash_quote_string": `x\"y`, "backslash_quote_string": `x\"y`,
"backslash": `x\y`, "backslash": `x\y`,
"ends_with_backslash": `x\`,
} }
m, err := New("cpu", tags, fields, now) m, err := New("cpu", tags, fields, now)
assert.NoError(t, err) assert.NoError(t, err)