diff --git a/models/points.go b/models/points.go index 6df1b22184c..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) @@ -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 b8afae9ee5c..49f6079bfa5 100644 --- a/models/points_test.go +++ b/models/points_test.go @@ -1774,20 +1774,10 @@ t159,label=another a=2i,value=1i 1` } func TestNewPointsWithBytesWithCorruptData(t *testing.T) { - ch := make(chan error) - go func() { - 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) - p.Fields() // this method should always return, even if the data is corrupt. - ch <- err - }() - select { - case err := <-ch: - if err != nil { - t.Fatalf("unexpected error: got: %v, expected: nil", err) - } - case _ = <-time.NewTimer(time.Second).C: - t.Fatalf("probable infite loop. got: timeout, expected: return") + 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) } }