From e0e0e315cd557d1649077d6cae9d4d10d503c50f Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Thu, 27 Aug 2026 14:55:57 -0500 Subject: [PATCH] fix(builtins): prevent panic in strings.count on empty needle strings.count panicked when called with an empty needle because .windows(0) is invalid. Fix three issues in strings_count: 1. Wrong param index: params[0] was used for the substring error span instead of params[1]. 2. Panic: .windows(needle.len()) panics when needle is empty (len == 0). 3. Semantics: the byte-level .windows() approach counted overlapping matches; OPA uses non-overlapping match semantics (Go strings.Count). The fix: - Correct the param index to params[1] for the substring argument. - Handle empty needle explicitly: return chars().count() + 1, matching Go's strings.Count behaviour (empty needle matches between every character and at both ends). - Use str::matches() for non-overlapping count on non-empty needles. A YAML regression test is added covering basic match, no-match, multi-match, empty-needle (must not panic), empty-needle on empty string, non-overlapping semantics, and multi-byte UTF-8 strings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/builtins/strings.rs | 18 +++--- .../cases/builtins/strings/strings_count.yaml | 59 +++++++++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 tests/interpreter/cases/builtins/strings/strings_count.yaml diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs index fd3a2ee8b..7ca949df3 100644 --- a/src/builtins/strings.rs +++ b/src/builtins/strings.rs @@ -542,15 +542,15 @@ fn strings_count( ensure_args_count(span, name, params, args, 2)?; let search = ensure_string(name, ¶ms[0], &args[0])?; - let substring = ensure_string(name, ¶ms[0], &args[1])?; - - Ok(Value::from( - search - .as_bytes() - .windows(substring.len()) - .filter(|&w| w == substring.as_bytes()) - .count(), - )) + let substring = ensure_string(name, ¶ms[1], &args[1])?; + + if substring.is_empty() { + // An empty needle matches between every character (and at both ends), + // consistent with Go's strings.Count and OPA semantics. + return Ok(Value::from(search.chars().count().saturating_add(1))); + } + + Ok(Value::from(search.matches(substring.as_ref()).count())) } fn startswith(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> Result { diff --git a/tests/interpreter/cases/builtins/strings/strings_count.yaml b/tests/interpreter/cases/builtins/strings/strings_count.yaml new file mode 100644 index 000000000..5de134629 --- /dev/null +++ b/tests/interpreter/cases/builtins/strings/strings_count.yaml @@ -0,0 +1,59 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +cases: + - note: basic match + data: {} + modules: [] + query: 'x := strings.count("cheese", "e")' + want_result: + x: 3 + + - note: no match + data: {} + modules: [] + query: 'x := strings.count("dummy", "x")' + want_result: + x: 0 + + - note: multiple separate matches + data: {} + modules: [] + query: 'x := strings.count("hello hello hello world", "hello")' + want_result: + x: 3 + + - note: empty needle returns char_count+1 + data: {} + modules: [] + query: 'x := strings.count("abc", "")' + want_result: + x: 4 + + - note: empty needle on empty string + data: {} + modules: [] + query: 'x := strings.count("", "")' + want_result: + x: 1 + + - note: empty needle on single char + data: {} + modules: [] + query: 'x := strings.count("a", "")' + want_result: + x: 2 + + - note: non-overlapping matches only + data: {} + modules: [] + query: 'x := strings.count("aaaa", "aa")' + want_result: + x: 2 + + - note: multibyte utf8 string with empty needle + data: {} + modules: [] + query: 'x := strings.count("\u4e16\u754c", "")' + want_result: + x: 3