Skip to content

Commit c72f067

Browse files
typotterdevflow.devflow-routing-intake
andauthored
fix(openfeature): return PARSE_ERROR for invalid regex in flag condition (#11071)
fix(openfeature): return PARSE_ERROR for invalid regex in flag condition test(openfeature): add NOT_MATCHES invalid-regex test and fix comment - Add test case for NOT_MATCHES with invalid regex pattern, which follows the same PatternSyntaxException propagation path as MATCHES - Fix misleading comment on integer-string-variant-flag test case to accurately describe the PARSE_ERROR trigger (unparseable variant value) Merge remote-tracking branch 'origin/master' into typo/parse-error-regex Merge remote-tracking branch 'origin/master' into typo/parse-error-regex Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent b088956 commit c72f067

2 files changed

Lines changed: 59 additions & 8 deletions

File tree

products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.concurrent.TimeUnit;
4040
import java.util.concurrent.atomic.AtomicReference;
4141
import java.util.regex.Pattern;
42+
import java.util.regex.PatternSyntaxException;
4243

4344
class DDEvaluator implements Evaluator, FeatureFlaggingGateway.ConfigListener {
4445

@@ -155,6 +156,8 @@ public <T> ProviderEvaluation<T> evaluate(
155156
.value(defaultValue)
156157
.reason(Reason.DEFAULT.name())
157158
.build();
159+
} catch (final PatternSyntaxException e) {
160+
return error(defaultValue, ErrorCode.PARSE_ERROR, e);
158161
} catch (final NumberFormatException e) {
159162
return error(defaultValue, ErrorCode.TYPE_MISMATCH, e);
160163
} catch (final Exception e) {
@@ -258,12 +261,10 @@ private static boolean evaluateCondition(
258261
}
259262

260263
private static boolean matchesRegex(final Object attributeValue, final Object conditionValue) {
261-
try {
262-
final Pattern pattern = Pattern.compile(String.valueOf(conditionValue));
263-
return pattern.matcher(String.valueOf(attributeValue)).find();
264-
} catch (Exception e) {
265-
return false;
266-
}
264+
// PatternSyntaxException is intentionally not caught here so it propagates to evaluate(),
265+
// which maps it to ErrorCode.PARSE_ERROR.
266+
final Pattern pattern = Pattern.compile(String.valueOf(conditionValue));
267+
return pattern.matcher(String.valueOf(attributeValue)).find();
267268
}
268269

269270
private static boolean isOneOf(final Object attributeValue, final Object conditionValue) {

products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,8 @@ private static List<TestCase<?>> evaluateTestCases() {
316316
.flag("integer-flag")
317317
.targetingKey("user-123")
318318
.result(new Result<>(0.0).reason(ERROR.name()).errorCode(ErrorCode.TYPE_MISMATCH)),
319-
// Variant value type mismatch: INTEGER flag with string variant value
319+
// Variant stores "not-a-number" for an INTEGER flag; Double.parseDouble fails ->
320+
// PARSE_ERROR
320321
new TestCase<>(0)
321322
.flag("integer-string-variant-flag")
322323
.targetingKey("user-123")
@@ -490,7 +491,17 @@ private static List<TestCase<?>> evaluateTestCases() {
490491
.result(
491492
new Result<>("null-handled")
492493
.reason(TARGETING_MATCH.name())
493-
.variant("null-variant")));
494+
.variant("null-variant")),
495+
new TestCase<>("default")
496+
.flag("invalid-regex-flag")
497+
.targetingKey("user-123")
498+
.context("email", "user@example.com")
499+
.result(new Result<>("default").reason(ERROR.name()).errorCode(ErrorCode.PARSE_ERROR)),
500+
new TestCase<>("default")
501+
.flag("invalid-regex-not-matches-flag")
502+
.targetingKey("user-123")
503+
.context("email", "user@example.com")
504+
.result(new Result<>("default").reason(ERROR.name()).errorCode(ErrorCode.PARSE_ERROR)));
494505
}
495506

496507
@MethodSource("evaluateTestCases")
@@ -582,6 +593,8 @@ private ServerConfiguration createTestConfiguration() {
582593
flags.put(
583594
"integer-string-variant-flag",
584595
createSimpleFlag("integer-string-variant-flag", ValueType.INTEGER, "not-a-number", "bad"));
596+
flags.put("invalid-regex-flag", createInvalidRegexFlag());
597+
flags.put("invalid-regex-not-matches-flag", createInvalidRegexNotMatchesFlag());
585598
return new ServerConfiguration(null, null, null, flags);
586599
}
587600

@@ -1267,6 +1280,43 @@ private Flag createCountryRuleFlag() {
12671280
asList(usAllocation, globalAllocation));
12681281
}
12691282

1283+
private Flag createInvalidRegexFlag() {
1284+
final Map<String, Variant> variants = new HashMap<>();
1285+
variants.put("matched", new Variant("matched", "matched-value"));
1286+
1287+
// Condition with an intentionally invalid regex pattern (unclosed bracket)
1288+
final List<ConditionConfiguration> conditions =
1289+
singletonList(new ConditionConfiguration(ConditionOperator.MATCHES, "email", "[invalid"));
1290+
final List<Rule> rules = singletonList(new Rule(conditions));
1291+
final List<Split> splits = singletonList(new Split(emptyList(), "matched", null));
1292+
final Allocation allocation =
1293+
new Allocation("invalid-regex-alloc", rules, null, null, splits, false);
1294+
1295+
return new Flag(
1296+
"invalid-regex-flag", true, ValueType.STRING, variants, singletonList(allocation));
1297+
}
1298+
1299+
private Flag createInvalidRegexNotMatchesFlag() {
1300+
final Map<String, Variant> variants = new HashMap<>();
1301+
variants.put("excluded", new Variant("excluded", "excluded-value"));
1302+
1303+
// Condition with an intentionally invalid regex pattern (unclosed bracket) under NOT_MATCHES
1304+
final List<ConditionConfiguration> conditions =
1305+
singletonList(
1306+
new ConditionConfiguration(ConditionOperator.NOT_MATCHES, "email", "[invalid"));
1307+
final List<Rule> rules = singletonList(new Rule(conditions));
1308+
final List<Split> splits = singletonList(new Split(emptyList(), "excluded", null));
1309+
final Allocation allocation =
1310+
new Allocation("invalid-regex-not-matches-alloc", rules, null, null, splits, false);
1311+
1312+
return new Flag(
1313+
"invalid-regex-not-matches-flag",
1314+
true,
1315+
ValueType.STRING,
1316+
variants,
1317+
singletonList(allocation));
1318+
}
1319+
12701320
private static Map<String, Object> mapOf(final Object... props) {
12711321
final Map<String, Object> result = new HashMap<>(props.length << 1);
12721322
int index = 0;

0 commit comments

Comments
 (0)