lib/flagutil: ArrayString: support commas inside quoted strings and inside [], {} and () braces

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3915

docs/CHANGELOG.md

@@ -41,6 +41,7 @@ created by v1.90.0 or newer versions. The solution is to upgrade to v1.90.0 or n
 * BUGFIX: allow using dashes and dots in environment variables names referred in config files via `%{ENV-VAR.SYNTAX}`. See [these docs](https://docs.victoriametrics.com/#environment-variables) and [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3999).
 * BUGFIX: return back query performance scalability on hosts with big number of CPU cores. The scalability has been reduced in [v1.86.0](https://docs.victoriametrics.com/CHANGELOG.html#v1860). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3966).
 * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly convert [VictoriaMetrics historgram buckets](https://valyala.medium.com/improving-histogram-usability-for-prometheus-and-grafana-bc7e5df0e350) to Prometheus histogram buckets when VictoriaMetrics histogram contain zero buckets. Previously these buckets were ignored, and this could lead to missing Prometheus histogram buckets after the conversion. Thanks to @zklapow for [the fix](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4021).
+* BUGFIX: properly support comma-separated filters inside [retention filters](https://docs.victoriametrics.com/#retention-filters). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3915).
 
 
 ## [v1.89.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.89.1)

lib/flagutil/array.go

@@ -59,16 +59,19 @@ func NewArrayBytes(name, description string) *ArrayBytes {
 //	-foo=value1 -foo=value2
 //	-foo=value1,value2
 //
-// Flag values may be quoted. For instance, the following arg creates an array of ("a", "b, c") items:
+// Each flag value may contain commas inside single quotes, double quotes, [], () or {} braces.
+// For example, -foo=[a,b,c] defines a single command-line flag with `[a,b,c]` value.
 //
-//	-foo='a,"b, c"'
+// Flag values may be quoted. For instance, the following arg creates an array of ("a", "b,c") items:
+//
+//	-foo='a,"b,c"'
 type ArrayString []string
 
 // String implements flag.Value interface
 func (a *ArrayString) String() string {
 	aEscaped := make([]string, len(*a))
 	for i, v := range *a {
-		if strings.ContainsAny(v, `", `+"\n") {
+		if strings.ContainsAny(v, `,'"{[(`+"\n") {
 			v = fmt.Sprintf("%q", v)
 		}
 		aEscaped[i] = v
@@ -94,55 +97,105 @@ func parseArrayValues(s string) []string {
 		if len(tail) == 0 {
 			return values
 		}
-		if tail[0] == ',' {
-			tail = tail[1:]
-		}
 		s = tail
+		if s[0] == ',' {
+			s = s[1:]
+		}
 	}
 }
 
+var closeQuotes = map[byte]byte{
+	'"':  '"',
+	'\'': '\'',
+	'[':  ']',
+	'{':  '}',
+	'(':  ')',
+}
+
 func getNextArrayValue(s string) (string, string) {
-	if len(s) == 0 {
-		return "", ""
+	v, tail := getNextArrayValueMaybeQuoted(s)
+	if strings.HasPrefix(v, `"`) && strings.HasSuffix(v, `"`) {
+		vUnquoted, err := strconv.Unquote(v)
+		if err == nil {
+			return vUnquoted, tail
+		}
+		v = v[1 : len(v)-1]
+		v = strings.ReplaceAll(v, `\"`, `"`)
+		v = strings.ReplaceAll(v, `\\`, `\`)
+		return v, tail
 	}
-	if s[0] != '"' {
-		// Fast path - unquoted string
-		n := strings.IndexByte(s, ',')
+	if strings.HasPrefix(v, `'`) && strings.HasSuffix(v, `'`) {
+		v = v[1 : len(v)-1]
+		v = strings.ReplaceAll(v, `\'`, "'")
+		v = strings.ReplaceAll(v, `\\`, `\`)
+		return v, tail
+	}
+	return v, tail
+}
+
+func getNextArrayValueMaybeQuoted(s string) (string, string) {
+	idx := 0
+	for {
+		n := strings.IndexAny(s[idx:], `,"'[{(`)
 		if n < 0 {
 			// The last item
 			return s, ""
 		}
-		return s[:n], s[n:]
+		idx += n
+		ch := s[idx]
+		if ch == ',' {
+			// The next item
+			return s[:idx], s[idx:]
+		}
+		idx++
+		m := indexCloseQuote(s[idx:], closeQuotes[ch])
+		idx += m
 	}
+}
 
-	// Find the end of quoted string
-	end := 1
-	ss := s[1:]
+func indexCloseQuote(s string, closeQuote byte) int {
+	if closeQuote == '"' || closeQuote == '\'' {
+		idx := 0
+		for {
+			n := strings.IndexByte(s[idx:], closeQuote)
+			if n < 0 {
+				return 0
+			}
+			idx += n
+			if n := getTrailingBackslashesCount(s[:idx]); n%2 == 1 {
+				// The quote is escaped with backslash. Skip it
+				idx++
+				continue
+			}
+			return idx + 1
+		}
+	}
+	idx := 0
 	for {
-		n := strings.IndexByte(ss, '"')
+		n := strings.IndexAny(s[idx:], `"'[{()}]`)
 		if n < 0 {
-			// Cannot find trailing quote. Return the whole string till the end.
-			return s, ""
+			return 0
 		}
-		end += n + 1
-		// Verify whether the trailing quote is escaped with backslash.
-		backslashes := 0
-		for n > backslashes && ss[n-backslashes-1] == '\\' {
-			backslashes++
+		idx += n
+		ch := s[idx]
+		if ch == closeQuote {
+			return idx + 1
 		}
-		if backslashes&1 == 0 {
-			// The trailing quote isn't escaped.
-			break
+		idx++
+		m := indexCloseQuote(s[idx:], closeQuotes[ch])
+		if m == 0 {
+			return 0
 		}
-		// The trailing quote is escaped. Continue searching for the next quote.
-		ss = ss[n+1:]
+		idx += m
 	}
-	v := s[:end]
-	vUnquoted, err := strconv.Unquote(v)
-	if err == nil {
-		v = vUnquoted
+}
+
+func getTrailingBackslashesCount(s string) int {
+	n := len(s)
+	for n > 0 && s[n-1] == '\\' {
+		n--
 	}
-	return v, s[end:]
+	return len(s) - n
 }
 
 // GetOptionalArg returns optional arg under the given argIdx.

lib/flagutil/array_test.go

@@ -53,15 +53,54 @@ func TestArrayString_Set(t *testing.T) {
 			t.Fatalf("unexpected values parsed;\ngot\n%q\nwant\n%q", a, expectedValues)
 		}
 	}
+	// Zero args
 	f("", nil)
+
+	// Single arg
 	f(`foo`, []string{`foo`})
-	f(`foo,b ar,baz`, []string{`foo`, `b ar`, `baz`})
-	f(`foo,b\"'ar,"baz,d`, []string{`foo`, `b\"'ar`, `"baz,d`})
-	f(`,foo,,ba"r,`, []string{``, `foo`, ``, `ba"r`, ``})
+	f(`fo"o`, []string{`fo"o`})
+	f(`fo'o`, []string{`fo'o`})
+	f(`fo{o`, []string{`fo{o`})
+	f(`fo[o`, []string{`fo[o`})
+	f(`fo(o`, []string{`fo(o`})
+
+	// Single arg with Prometheus label filters
+	f(`foo{bar="baz",x="y"}`, []string{`foo{bar="baz",x="y"}`})
+	f(`foo{bar="ba}z",x="y"}`, []string{`foo{bar="ba}z",x="y"}`})
+	f(`foo{bar='baz',x="y"}`, []string{`foo{bar='baz',x="y"}`})
+	f(`foo{bar='baz',x='y'}`, []string{`foo{bar='baz',x='y'}`})
+	f(`foo{bar='ba}z',x='y'}`, []string{`foo{bar='ba}z',x='y'}`})
+	f(`{foo="ba[r",baz='a'}`, []string{`{foo="ba[r",baz='a'}`})
+
+	// Single arg with JSON
+	f(`[1,2,3]`, []string{`[1,2,3]`})
+	f(`{"foo":"ba,r",baz:x}`, []string{`{"foo":"ba,r",baz:x}`})
+
+	// Single quoted arg
+	f(`"foo"`, []string{`foo`})
+	f(`"fo,'o"`, []string{`fo,'o`})
+	f(`"f\\o,\'\"o"`, []string{`f\o,\'"o`})
+	f(`"foo{bar='baz',x='y'}"`, []string{`foo{bar='baz',x='y'}`})
+	f(`'foo'`, []string{`foo`})
+	f(`'fo,"o'`, []string{`fo,"o`})
+	f(`'f\\o,\'\"o'`, []string{`f\o,'\"o`})
+	f(`'foo{bar="baz",x="y"}'`, []string{`foo{bar="baz",x="y"}`})
+
+	// Multiple args
+	f(`foo,bar,baz`, []string{`foo`, `bar`, `baz`})
+	f(`"foo",'bar',{[(ba'",z"`, []string{`foo`, `bar`, `{[(ba'",z"`})
+	f(`foo,b"'ar,"baz,d`, []string{`foo`, `b"'ar,"baz`, `d`})
+	f(`{foo="b,ar"},baz{x="y",z="d"}`, []string{`{foo="b,ar"}`, `baz{x="y",z="d"}`})
+
+	// Empty args
 	f(`""`, []string{``})
-	f(`"foo,b\nar"`, []string{`foo,b` + "\n" + `ar`})
-	f(`"foo","bar",baz`, []string{`foo`, `bar`, `baz`})
-	f(`,fo,"\"b, a'\\",,r,`, []string{``, `fo`, `"b, a'\`, ``, `r`, ``})
+	f(`''`, []string{``})
+	f(`,`, []string{``, ``})
+	f(`,foo,,ba"r,`, []string{``, `foo`, ``, `ba"r`, ``})
+
+	// Special chars inside double quotes
+	f(`"foo,b\nar"`, []string{"foo,b\nar"})
+	f(`"foo\x23bar"`, []string{"foo\x23bar"})
 }
 
 func TestArrayString_GetOptionalArg(t *testing.T) {
@@ -100,6 +139,7 @@ func TestArrayString_String(t *testing.T) {
 	f(",foo,")
 	f(`", foo","b\"ar",`)
 	f(`,"\nfoo\\",bar`)
+	f(`"foo{bar=~\"baz\",a!=\"b\"}","{a='b,{[(c'}"`)
 }
 
 func TestArrayDuration(t *testing.T) {