lib/logstorage: disallow using pipe names as the first unquoted words in filter pipe

Improperly written pipes could be silently parsed as filter pipe.
For example, the following query:

   * | by (x)

was silently parsed to:

   * | filter "by" x

It is better to return error, so the user could identify and fix invalid pipe
instead of silently executing invalid query with `filter` pipe.
This commit is contained in:
Aliaksandr Valialkin 2024-10-09 16:10:10 +02:00
parent 252aa792f7
commit 7b475ed95d
No known key found for this signature in database
GPG key ID: 52C003EE2BCDB9EB
2 changed files with 26 additions and 9 deletions

View file

@ -714,13 +714,6 @@ func ParseQuery(s string) (*Query, error) {
func ParseQueryAtTimestamp(s string, timestamp int64) (*Query, error) {
lex := newLexerAtTimestamp(s, timestamp)
// Verify the first token doesn't match pipe names.
firstToken := strings.ToLower(lex.rawToken)
if _, ok := pipeNames[firstToken]; ok {
return nil, fmt.Errorf("the query [%s] cannot start with pipe - it must start with mandatory filter; see https://docs.victoriametrics.com/victorialogs/logsql/#query-syntax; "+
"if the filter isn't missing, then please put the first word of the filter into quotes: %q", s, firstToken)
}
q, err := parseQuery(lex)
if err != nil {
return nil, err
@ -759,9 +752,17 @@ func parseQuery(lex *lexer) (*Query, error) {
}
func parseFilter(lex *lexer) (filter, error) {
if lex.isKeyword("|", "") {
if lex.isKeyword("|", ")", "") {
return nil, fmt.Errorf("missing query")
}
// Verify the first token in the filter doesn't match pipe names.
firstToken := strings.ToLower(lex.rawToken)
if _, ok := pipeNames[firstToken]; ok {
return nil, fmt.Errorf("query filter cannot start with pipe keyword %q; see https://docs.victoriametrics.com/victorialogs/logsql/#query-syntax; "+
"please put the first word of the filter into quotes", firstToken)
}
fo, err := parseFilterOr(lex, "")
if err != nil {
return nil, err

View file

@ -1196,6 +1196,11 @@ func TestParseQuerySuccess(t *testing.T) {
// filter pipe
f(`* | filter error ip:12.3.4.5 or warn`, `* | filter error ip:12.3.4.5 or warn`)
f(`foo | stats by (host) count() logs | filter logs:>50 | sort by (logs desc) | limit 10`, `foo | stats by (host) count(*) as logs | filter logs:>50 | sort by (logs desc) | limit 10`)
f(`* | error`, `* | filter error`)
f(`* | "by"`, `* | filter "by"`)
f(`* | "stats"`, `* | filter "stats"`)
f(`* | "count"`, `* | filter "count"`)
f(`* | foo:bar AND baz:<10`, `* | filter foo:bar baz:<10`)
// extract pipe
f(`* | extract "foo<bar>baz"`, `* | extract "foo<bar>baz"`)
@ -1237,7 +1242,7 @@ func TestParseQueryFailure(t *testing.T) {
t.Helper()
q, err := ParseQuery(s)
if q != nil {
t.Fatalf("expecting nil result; got %s", q)
t.Fatalf("expecting nil result; got [%s]", q)
}
if err == nil {
t.Fatalf("expecting non-nil error")
@ -1635,6 +1640,9 @@ func TestParseQueryFailure(t *testing.T) {
// stats result names identical to by fields
f(`foo | stats by (x) count() x`)
// missing stats function
f(`foo | by (bar)`)
// invalid sort pipe
f(`foo | sort bar`)
f(`foo | sort by`)
@ -1669,6 +1677,14 @@ func TestParseQueryFailure(t *testing.T) {
f(`foo | filter (`)
f(`foo | filter )`)
f(`foo | filter stats`)
f(`foo | filter fields`)
f(`foo | filter by`)
f(`foo | count`)
f(`foo | filter count`)
f(`foo | (`)
f(`foo | )`)
// invalid extract pipe
f(`foo | extract`)
f(`foo | extract bar`)