diff --git a/models/points.go b/models/points.go index 394b8f6997f..336dc4f2836 100644 --- a/models/points.go +++ b/models/points.go @@ -3,6 +3,7 @@ package models import ( "bytes" "encoding/binary" + "errors" "fmt" "hash/fnv" "math" @@ -25,6 +26,8 @@ var ( ' ': []byte(`\ `), '=': []byte(`\=`), } + + ErrPointMustHaveAField = errors.New("point without fields is unsupported") ) // Point defines the values that will be written to the database @@ -37,7 +40,6 @@ type Point interface { SetTags(tags Tags) Fields() Fields - AddField(name string, value interface{}) Time() time.Time SetTime(t time.Time) @@ -888,8 +890,8 @@ func scanTo(buf []byte, i int, stop byte) (int, []byte) { break } - // reached end of block? - if buf[i] == stop && buf[i-1] != '\\' { + // Reached unescaped stop value? + if buf[i] == stop && (i == 0 || buf[i-1] != '\\') { break } i++ @@ -1063,7 +1065,7 @@ func unescapeStringField(in string) string { // an unsupported field value (NaN) or out of range time is passed, this function returns an error. func NewPoint(name string, tags Tags, fields Fields, time time.Time) (Point, error) { if len(fields) == 0 { - return nil, fmt.Errorf("Point without fields is unsupported") + return nil, ErrPointMustHaveAField } if !time.IsZero() { if err := CheckTime(time); err != nil { @@ -1078,6 +1080,9 @@ func NewPoint(name string, tags Tags, fields Fields, time time.Time) (Point, err return nil, fmt.Errorf("NaN is an unsupported value for field %s", key) } } + if len(key) == 0 { + return nil, fmt.Errorf("all fields must have non-empty names") + } } return &point{ @@ -1092,6 +1097,9 @@ func NewPointFromBytes(b []byte) (Point, error) { if err := p.UnmarshalBinary(b); err != nil { return nil, err } + if len(p.Fields()) == 0 { + return nil, ErrPointMustHaveAField + } return p, nil } @@ -1212,14 +1220,6 @@ func (p *point) Fields() Fields { return p.cachedFields } -// AddField adds or replaces a field value for a point -func (p *point) AddField(name string, value interface{}) { - fields := p.Fields() - fields[name] = value - p.fields = fields.MarshalBinary() - p.cachedFields = nil -} - // SetPrecision will round a time to the specified precision func (p *point) SetPrecision(precision string) { switch precision { @@ -1394,38 +1394,37 @@ func newFieldsFromBinary(buf []byte) Fields { } i, name = scanTo(buf, i, '=') - if len(name) == 0 { - continue - } name = escape.Unescape(name) i, valueBuf = scanFieldValue(buf, i+1) - if len(valueBuf) == 0 { - fields[string(name)] = nil - continue - } + if len(name) > 0 { + if len(valueBuf) == 0 { + fields[string(name)] = nil + continue + } - // If the first char is a double-quote, then unmarshal as string - if valueBuf[0] == '"' { - value = unescapeStringField(string(valueBuf[1 : len(valueBuf)-1])) - // Check for numeric characters and special NaN or Inf - } else if (valueBuf[0] >= '0' && valueBuf[0] <= '9') || valueBuf[0] == '-' || valueBuf[0] == '+' || valueBuf[0] == '.' || - valueBuf[0] == 'N' || valueBuf[0] == 'n' || // NaN - valueBuf[0] == 'I' || valueBuf[0] == 'i' { // Inf + // If the first char is a double-quote, then unmarshal as string + if valueBuf[0] == '"' { + value = unescapeStringField(string(valueBuf[1 : len(valueBuf)-1])) + // Check for numeric characters and special NaN or Inf + } else if (valueBuf[0] >= '0' && valueBuf[0] <= '9') || valueBuf[0] == '-' || valueBuf[0] == '+' || valueBuf[0] == '.' || + valueBuf[0] == 'N' || valueBuf[0] == 'n' || // NaN + valueBuf[0] == 'I' || valueBuf[0] == 'i' { // Inf - value, err = parseNumber(valueBuf) - if err != nil { - panic(fmt.Sprintf("unable to parse number value '%v': %v", string(valueBuf), err)) - } + value, err = parseNumber(valueBuf) + if err != nil { + panic(fmt.Sprintf("unable to parse number value '%v': %v", string(valueBuf), err)) + } - // Otherwise parse it as bool - } else { - value, err = strconv.ParseBool(string(valueBuf)) - if err != nil { - panic(fmt.Sprintf("unable to parse bool value '%v': %v\n", string(valueBuf), err)) + // Otherwise parse it as bool + } else { + value, err = strconv.ParseBool(string(valueBuf)) + if err != nil { + panic(fmt.Sprintf("unable to parse bool value '%v': %v\n", string(valueBuf), err)) + } } + fields[string(name)] = value } - fields[string(name)] = value i++ } return fields diff --git a/models/points_test.go b/models/points_test.go index db3065ae338..49f6079bfa5 100644 --- a/models/points_test.go +++ b/models/points_test.go @@ -1772,3 +1772,17 @@ t159,label=another a=2i,value=1i 1` t.Fatalf("expected 2 points, got %d", len(points)) } } + +func TestNewPointsWithBytesWithCorruptData(t *testing.T) { + corrupted := []byte{0, 0, 0, 3, 102, 111, 111, 0, 0, 0, 4, 61, 34, 65, 34, 1, 0, 0, 0, 14, 206, 86, 119, 24, 32, 72, 233, 168, 2, 148} + p, err := models.NewPointFromBytes(corrupted) + if p != nil || err == nil { + t.Fatalf("NewPointFromBytes: got: (%v, %v), expected: (nil, error)", p, err) + } +} + +func TestNewPointsRejectsEmptyFieldNames(t *testing.T) { + if _, err := models.NewPoint("foo", nil, models.Fields{"": 1}, time.Now()); err == nil { + t.Fatalf("new point with empty field name. got: nil, expected: error") + } +}