Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect malformed numbers #6009

Merged
merged 2 commits into from
Mar 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
- [#5963](https://github.com/influxdata/influxdb/pull/5963): Fix possible deadlock
- [#4688](https://github.com/influxdata/influxdb/issues/4688): admin UI doesn't display results for some SHOW queries
- [#6006](https://github.com/influxdata/influxdb/pull/6006): Fix deadlock while running backups
- [#5965](https://github.com/influxdata/influxdb/issues/5965): InfluxDB panic crashes while parsing "-" as Float

## v0.10.2 [2016-03-03]

Expand Down
44 changes: 30 additions & 14 deletions models/points.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var (
}

ErrPointMustHaveAField = errors.New("point without fields is unsupported")
ErrInvalidNumber = errors.New("invalid number")
)

// Point defines the values that will be written to the database
Expand Down Expand Up @@ -664,12 +665,12 @@ func scanNumber(buf []byte, i int) (int, error) {
i++
// There must be more characters now, as just '-' is illegal.
if i == len(buf) {
return i, fmt.Errorf("invalid number")
return i, ErrInvalidNumber
}
}

// how many decimal points we've see
decimals := 0
decimal := false

// indicates the number is float in scientific notation
scientific := false
Expand All @@ -690,12 +691,11 @@ func scanNumber(buf []byte, i int) (int, error) {
}

if buf[i] == '.' {
decimals++
}

// Can't have more than 1 decimal (e.g. 1.1.1 should fail)
if decimals > 1 {
return i, fmt.Errorf("invalid number")
// Can't have more than 1 decimal (e.g. 1.1.1 should fail)
if decimal {
return i, ErrInvalidNumber
}
decimal = true
}

// `e` is valid for floats but not as the first char
Expand All @@ -713,16 +713,32 @@ func scanNumber(buf []byte, i int) (int, error) {

// NaN is an unsupported value
if i+2 < len(buf) && (buf[i] == 'N' || buf[i] == 'n') {
return i, fmt.Errorf("invalid number")
return i, ErrInvalidNumber
}

if !isNumeric(buf[i]) {
return i, fmt.Errorf("invalid number")
return i, ErrInvalidNumber
}
i++
}
if isInt && (decimals > 0 || scientific) {
return i, fmt.Errorf("invalid number")

if isInt && (decimal || scientific) {
return i, ErrInvalidNumber
}

numericDigits := i - start
if isInt {
numericDigits--
}
if decimal {
numericDigits--
}
if buf[start] == '-' {
numericDigits--
}

if numericDigits == 0 {
return i, ErrInvalidNumber
}

// It's more common that numbers will be within min/max range for their type but we need to prevent
Expand All @@ -732,7 +748,7 @@ func scanNumber(buf []byte, i int) (int, error) {
if isInt {
// Make sure the last char is an 'i' for integers (e.g. 9i10 is not valid)
if buf[i-1] != 'i' {
return i, fmt.Errorf("invalid number")
return i, ErrInvalidNumber
}
// Parse the int to check bounds the number of digits could be larger than the max range
// We subtract 1 from the index to remove the `i` from our tests
Expand Down Expand Up @@ -1417,7 +1433,7 @@ func newFieldsFromBinary(buf []byte) Fields {
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] == '.' ||
} else if (valueBuf[0] >= '0' && valueBuf[0] <= '9') || valueBuf[0] == '-' || valueBuf[0] == '.' ||
valueBuf[0] == 'N' || valueBuf[0] == 'n' || // NaN
valueBuf[0] == 'I' || valueBuf[0] == 'i' { // Inf

Expand Down
27 changes: 16 additions & 11 deletions models/points_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,17 +362,22 @@ func TestParsePointMissingFieldValue(t *testing.T) {
}

func TestParsePointBadNumber(t *testing.T) {
_, err := models.ParsePointsString(`cpu,host=serverA,region=us-west value=1a`)
if err == nil {
t.Errorf(`ParsePoints("%s") mismatch. got nil, exp error`, `cpu,host=serverA,region=us-west value=1a`)
}
_, err = models.ParsePointsString(`cpu,host=serverA,region=us-west value=1ii`)
if err == nil {
t.Errorf(`ParsePoints("%s") mismatch. got nil, exp error`, `cpu,host=serverA,region=us-west value=1ii`)
}
_, err = models.ParsePointsString(`cpu,host=serverA,region=us-west value=1.0i`)
if err == nil {
t.Errorf(`ParsePoints("%s") mismatch. got nil, exp error`, `cpu,host=serverA,region=us-west value=1.0i`)
for _, tt := range []string{
"cpu v=- ",
"cpu v=-i ",
"cpu v=-. ",
"cpu v=. ",
"cpu v=1.0i ",
"cpu v=1ii ",
"cpu v=1a ",
"cpu v=-e-e-e ",
"cpu v=42+3 ",
"cpu v= ",
} {
_, err := models.ParsePointsString(tt)
if err == nil {
t.Errorf("Point %q should be invalid", tt)
}
}
}

Expand Down