fix: detect misquoted tag values and return an error #22754

davidby-influx · 2021-10-25T23:18:34Z

SHOW TAG KEYS FROM "foo" where bar="misquoted" is
erroneous, because the tag value must be enclosed
in single, not double, quotes. Although this
correctly returns no tag keys, it is very
inefficient and has caused out-of-memory failures
at a customer. This fix short-circuits the query and returns
an error. This fix also handles SHOW TAG VALUES
with a misquoted WHERE clause.

Closes #22755

Well-formatted commit messages
Rebased/mergeable
Tests pass

SHOW TAG KEYS FROM "foo" where bar="misquoted" is erroneous, because the tag value must be enclosed in single, not double, quotes. Although this correctly returns no tag keys, it is very inefficient and has cause out-of-memory failures at a customer. This fix short-circuits the query.

lesam · 2021-10-26T13:47:12Z

tsdb/store.go

@@ -1822,6 +1824,27 @@ func isTagKeyClause(e influxql.Expr) (bool, error) {
 	return false, nil
 }

+func fixBadQuoteTagValueClause(e influxql.Expr) influxql.Expr {


As discussed offline, I'd prefer that semantics we don't support become an error instead of hiding the problem by returning a false.

lesam · 2021-10-27T12:00:20Z

tsdb/store.go

@@ -1824,25 +1825,29 @@ func isTagKeyClause(e influxql.Expr) (bool, error) {
 	return false, nil
 }

-func fixBadQuoteTagValueClause(e influxql.Expr) influxql.Expr {
+func isBadQuoteTagValueClause(e influxql.Expr) error {


I don't think we should put in there permanently, but do any tests fail if we put this check on SELECT as well? might highlight a case we missed.

SELECT ... WHERE supports the syntax this disallows, so this will err on legal queries.

Here are the test failures where this test breaks correct queries in SELECT

--- FAIL: TestServer_Query_Where_With_Tags (0.57s) --- FAIL: TestServer_Query_Where_With_Tags/where_comparing_tag_and_field (0.00s) server_test.go:6457: where comparing tag and field: unexpected results query: select foo from where_events where tennant != foo params: map[db:[db0]] exp: {"results":[{"statement_id":0,"series":[{"name":"where_events","columns":["time","foo"],"values":[["2009-11-10T23:00:02Z","bar"],["2009-11-10T23:00:03Z","baz"],["2009-11-10T23:00:04Z","bat"],["2009-11-10T23:00:05Z","bar"],["2009-11-10T23:00:06Z","bap"]]}]}]} actual: {"results":[{"statement_id":0,"error":"bad WHERE clause. tag value must be inside single quotes: tennant != foo"}]} --- FAIL: TestServer_Query_Where_With_Tags/where_comparing_tag_and_tag (0.00s) server_test.go:6457: where comparing tag and tag: unexpected results query: select foo from where_events where tennant = tennant params: map[db:[db0]] exp: {"results":[{"statement_id":0,"series":[{"name":"where_events","columns":["time","foo"],"values":[["2009-11-10T23:00:02Z","bar"],["2009-11-10T23:00:03Z","baz"],["2009-11-10T23:00:04Z","bat"],["2009-11-10T23:00:05Z","bar"],["2009-11-10T23:00:06Z","bap"]]}]}]} actual: {"results":[{"statement_id":0,"error":"bad WHERE clause. tag value must be inside single quotes: tennant = tennant"}]}

lesam

One comment

lesam · 2021-10-27T17:38:49Z

tsdb/store.go

+			_, lOk := e.LHS.(*influxql.VarRef)
+			_, rOk := e.RHS.(*influxql.VarRef)
+			if lOk && rOk {
+				return fmt.Errorf("bad WHERE clause. tag value must be inside single quotes: %s", e.String())


Suggested change

return fmt.Errorf("bad WHERE clause. tag value must be inside single quotes: %s", e.String())

return fmt.Errorf("bad WHERE clause for metaquery: either LHS or RHS must be a constant, not a tag or field: %s", e.String())

I think we should emphasize in the error that this is only a metaquery restriction, since normal selects are OK.

I want to mention single quotes so they know how to fix it. I don't think the fact that single quotes give literals and double quotes give fields and tags is very obvious. I also don't like LHS and RHS; too compiler-geeky. How about:

"bad WHERE clause for metaquery. one term must be a string literal tag value within single quotes: %s"

The user will know which term should be the tag value, presumably.

Sounds good to me, though I'd slightly prefer : instead of . since that's generally the style we use for error messages.

I was formatting with period based on another message in that file:

"cannot delete data. DB contains shards using both inmem and tsi1 indexes. Please convert all shards to use the same index type to delete data."

Maybe I should have made a wider search for compound error statements.

compromise, because I hate two colons in a singe sentence. semi-colon.

SHOW TAG KEYS FROM "foo" where bar="misquoted" is erroneous, because the tag value must be enclosed in single, not double, quotes. Although this correctly returns no tag keys, it is very inefficient and has cause out-of-memory failures at a customer. This fix short-circuits the query. closes #22755 (cherry picked from commit af9e89a)

SHOW TAG KEYS FROM "foo" where bar="misquoted" is erroneous, because the tag value must be enclosed in single, not double, quotes. Although this correctly returns no tag keys, it is very inefficient and has cause out-of-memory failures at a customer. This fix short-circuits the query. closes #22755 (cherry picked from commit af9e89a) closes #22756

SHOW TAG KEYS FROM "foo" where bar="misquoted" is erroneous, because the tag value must be enclosed in single, not double, quotes. Although this correctly returns no tag keys, it is very inefficient and has cause out-of-memory failures at a customer. This fix short-circuits the query. closes #22755 (cherry picked from commit af9e89a)

SHOW TAG KEYS FROM "foo" where bar="misquoted" is erroneous, because the tag value must be enclosed in single, not double, quotes. Although this correctly returns no tag keys, it is very inefficient and has cause out-of-memory failures at a customer. This fix short-circuits the query. closes #22755 (cherry picked from commit af9e89a) closes #22786

SHOW TAG KEYS FROM "foo" where bar="misquoted" is erroneous, because the tag value must be enclosed in single, not double, quotes. Although this correctly returns no tag keys, it is very inefficient and has cause out-of-memory failures at a customer. This fix short-circuits the query. closes #22755 (cherry picked from commit af9e89a) closes #22757

davidby-influx added area/meta queries 1.x labels Oct 25, 2021

davidby-influx requested a review from lesam October 25, 2021 23:18

davidby-influx self-assigned this Oct 25, 2021

davidby-influx linked an issue Oct 25, 2021 that may be closed by this pull request

Misquoted tag value in SHOW TAG KEYS query can cause OOM #22755

Closed

davidby-influx added team/edge kind/bug labels Oct 25, 2021

lesam reviewed Oct 26, 2021

View reviewed changes

fix: return an error for misquoted tag value

72ee1f3

davidby-influx marked this pull request as ready for review October 26, 2021 20:26

davidby-influx requested a review from lesam October 26, 2021 20:26

davidby-influx changed the title ~~fix: detect misquoted tag values and replace them~~ fix: detect misquoted tag values and return an error Oct 26, 2021

davidby-influx added 2 commits October 26, 2021 15:07

fix: check for misquoted tag values in SHOW TAG VALUES

31d17de

chore: formatting

46327a6

lesam reviewed Oct 27, 2021

View reviewed changes

davidby-influx requested a review from lesam October 27, 2021 16:27

lesam suggested changes Oct 27, 2021

View reviewed changes

chore: clarify error message

c9cfa50

davidby-influx requested a review from lesam October 27, 2021 18:10

lesam approved these changes Oct 27, 2021

View reviewed changes

davidby-influx merged commit af9e89a into master-1.x Oct 27, 2021

davidby-influx deleted the DSB_bad_quote branch October 27, 2021 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: detect misquoted tag values and return an error #22754

fix: detect misquoted tag values and return an error #22754

davidby-influx commented Oct 25, 2021 •

edited

Loading

lesam Oct 26, 2021

lesam Oct 27, 2021

davidby-influx Oct 27, 2021

davidby-influx Oct 27, 2021

lesam left a comment

lesam Oct 27, 2021

lesam Oct 27, 2021

davidby-influx Oct 27, 2021

lesam Oct 27, 2021

davidby-influx Oct 27, 2021

davidby-influx Oct 27, 2021

	return fmt.Errorf("bad WHERE clause. tag value must be inside single quotes: %s", e.String())
	return fmt.Errorf("bad WHERE clause for metaquery: either LHS or RHS must be a constant, not a tag or field: %s", e.String())

fix: detect misquoted tag values and return an error #22754

fix: detect misquoted tag values and return an error #22754

Conversation

davidby-influx commented Oct 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lesam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidby-influx commented Oct 25, 2021 •

edited

Loading