diff --git a/finder/index.go b/finder/index.go index 1d4e59ee..6ec4a0f3 100644 --- a/finder/index.go +++ b/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( diff --git a/finder/tagged.go b/finder/tagged.go index 811b4012..7c7e1c75 100644 --- a/finder/tagged.go +++ b/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) ) diff --git a/helper/clickhouse/clickhouse.go b/helper/clickhouse/clickhouse.go index 326c0129..ebba7fec 100644 --- a/helper/clickhouse/clickhouse.go +++ b/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" } diff --git a/helper/clickhouse/clickhouse_test.go b/helper/clickhouse/clickhouse_test.go index 4a3304a8..5c72d399 100644 --- a/helper/clickhouse/clickhouse_test.go +++ b/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, diff --git a/pkg/where/match.go b/pkg/where/match.go index 9c002e41..0fe678e6 100644 --- a/pkg/where/match.go +++ b/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 "" diff --git a/pkg/where/match_test.go b/pkg/where/match_test.go index 378c1679..f6b31ad6 100644 --- a/pkg/where/match_test.go +++ b/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 { diff --git a/tests/feature_flags_both_true/test.toml b/tests/feature_flags_both_true/test.toml index 83e039c9..fd383dca 100644 --- a/tests/feature_flags_both_true/test.toml +++ b/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 # ######################################################################### diff --git a/tests/feature_flags_dont_match_missing_tags/test.toml b/tests/feature_flags_dont_match_missing_tags/test.toml index e58016cb..8e38d444 100644 --- a/tests/feature_flags_dont_match_missing_tags/test.toml +++ b/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 # ######################################################################### diff --git a/tests/feature_flags_false/test.toml b/tests/feature_flags_false/test.toml index a6a9e6b1..f3f7a65f 100644 --- a/tests/feature_flags_false/test.toml +++ b/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 # ######################################################################### diff --git a/tests/feature_flags_use_carbon_behaviour/test.toml b/tests/feature_flags_use_carbon_behaviour/test.toml index 61fb2801..c332be71 100644 --- a/tests/feature_flags_use_carbon_behaviour/test.toml +++ b/tests/feature_flags_use_carbon_behaviour/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"}] @@ -1336,6 +1332,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 # #########################################################################