diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b9c4a4916..d5bd0122e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ - [#5656](https://github.com/influxdata/influxdb/issues/5656): influx\_tsm: panic during conversion - [#5696](https://github.com/influxdata/influxdb/issues/5696): Do not drop the database when creating with a retention policy - [#5724](https://github.com/influxdata/influxdb/issues/5724): influx\_tsm doesn't close file handles properly +- [#5664](https://github.com/influxdata/influxdb/issues/5664): panic in model.Points.scanTo #5664 +- [#5716](https://github.com/influxdata/influxdb/pull/5716): models: improve handling of points with empty field names or with no fields. ## v0.10.1 [2016-02-18] diff --git a/models/points.go b/models/points.go index d9da3f2555..139e1cb322 100644 --- a/models/points.go +++ b/models/points.go @@ -3,6 +3,7 @@ package models // import "github.com/influxdata/influxdb/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) @@ -889,8 +891,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++ @@ -1064,7 +1066,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 { @@ -1079,6 +1081,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{ @@ -1094,6 +1099,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 } @@ -1214,14 +1222,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 { @@ -1404,38 +1404,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 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)) + if len(name) > 0 { + if len(valueBuf) == 0 { + fields[string(name)] = nil + continue } - // 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)) + // 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)) + } + + // 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 99161780db..052e9777f4 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") + } +}