Merge pull request #5716 from jonseymour/js-tolerate-empty-field-names

models: tolerate empty field names when unpacking binary points

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]
 

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

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")
+	}
+}