Fix bug in logql parsing that leads to crash.

``` panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x20ae356] goroutine 39530021 [running]: github.com/grafana/loki/pkg/logql.andFilter.Filter(0x0, 0x0, 0x3beffc0, 0xc01f065bc0, 0xc01aaaec00, 0x13d, 0x200, 0xffffffffffffffff) /src/loki/pkg/logql/filter.go:66 +0x26 github.com/grafana/loki/pkg/chunkenc.(*bufferedIterator).Next(0xc00a48fba0, 0x0) /src/loki/pkg/chunkenc/memchunk.go:616 +0x1cb github.com/grafana/loki/pkg/iter.(*nonOverlappingIterator).Next(0xc00130ae40, 0x0) /src/loki/pkg/iter/iterator.go:438 +0x135 github.com/grafana/loki/pkg/iter.(*timeRangedIterator).Next(0xc00130ae80, 0x0) /src/loki/pkg/iter/iterator.go:497 +0x48 github.com/grafana/loki/pkg/iter.(*reverseIterator).load(0xc00072ac40) /src/loki/pkg/iter/iterator.go:555 +0x70 github.com/grafana/loki/pkg/iter.(*reverseIterator).Next(0xc00072ac40, 0x0) /src/loki/pkg/iter/iterator.go:563 +0x2f github.com/grafana/loki/pkg/iter.(*nonOverlappingIterator).Next(0xc00130af40, 0x10) /src/loki/pkg/iter/iterator.go:438 +0x135 ``` This is caused by people doing `|= ""` which would result in parsing of TrueFilter but would be returned as nil. Signed-off-by: Goutham Veeramachaneni <[email protected]>
grafana · May 6, 2020 · 18270fd · 18270fd
1 parent 2629e9a
commit 18270fd
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 7 deletions.
diff --git a/pkg/logql/ast.go b/pkg/logql/ast.go
@@ -131,11 +131,15 @@ func (e *filterExpr) Filter() (LineFilter, error) {
 		if err != nil {
 			return nil, err
 		}
-		f = newAndFilter(nextFilter, f)
+		if nextFilter != nil {
+			f = newAndFilter(nextFilter, f)
+		}
 	}
+
 	if f == TrueFilter {
 		return nil, nil
 	}
+
 	return f, nil
 }
 

diff --git a/pkg/logql/filter.go b/pkg/logql/filter.go
@@ -53,9 +53,15 @@ type andFilter struct {
 
 // newAndFilter creates a new filter which matches only if left and right matches.
 func newAndFilter(left LineFilter, right LineFilter) LineFilter {
-	if (right == TrueFilter || right == nil) && (left == TrueFilter || left == nil) {
-		return TrueFilter
+	// Make sure we take care of panics in case a nil or noop filter is passed.
+	if right == nil || right == TrueFilter {
+		return left
+	}
+
+	if left == nil || left == TrueFilter {
+		return right
 	}
+
 	return andFilter{
 		left:  left,
 		right: right,
@@ -73,9 +79,14 @@ type orFilter struct {
 
 // newOrFilter creates a new filter which matches only if left or right matches.
 func newOrFilter(left LineFilter, right LineFilter) LineFilter {
-	if (right == TrueFilter || right == nil) && (left == TrueFilter || left == nil) {
-		return TrueFilter
+	if left == nil || left == TrueFilter {
+		return right
+	}
+
+	if right == nil || right == TrueFilter {
+		return left
 	}
+
 	return orFilter{
 		left:  left,
 		right: right,

diff --git a/pkg/logql/filter_test.go b/pkg/logql/filter_test.go
@@ -109,8 +109,8 @@ func Test_TrueFilter(t *testing.T) {
 		{"nil left or", newOrFilter(nil, newContainsFilter(empty, false)), true},
 		{"nil right and not empty", newAndFilter(newContainsFilter([]byte("foo"), false), nil), false},
 		{"nil left or not empty", newOrFilter(nil, newContainsFilter([]byte("foo"), false)), false},
-		{"nil both and", newAndFilter(nil, nil), true},
-		{"nil both or", newOrFilter(nil, nil), true},
+		{"nil both and", newAndFilter(nil, nil), false}, // returns nil
+		{"nil both or", newOrFilter(nil, nil), false},   // returns nil
 		{"empty match and chained", newAndFilter(newContainsFilter(empty, false), newAndFilter(newContainsFilter(empty, false), newAndFilter(newContainsFilter(empty, false), newContainsFilter(empty, false)))), true},
 		{"empty match or chained", newOrFilter(newContainsFilter(empty, false), newOrFilter(newContainsFilter(empty, true), newOrFilter(newContainsFilter(empty, false), newContainsFilter(empty, false)))), true},
 		{"empty match and", newNotFilter(newAndFilter(newContainsFilter(empty, false), newContainsFilter(empty, false))), false},