From f10ff8a805254aea3907d69ceaf145d3d5b6626d Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Mon, 10 Aug 2026 14:45:02 -0500 Subject: [PATCH] fix(regex): register provider and fix match/compile parity The BuiltinProvider entry for opa-builtins-regex was commented out in its META-INF/services file, so ServiceLoader never discovered it and none of the eight regex builtins were reachable for consumers. Registering it exposed two parity gaps the fixtures had been skipping: - regex.match used Matcher.matches(), which requires the pattern to match the entire input. Go's regexp.MatchString searches instead, so regex.match("", "x") must be true and an unanchored pattern must match a substring. - An invalid pattern let Java's unchecked PatternSyntaxException escape and abort evaluation. OPA reports this as a builtin error, so the call is undefined by default and only aborts under strict-builtin-errors. All six compile sites now go through a helper that converts it to BuiltinError. Adds RegexBuiltinsTest, including a ServiceLoader assertion: the module had no unit tests, and the existing ones elsewhere call impl classes directly, which is why the registration gap went unnoticed. Eight of the ten new tests fail against the unfixed code. Removes the eight regex entries from known-missing-builtins.txt (64 -> 56). Signed-off-by: Sebastian Spaink --- .../opa-builtins-regex/build.gradle.kts | 7 ++ .../opa/ast/builtin/impls/RegexBuiltins.java | 28 ++++- ...licy_agent.opa.ast.builtin.BuiltinProvider | 2 +- .../ast/builtin/impls/RegexBuiltinsTest.java | 116 ++++++++++++++++++ .../compliance/known-missing-builtins.txt | 8 -- 5 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 opa-builtins/opa-builtins-regex/src/test/java/io/github/open_policy_agent/opa/ast/builtin/impls/RegexBuiltinsTest.java diff --git a/opa-builtins/opa-builtins-regex/build.gradle.kts b/opa-builtins/opa-builtins-regex/build.gradle.kts index 8d9240b3..f6400dc2 100644 --- a/opa-builtins/opa-builtins-regex/build.gradle.kts +++ b/opa-builtins/opa-builtins-regex/build.gradle.kts @@ -10,6 +10,9 @@ dependencies { api(project(":opa-evaluator")) implementation("dk.brics.automaton:automaton:1.11-8") + + testImplementation("org.junit.jupiter:junit-jupiter:6.1.2") + testRuntimeOnly("org.junit.platform:junit-platform-launcher:6.1.2") } java { @@ -17,3 +20,7 @@ java { languageVersion = JavaLanguageVersion.of(17) } } + +tasks.test { + useJUnitPlatform() +} diff --git a/opa-builtins/opa-builtins-regex/src/main/java/io/github/open_policy_agent/opa/ast/builtin/impls/RegexBuiltins.java b/opa-builtins/opa-builtins-regex/src/main/java/io/github/open_policy_agent/opa/ast/builtin/impls/RegexBuiltins.java index a2443091..cad00e75 100644 --- a/opa-builtins/opa-builtins-regex/src/main/java/io/github/open_policy_agent/opa/ast/builtin/impls/RegexBuiltins.java +++ b/opa-builtins/opa-builtins-regex/src/main/java/io/github/open_policy_agent/opa/ast/builtin/impls/RegexBuiltins.java @@ -6,6 +6,7 @@ import java.util.function.BiFunction; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; import io.github.open_policy_agent.opa.ast.builtin.BuiltinError; import io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider; import io.github.open_policy_agent.opa.ast.builtin.OpaBuiltin; @@ -22,6 +23,19 @@ public class RegexBuiltins implements BuiltinProvider { + /** + * Compiles `pattern`, reporting a syntax error as a {@link BuiltinError} rather than letting + * Java's unchecked {@link PatternSyntaxException} escape. OPA treats an invalid pattern as a + * builtin error, so the call is undefined by default and only aborts under strict-builtin-errors. + */ + private static Pattern compile(String pattern) { + try { + return Pattern.compile(pattern); + } catch (PatternSyntaxException e) { + throw new BuiltinError("error parsing regexp: " + e.getMessage()); + } + } + @Override public Map> builtins() { RegexBuiltins instance = new RegexBuiltins(); @@ -51,7 +65,9 @@ public RegoBoolean match(EvaluationContext ctx, RegoValue[] args) { String pattern = getArg(args, 0, RegoString.class).getValue(); String value = getArg(args, 1, RegoString.class).getValue(); - return RegoBoolean.of(Pattern.compile(pattern).matcher(value).matches()); + // Go's regexp.MatchString searches for a match anywhere in the input, so an unanchored + // pattern matches a substring. Matcher.matches() would instead require the whole input. + return RegoBoolean.of(compile(pattern).matcher(value).find()); } @OpaBuiltin( @@ -88,7 +104,7 @@ public RegoArray split(EvaluationContext ctx, RegoValue[] args) { String value = getArg(args, 1, RegoString.class).getValue(); RegoArray result = new RegoArray(); - for (String split : Pattern.compile(pattern).split(value, -1)) { + for (String split : compile(pattern).split(value, -1)) { result.addValue(new RegoString(split)); } return result; @@ -115,7 +131,7 @@ public RegoArray find(EvaluationContext ctx, RegoValue[] args) { } RegoArray result = new RegoArray(); - Matcher matcher = Pattern.compile(pattern).matcher(value); + Matcher matcher = compile(pattern).matcher(value); int count = 0; while (matcher.find() && count < number) { result.addValue(new RegoString(matcher.group())); @@ -144,7 +160,7 @@ public RegoArray findSubstringMatch(EvaluationContext ctx, RegoValue[] args) { } RegoArray result = new RegoArray(); - Matcher matcher = Pattern.compile(pattern).matcher(value); + Matcher matcher = compile(pattern).matcher(value); int count = 0; while (matcher.find() && count < number) { RegoArray matchGroups = new RegoArray(); @@ -171,7 +187,7 @@ public RegoString replace(EvaluationContext ctx, RegoValue[] args) { String s = getArg(args, 0, RegoString.class).getValue(); String pattern = getArg(args, 1, RegoString.class).getValue(); String value = getArg(args, 2, RegoString.class).getValue(); - return new RegoString(Pattern.compile(pattern).matcher(s).replaceAll(value)); + return new RegoString(compile(pattern).matcher(s).replaceAll(value)); } @OpaBuiltin( @@ -230,7 +246,7 @@ public RegoBoolean templateMatch(EvaluationContext ctx, RegoValue[] args) { // Match value against constructed pattern String pattern = patternBuilder.toString(); - return RegoBoolean.of(Pattern.compile(pattern).matcher(value).matches()); + return RegoBoolean.of(compile(pattern).matcher(value).matches()); } @OpaBuiltin( diff --git a/opa-builtins/opa-builtins-regex/src/main/resources/META-INF/services/io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider b/opa-builtins/opa-builtins-regex/src/main/resources/META-INF/services/io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider index dad75273..0be642ae 100644 --- a/opa-builtins/opa-builtins-regex/src/main/resources/META-INF/services/io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider +++ b/opa-builtins/opa-builtins-regex/src/main/resources/META-INF/services/io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider @@ -1 +1 @@ -#io.github.open_policy_agent.opa.ast.builtin.impls.RegexBuiltins +io.github.open_policy_agent.opa.ast.builtin.impls.RegexBuiltins diff --git a/opa-builtins/opa-builtins-regex/src/test/java/io/github/open_policy_agent/opa/ast/builtin/impls/RegexBuiltinsTest.java b/opa-builtins/opa-builtins-regex/src/test/java/io/github/open_policy_agent/opa/ast/builtin/impls/RegexBuiltinsTest.java new file mode 100644 index 00000000..c5adb376 --- /dev/null +++ b/opa-builtins/opa-builtins-regex/src/test/java/io/github/open_policy_agent/opa/ast/builtin/impls/RegexBuiltinsTest.java @@ -0,0 +1,116 @@ +package io.github.open_policy_agent.opa.ast.builtin.impls; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.github.open_policy_agent.opa.ast.builtin.BuiltinError; +import io.github.open_policy_agent.opa.ast.types.RegoBoolean; +import io.github.open_policy_agent.opa.ast.types.RegoInt32; +import io.github.open_policy_agent.opa.ast.types.RegoString; +import io.github.open_policy_agent.opa.ast.types.RegoValue; +import java.util.ServiceLoader; +import org.junit.jupiter.api.Test; + +class RegexBuiltinsTest { + + private final RegexBuiltins builtins = new RegexBuiltins(); + + /** + * The provider entry in META-INF/services was commented out, so none of these builtins were + * reachable for consumers even though the module was documented as supported. + */ + @Test + void providerIsDiscoverableViaServiceLoader() { + boolean found = + ServiceLoader.load( + io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider.class, + RegexBuiltins.class.getClassLoader()) + .stream() + .anyMatch(p -> p.type().equals(RegexBuiltins.class)); + + assertTrue(found, "RegexBuiltins should be registered as a BuiltinProvider"); + } + + @Test + void registersAllRegexBuiltins() { + assertTrue( + builtins + .builtins() + .keySet() + .containsAll( + java.util.List.of( + "regex.match", + "regex.is_valid", + "regex.split", + "regex.find_n", + "regex.find_all_string_submatch_n", + "regex.replace", + "regex.template_match", + "regex.globs_match"))); + } + + // regex.match mirrors Go's regexp.MatchString, which searches rather than matching the whole + // input. An unanchored pattern therefore matches a substring. + @Test + void matchSearchesRatherThanRequiringFullInput() { + assertEquals(RegoBoolean.TRUE, match("", "x")); + assertEquals(RegoBoolean.TRUE, match("b", "abc")); + assertEquals(RegoBoolean.TRUE, match("^[a-z]+\\[[0-9]+\\]$", "foo[1]")); + } + + @Test + void matchHonoursAnchors() { + assertEquals(RegoBoolean.TRUE, match("^$", "")); + assertEquals(RegoBoolean.FALSE, match("^$", "something")); + assertEquals(RegoBoolean.FALSE, match("^b", "abc")); + } + + // An invalid pattern is a builtin error in OPA, not an unchecked PatternSyntaxException. + @Test + void matchRaisesBuiltinErrorForInvalidPattern() { + assertThrows(BuiltinError.class, () -> match("$^[[[", "something")); + } + + @Test + void replaceRaisesBuiltinErrorForInvalidPattern() { + RegoValue[] args = {new RegoString("foo"), new RegoString("["), new RegoString("$1")}; + + assertThrows(BuiltinError.class, () -> builtins.replace(null, args)); + } + + @Test + void splitRaisesBuiltinErrorForInvalidPattern() { + RegoValue[] args = {new RegoString("["), new RegoString("foo")}; + + assertThrows(BuiltinError.class, () -> builtins.split(null, args)); + } + + @Test + void findRaisesBuiltinErrorForInvalidPattern() { + RegoValue[] args = {new RegoString("["), new RegoString("foo"), RegoInt32.of(-1)}; + + assertThrows(BuiltinError.class, () -> builtins.find(null, args)); + } + + @Test + void findAllStringSubmatchRaisesBuiltinErrorForInvalidPattern() { + RegoValue[] args = {new RegoString("["), new RegoString("foo"), RegoInt32.of(-1)}; + + assertThrows(BuiltinError.class, () -> builtins.findSubstringMatch(null, args)); + } + + @Test + void templateMatchRaisesBuiltinErrorForInvalidPattern() { + RegoValue[] args = { + new RegoString("{[}"), new RegoString("foo"), new RegoString("{"), new RegoString("}") + }; + + assertThrows(BuiltinError.class, () -> builtins.templateMatch(null, args)); + } + + private RegoBoolean match(String pattern, String value) { + RegoValue[] args = {new RegoString(pattern), new RegoString(value)}; + return builtins.match(null, args); + } +} diff --git a/opa-evaluator/src/test/resources/compliance/known-missing-builtins.txt b/opa-evaluator/src/test/resources/compliance/known-missing-builtins.txt index 9acde2d1..064f6881 100644 --- a/opa-evaluator/src/test/resources/compliance/known-missing-builtins.txt +++ b/opa-evaluator/src/test/resources/compliance/known-missing-builtins.txt @@ -44,14 +44,6 @@ net.cidr_merge net.lookup_ip_addr # opa-builtins-regex -regex.find_all_string_submatch_n -regex.find_n -regex.globs_match -regex.is_valid -regex.match -regex.replace -regex.split -regex.template_match # --- Not implemented anywhere in the SDK -------------------------------------