From 580f0b816e9282a837f10642c490bb7a21c68f3c Mon Sep 17 00:00:00 2001 From: Nityananda Gohain Date: Fri, 1 Nov 2024 13:52:13 +0530 Subject: [PATCH] fix: issues with resource query builder w.r.t quotes (#6318) --- .../app/logs/v4/query_builder.go | 2 +- .../app/resource/resource_query_builder.go | 22 +++++++++---------- .../resource/resource_query_builder_test.go | 14 ++++++------ pkg/query-service/utils/format.go | 11 +++++++++- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/pkg/query-service/app/logs/v4/query_builder.go b/pkg/query-service/app/logs/v4/query_builder.go index e38fb94934..3952d0e7e1 100644 --- a/pkg/query-service/app/logs/v4/query_builder.go +++ b/pkg/query-service/app/logs/v4/query_builder.go @@ -149,7 +149,7 @@ func buildAttributeFilter(item v3.FilterItem) (string, error) { return fmt.Sprintf(logsOp, keyName, fmtVal), nil case v3.FilterOperatorContains, v3.FilterOperatorNotContains: // we also want to treat %, _ as literals for contains - val := utils.QuoteEscapedStringForContains(fmt.Sprintf("%s", item.Value)) + val := utils.QuoteEscapedStringForContains(fmt.Sprintf("%s", item.Value), false) // for body the contains is case insensitive if keyName == BODY { logsOp = strings.Replace(logsOp, "ILIKE", "LIKE", 1) // removing i from ilike and not ilike diff --git a/pkg/query-service/app/resource/resource_query_builder.go b/pkg/query-service/app/resource/resource_query_builder.go index bbf9310386..c03807b130 100644 --- a/pkg/query-service/app/resource/resource_query_builder.go +++ b/pkg/query-service/app/resource/resource_query_builder.go @@ -49,7 +49,7 @@ func buildResourceFilter(logsOp string, key string, op v3.FilterOperator, value case v3.FilterOperatorContains, v3.FilterOperatorNotContains: // this is required as clickhouseFormattedValue add's quotes to the string // we also want to treat %, _ as literals for contains - escapedStringValue := utils.QuoteEscapedStringForContains(lowerValue) + escapedStringValue := utils.QuoteEscapedStringForContains(lowerValue, false) return fmt.Sprintf("%s %s '%%%s%%'", lowerSearchKey, logsOp, escapedStringValue) case v3.FilterOperatorLike, v3.FilterOperatorNotLike: // this is required as clickhouseFormattedValue add's quotes to the string @@ -92,7 +92,7 @@ func buildIndexFilterForInOperator(key string, op v3.FilterOperator, value inter // if there are no values to filter on, return an empty string if len(values) > 0 { for _, v := range values { - value := utils.QuoteEscapedStringForContains(v) + value := utils.QuoteEscapedStringForContains(v, true) conditions = append(conditions, fmt.Sprintf("labels %s '%%\"%s\":\"%s\"%%'", sqlOp, key, value)) } return "(" + strings.Join(conditions, separator) + ")" @@ -110,24 +110,24 @@ func buildIndexFilterForInOperator(key string, op v3.FilterOperator, value inter func buildResourceIndexFilter(key string, op v3.FilterOperator, value interface{}) string { // not using clickhouseFormattedValue as we don't wan't the quotes strVal := fmt.Sprintf("%s", value) - formattedValueEscapedForContains := strings.ToLower(utils.QuoteEscapedStringForContains(strVal)) - formattedValueEscaped := utils.QuoteEscapedString(strVal) - formattedValueEscapedLower := strings.ToLower(formattedValueEscaped) + fmtValEscapedForContains := utils.QuoteEscapedStringForContains(strVal, true) + fmtValEscapedForContainsLower := strings.ToLower(fmtValEscapedForContains) + fmtValEscapedLower := strings.ToLower(utils.QuoteEscapedString(strVal)) // add index filters switch op { case v3.FilterOperatorContains: - return fmt.Sprintf("lower(labels) like '%%%s%%%s%%'", key, formattedValueEscapedForContains) + return fmt.Sprintf("lower(labels) like '%%%s%%%s%%'", key, fmtValEscapedForContainsLower) case v3.FilterOperatorNotContains: - return fmt.Sprintf("lower(labels) not like '%%%s%%%s%%'", key, formattedValueEscapedForContains) + return fmt.Sprintf("lower(labels) not like '%%%s%%%s%%'", key, fmtValEscapedForContainsLower) case v3.FilterOperatorLike: - return fmt.Sprintf("lower(labels) like '%%%s%%%s%%'", key, formattedValueEscapedLower) + return fmt.Sprintf("lower(labels) like '%%%s%%%s%%'", key, fmtValEscapedLower) case v3.FilterOperatorNotLike: - return fmt.Sprintf("lower(labels) not like '%%%s%%%s%%'", key, formattedValueEscapedLower) + return fmt.Sprintf("lower(labels) not like '%%%s%%%s%%'", key, fmtValEscapedLower) case v3.FilterOperatorEqual: - return fmt.Sprintf("labels like '%%%s%%%s%%'", key, formattedValueEscaped) + return fmt.Sprintf("labels like '%%%s%%%s%%'", key, fmtValEscapedForContains) case v3.FilterOperatorNotEqual: - return fmt.Sprintf("labels not like '%%%s%%%s%%'", key, formattedValueEscaped) + return fmt.Sprintf("labels not like '%%%s%%%s%%'", key, fmtValEscapedForContains) case v3.FilterOperatorRegex, v3.FilterOperatorNotRegex: // don't try to do anything for regex. return "" diff --git a/pkg/query-service/app/resource/resource_query_builder_test.go b/pkg/query-service/app/resource/resource_query_builder_test.go index f390ff9c6e..ff9c251882 100644 --- a/pkg/query-service/app/resource/resource_query_builder_test.go +++ b/pkg/query-service/app/resource/resource_query_builder_test.go @@ -138,9 +138,9 @@ func Test_buildIndexFilterForInOperator(t *testing.T) { args: args{ key: "service.name", op: v3.FilterOperatorNotIn, - value: "application'\"_s", + value: `application'"_s`, }, - want: `(labels not like '%"service.name":"application\'"\_s"%')`, + want: `(labels not like '%"service.name":"application\'\\\\"\_s"%')`, }, } for _, tt := range tests { @@ -231,9 +231,9 @@ func Test_buildResourceIndexFilter(t *testing.T) { args: args{ key: "service.name", op: v3.FilterOperatorEqual, - value: "Application", + value: `Application"`, }, - want: `labels like '%service.name%Application%'`, + want: `labels like '%service.name%Application\\\\"%'`, }, } for _, tt := range tests { @@ -319,7 +319,7 @@ func Test_buildResourceFiltersFromFilterItems(t *testing.T) { Type: v3.AttributeKeyTypeResource, }, Operator: v3.FilterOperatorContains, - Value: "test1", + Value: `test1"`, }, }, }, @@ -327,8 +327,8 @@ func Test_buildResourceFiltersFromFilterItems(t *testing.T) { want: []string{ "simpleJSONExtractString(labels, 'service.name') = 'test'", "labels like '%service.name%test%'", - "simpleJSONExtractString(lower(labels), 'namespace') LIKE '%test1%'", - "lower(labels) like '%namespace%test1%'", + `simpleJSONExtractString(lower(labels), 'namespace') LIKE '%test1"%'`, + `lower(labels) like '%namespace%test1\\\\"%'`, }, wantErr: false, }, diff --git a/pkg/query-service/utils/format.go b/pkg/query-service/utils/format.go index f09f4dffd6..963abac219 100644 --- a/pkg/query-service/utils/format.go +++ b/pkg/query-service/utils/format.go @@ -156,9 +156,18 @@ func QuoteEscapedString(str string) string { return str } -func QuoteEscapedStringForContains(str string) string { +func QuoteEscapedStringForContains(str string, isIndex bool) string { // https: //clickhouse.com/docs/en/sql-reference/functions/string-search-functions#like str = QuoteEscapedString(str) + + // we are adding this because if a string contains quote `"` it will be stored as \" in clickhouse + // to query that using like our query should be \\\\" + if isIndex { + // isIndex is true means that the extra slash is present + // [\"a\",\"b\",\"sdf\"] + str = strings.ReplaceAll(str, `"`, `\\\\"`) + } + str = strings.ReplaceAll(str, `%`, `\%`) str = strings.ReplaceAll(str, `_`, `\_`) return str