Add validation for brackets in plain metrics and regex error parsing …

…for tagged metrics (go-graphite#284)

finder/index.go

@@ -4,11 +4,13 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"net/http"
 	"strings"
 
 	"github.com/lomik/graphite-clickhouse/config"
 	"github.com/lomik/graphite-clickhouse/helper/clickhouse"
 	"github.com/lomik/graphite-clickhouse/helper/date"
+	"github.com/lomik/graphite-clickhouse/helper/errs"
 	"github.com/lomik/graphite-clickhouse/pkg/scope"
 	"github.com/lomik/graphite-clickhouse/pkg/where"
 )
@@ -153,7 +155,18 @@ func (idx *IndexFinder) whereFilter(query string, from int64, until int64) *wher
 	return w
 }
 
+func (idx *IndexFinder) validatePlainQuery(query string) error {
+	if where.HasUnmatchedBrackets(query) {
+		return errs.NewErrorWithCode("query has unmatched brackets", http.StatusBadRequest)
+	}
+	return nil
+}
+
 func (idx *IndexFinder) Execute(ctx context.Context, config *config.Config, query string, from int64, until int64, stat *FinderStat) (err error) {
+	err = idx.validatePlainQuery(query)
+	if err != nil {
+		return err
+	}
 	w := idx.whereFilter(query, from, until)
 
 	idx.body, stat.ChReadRows, stat.ChReadBytes, err = clickhouse.Query(

finder/tagged.go

@@ -19,7 +19,6 @@ import (
 )
 
 var (
-	// ErrEmptyArgs         = errors.New("empty arguments")
 	ErrCostlySeriesByTag = errs.NewErrorWithCode("seriesByTag argument has too much wildcard and regex terms", http.StatusForbidden)
 )
 

helper/clickhouse/clickhouse.go

@@ -75,6 +75,9 @@ func extractClickhouseError(e string) (int, string) {
 	if strings.HasPrefix(e, "clickhouse response status 404: Code: 60. DB::Exception: Table default.") {
 		return http.StatusServiceUnavailable, "Storage default tables damaged"
 	}
+	if strings.HasPrefix(e, "clickhouse response status 500: Code: 427") || strings.HasPrefix(e, "clickhouse response status 400: Code: 427.") {
+		return http.StatusBadRequest, "Incorrect regex syntax"
+	}
 	return http.StatusServiceUnavailable, "Storage unavailable"
 }
 

helper/clickhouse/clickhouse_test.go

@@ -39,6 +39,16 @@ func Test_extractClickhouseError(t *testing.T) {
 			wantStatus:  http.StatusServiceUnavailable,
 			wantMessage: "Storage default tables damaged",
 		},
+		{
+			errStr:      `clickhouse response status 500: Code: 427, e.displayText() = DB::Exception: OptimizedRegularExpression: cannot compile re2: ^t=.**a*, error: bad repetition operator: **. Look at https://github.com/google/re2/wiki/Syntax for reference. Please note that if you specify regex as an SQL string literal, the slashes have to be additionally escaped. For example, to match an opening brace, write '\(' -- the first slash is for SQL and the second one is for regex: while executing 'FUNCTION match(x :: 0, '^t=.**a*' :: 2) -> match(x, '^t=.**a*') UInt8 : 1': while executing 'FUNCTION arrayExists(__lambda_11 :: 7, Tags :: 3) -> arrayExists(lambda(tuple(x), and(like(x, 't=%'), match(x, '^t=.**a*'))), Tags) UInt8 : 6' (version 21.3.20.1 (official build))`,
+			wantStatus:  http.StatusBadRequest,
+			wantMessage: "Incorrect regex syntax",
+		},
+		{
+			errStr:      `clickhouse response status 500: Code: 427. DB::Exception: OptimizedRegularExpression: cannot compile re2: ^t=.**a*, error: bad repetition operator: **. Look at https://github.com/google/re2/wiki/Syntax for reference. Please note that if you specify regex as an SQL string literal, the slashes have to be additionally escaped. For example, to match an opening brace, write '\(' -- the first slash is for SQL and the second one is for regex: while executing 'FUNCTION and(like(x, 't=%') :: 3, match(x, '^t=.**a*') :: 1) -> and(like(x, 't=%'), match(x, '^t=.**a*')) UInt8 : 2': while executing 'FUNCTION and(and(greaterOrEquals(Date, '2024-10-28'), lessOrEquals(Date, '2024-10-28')) :: 0, and(equals(Tag1, '__name__=request_success_total.counter'), arrayExists(lambda(tuple(x), equals(x, 'app=test')), Tags), arrayExists(lambda(tuple(x), equals(x, 'project=Test')), Tags), arrayExists(lambda(tuple(x), equals(x, 'environment=TEST')), Tags), arrayExists(lambda(tuple(x), and(like(x, 't=%'), match(x, '^t=.**a*'))), Tags)) :: 3) -> and(and(greaterOrEquals(Date, '2024-10-28'), lessOrEquals(Date, '2024-10-28')), and(equals(Tag1, '__name__=request_success_total.counter'), arrayExists(lambda(tuple(x), equals(x, 'app=test')), Tags), arrayExists(lambda(tuple(x), equals(x, 'project=Test')), Tags), arrayExists(lambda(tuple(x), equals(x, 'environment=TEST')), Tags), arrayExists(lambda(tuple(x), and(like(x, 't=%'), match(x, '^t=.**a*'))), Tags))) UInt8 : 6'. (CANNOT_COMPILE_REGEXP) (version 22.8.21.38 (official build))`,
+			wantStatus:  http.StatusBadRequest,
+			wantMessage: "Incorrect regex syntax",
+		},
 		{
 			errStr:      "Other error",
 			wantStatus:  http.StatusServiceUnavailable,

pkg/where/match.go

@@ -85,6 +85,37 @@ func clearGlob(query string) string {
 	return query
 }
 
+func HasUnmatchedBrackets(query string) bool {
+	matchingBracket := map[rune]rune{
+		'}': '{',
+		']': '[',
+	}
+	stack := make([]rune, 0)
+
+	nodeHasUnmatched := func(query string) bool {
+		for _, c := range query {
+			if c == '{' || c == '[' {
+				stack = append(stack, c)
+			}
+			if c == '}' || c == ']' {
+				if len(stack) == 0 || stack[len(stack)-1] != matchingBracket[c] {
+					return true
+				}
+				stack = stack[:len(stack)-1]
+			}
+		}
+		return len(stack) != 0
+	}
+
+	for _, node := range strings.Split(query, ".") {
+		if nodeHasUnmatched(node) {
+			return true
+		}
+	}
+
+	return false
+}
+
 func glob(field string, query string, optionalDotAtEnd bool) string {
 	if query == "*" {
 		return ""

pkg/where/match_test.go

@@ -28,6 +28,36 @@ func Test_clearGlob(t *testing.T) {
 	}
 }
 
+func Test_HasUnmatchedBrackets(t *testing.T) {
+	tests := []struct {
+		query string
+		want  bool
+	}{
+		{"a.{a,b.te{s}t.b", true},
+		{"a.{a,b}.te{s}t.b", false},
+		{"a.{a,b}.te{s,t}}*.b", true},
+		{"a.{a,b}.test*.b", false},
+		{"a.a,b}.test*.b", true},
+		{"a.{a,b.test*.b}", true},
+		{"a.[a,b.test*.b]", true},
+		{"a.[a,b].test*.b", false},
+		{"a.[b].te{s}t.b", false},
+		{"a.{[cd],[ef]}.b", false},
+		{"a.[ab].te{s,t}*.b", false},
+		{"a.{a,b.}.te{s,t}*.b", true}, // dots are not escaped inside curly brackets
+		{"О.[б].те{s}t.b", false},     // utf-8 string
+		{"О.[б.теs}t.b", true},
+		{"О.[].те{}t.b", false}, // utf-8 string with empthy blocks
+	}
+	for _, tt := range tests {
+		t.Run(tt.query, func(t *testing.T) {
+			if got := HasUnmatchedBrackets(tt.query); got != tt.want {
+				t.Errorf("HasUnmatchedBrackets() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestGlob(t *testing.T) {
 	field := "test"
 	tests := []struct {

tests/feature_flags_both_true/test.toml

@@ -37,10 +37,6 @@ points = [{value = 1.0, time = "rnow-10"}]
 name = "request_success_total.counter;app=test;project=Test;environment=TEST;t=cqa"
 points = [{value = 1.0, time = "rnow-10"}]
 
-[[test.input]]
-name = "request_success_total.counter;app=test;project=Test;environment=TEST;t=cqa"
-points = [{value = 1.0, time = "rnow-10"}]
-
 [[test.input]]
 name = "test;env=prod"
 points = [{value = 1.0, time = "rnow-10"}]
@@ -974,6 +970,60 @@ req_start = "rnow-10"
 req_stop = "rnow+10"
 values = [1.0, nan]
 
+# ### seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*a*') ###
+
+[[test.render_checks]]
+from = "rnow-10"
+until = "rnow+1"
+timeout = "1h"
+targets = [ 
+    "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*a*')", 
+]
+error_regexp = "^400: Incorrect regex syntax"
+
+# ### seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*') ###
+
+[[test.render_checks]]
+from = "rnow-10"
+until = "rnow+1"
+timeout = "1h"
+targets = [ 
+    "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')", 
+]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=cqa"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=q"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=qac"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
 # # End - Test no flags
 # #########################################################################
 

tests/feature_flags_dont_match_missing_tags/test.toml

@@ -37,10 +37,6 @@ points = [{value = 1.0, time = "rnow-10"}]
 name = "request_success_total.counter;app=test;project=Test;environment=TEST;t=cqa"
 points = [{value = 1.0, time = "rnow-10"}]
 
-[[test.input]]
-name = "request_success_total.counter;app=test;project=Test;environment=TEST;t=cqa"
-points = [{value = 1.0, time = "rnow-10"}]
-
 [[test.input]]
 name = "test;env=prod"
 points = [{value = 1.0, time = "rnow-10"}]
@@ -930,6 +926,60 @@ req_start = "rnow-10"
 req_stop = "rnow+10"
 values = [1.0, nan]
 
+# ### seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*a*') ###
+
+[[test.render_checks]]
+from = "rnow-10"
+until = "rnow+1"
+timeout = "1h"
+targets = [ 
+    "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*a*')", 
+]
+error_regexp = "^400: Incorrect regex syntax"
+
+# ### seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*') ###
+
+[[test.render_checks]]
+from = "rnow-10"
+until = "rnow+1"
+timeout = "1h"
+targets = [ 
+    "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')", 
+]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=cqa"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=q"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=qac"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
 # # End - Test no flags
 # #########################################################################
 

tests/feature_flags_false/test.toml

@@ -37,10 +37,6 @@ points = [{value = 1.0, time = "rnow-10"}]
 name = "request_success_total.counter;app=test;project=Test;environment=TEST;t=cqa"
 points = [{value = 1.0, time = "rnow-10"}]
 
-[[test.input]]
-name = "request_success_total.counter;app=test;project=Test;environment=TEST;t=cqa"
-points = [{value = 1.0, time = "rnow-10"}]
-
 [[test.input]]
 name = "test;env=prod"
 points = [{value = 1.0, time = "rnow-10"}]
@@ -1292,6 +1288,60 @@ req_start = "rnow-10"
 req_stop = "rnow+10"
 values = [1.0, nan]
 
+# ### seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*a*') ###
+
+[[test.render_checks]]
+from = "rnow-10"
+until = "rnow+1"
+timeout = "1h"
+targets = [ 
+    "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*a*')", 
+]
+error_regexp = "^400: Incorrect regex syntax"
+
+# ### seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*') ###
+
+[[test.render_checks]]
+from = "rnow-10"
+until = "rnow+1"
+timeout = "1h"
+targets = [ 
+    "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')", 
+]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=cqa"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=q"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
+[[test.render_checks.result]]
+name = "request_success_total.counter;app=test;environment=TEST;project=Test;t=qac"
+path = "seriesByTag('name=request_success_total.counter', 'app=test', 'project=Test', 'environment=TEST', 't=~*')"
+consolidation = "avg"
+start = "rnow-10"
+stop = "rnow+10"
+step = 10
+req_start = "rnow-10"
+req_stop = "rnow+10"
+values = [1.0, nan]
+
 # # End - Test no flags
 # #########################################################################