From c20e25f71814924f2820c42a6333e78e9765bdef Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Mon, 3 Aug 2015 12:28:40 +0200 Subject: [PATCH 1/2] Add missing check for nil expression --- promql/parse.go | 10 ++++------ promql/parse_test.go | 25 +++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/promql/parse.go b/promql/parse.go index a51e0c550..aac36cedf 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -464,9 +464,6 @@ func (p *parser) recordStmt() *RecordStmt { func (p *parser) expr() Expr { // Parse the starting expression. expr := p.unaryExpr() - if expr == nil { - p.errorf("no valid expression found") - } // Loop through the operations and construct a binary operation tree based // on the operators' precedence. @@ -514,9 +511,6 @@ func (p *parser) expr() Expr { // Parse the next operand. rhs := p.unaryExpr() - if rhs == nil { - p.errorf("missing right-hand side in binary expression") - } // Assign the new root based on the precendence of the LHS and RHS operators. if lhs, ok := expr.(*BinaryExpr); ok && lhs.Op.precedence() < op.precedence() { @@ -552,6 +546,7 @@ func (p *parser) unaryExpr() Expr { case itemADD, itemSUB: p.next() e := p.unaryExpr() + // Simplify unary expressions for number literals. if nl, ok := e.(*NumberLiteral); ok { if t.typ == itemSUB { @@ -665,6 +660,9 @@ func (p *parser) primaryExpr() Expr { case t.typ.isAggregator(): p.backup() return p.aggrExpr() + + default: + p.errorf("no valid expression found") } return nil } diff --git a/promql/parse_test.go b/promql/parse_test.go index 341fe8e99..7f938c3dc 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -134,7 +134,7 @@ var testExpr = []struct { }, { input: "1+", fail: true, - errMsg: "missing right-hand side in binary expression", + errMsg: "no valid expression found", }, { input: ".", fail: true, @@ -154,7 +154,7 @@ var testExpr = []struct { }, { input: "1 /", fail: true, - errMsg: "missing right-hand side in binary expression", + errMsg: "no valid expression found", }, { input: "*1", fail: true, @@ -945,6 +945,27 @@ var testExpr = []struct { fail: true, errMsg: "expected type matrix in call to function \"rate\", got vector", }, + // Fuzzing regression tests. + { + input: "-=", + fail: true, + errMsg: `no valid expression found`, + }, + { + input: "++-++-+-+-<", + fail: true, + errMsg: `no valid expression found`, + }, + { + input: "e-+=/(0)", + fail: true, + errMsg: `no valid expression found`, + }, + { + input: "-If", + fail: true, + errMsg: `no valid expression found`, + }, } func TestParseExpressions(t *testing.T) { From adf109795c175251fcc7a32667378e0246da1862 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Mon, 3 Aug 2015 12:53:31 +0200 Subject: [PATCH 2/2] forbid unexpected (runtime) errors in parse tests --- promql/parse.go | 4 +++- promql/parse_test.go | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/promql/parse.go b/promql/parse.go index aac36cedf..c1bc1fd7d 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -321,6 +321,8 @@ func (p *parser) expectOneOf(exp1, exp2 itemType, context string) item { return token } +var errUnexpected = fmt.Errorf("unexpected error") + // recover is the handler that turns panics into returns from the top level of Parse. func (p *parser) recover(errp *error) { e := recover() @@ -331,7 +333,7 @@ func (p *parser) recover(errp *error) { buf = buf[:runtime.Stack(buf, false)] log.Errorf("parser panic: %v\n%s", e, buf) - *errp = fmt.Errorf("unexpected error") + *errp = errUnexpected } else { *errp = e.(error) } diff --git a/promql/parse_test.go b/promql/parse_test.go index 7f938c3dc..3fc1df873 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -973,6 +973,12 @@ func TestParseExpressions(t *testing.T) { parser := newParser(test.input) expr, err := parser.parseExpr() + + // Unexpected errors are always caused by a bug. + if err == errUnexpected { + t.Fatalf("unexpected error occurred") + } + if !test.fail && err != nil { t.Errorf("error in input '%s'", test.input) t.Fatalf("could not parse: %s", err) @@ -1219,6 +1225,19 @@ var testStatement = []struct { `, fail: true, }, + // Fuzzing regression tests. + { + input: `I=-/`, + fail: true, + }, + { + input: `I=3E8/-=`, + fail: true, + }, + { + input: `M=-=-0-0`, + fail: true, + }, } func TestParseStatements(t *testing.T) { @@ -1226,6 +1245,12 @@ func TestParseStatements(t *testing.T) { parser := newParser(test.input) stmts, err := parser.parseStmts() + + // Unexpected errors are always caused by a bug. + if err == errUnexpected { + t.Fatalf("unexpected error occurred") + } + if !test.fail && err != nil { t.Errorf("error in input: \n\n%s\n", test.input) t.Fatalf("could not parse: %s", err) @@ -1353,6 +1378,12 @@ func TestParseSeries(t *testing.T) { parser.lex.seriesDesc = true metric, vals, err := parser.parseSeriesDesc() + + // Unexpected errors are always caused by a bug. + if err == errUnexpected { + t.Fatalf("unexpected error occurred") + } + if !test.fail && err != nil { t.Errorf("error in input: \n\n%s\n", test.input) t.Fatalf("could not parse: %s", err) @@ -1385,8 +1416,8 @@ func TestRecoverRuntime(t *testing.T) { var a []int a[123] = 1 - if err.Error() != "unexpected error" { - t.Fatalf("wrong error message: %q, expected %q", err, "unexpected error") + if err != errUnexpected { + t.Fatalf("wrong error message: %q, expected %q", err, errUnexpected) } }