ojasvajain commented on code in PR #23696:
URL: https://github.com/apache/kafka/pull/23696#discussion_r4228799698
##########
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsConfigs.java:
##########
@@ -169,11 +185,39 @@ private static void validateConfigs(Map<?, ?> configs) {
}
}
- // Make sure that client match patterns are valid by parsing them.
- if (configs.containsKey(MATCH_CONFIG)) {
+ // Make sure that new or changed match patterns are valid RE2/J syntax.
+ if (configs.containsKey(MATCH_CONFIG) && matchPatternsChanged(configs,
oldProps)) {
List<String> patterns = (List<String>) parsed.get(MATCH_CONFIG);
- // Parse the client matching patterns to validate if the patterns
are valid.
- parseMatchingPatterns(patterns);
+ patterns.forEach(pattern -> {
+ String[] nameValuePair = splitMatchPattern(pattern);
+ String param = nameValuePair[0];
+ String patternValue = nameValuePair[1];
+
+ compileStrict(param, patternValue);
+ });
+ }
+ }
+
+ private static boolean matchPatternsChanged(Map<?, ?> newProps, Map<?, ?>
oldProps) {
+ Object newMatch = newProps.get(MATCH_CONFIG);
+ Object oldMatch = oldProps == null ? null : oldProps.get(MATCH_CONFIG);
+ return !Objects.equals(newMatch, oldMatch);
+ }
+
+ /**
+ * Compiles a single match pattern with RE2/J only.
+ *
+ * See also {@link #compileLenient(String, String, String)}.
+ *
+ * @throws InvalidConfigurationException if the pattern is not valid RE2/J
syntax
+ */
+ private static ClientMatchPattern compileStrict(String param, String
patternValue) {
+ try {
+ return ClientMatchPattern.ofRe2(Pattern.compile(patternValue));
+ } catch (PatternSyntaxException e) {
+ throw new InvalidConfigurationException(
+ String.format("Client match pattern `%s=%s` is not a valid
regular expression: %s.",
+ param, patternValue, e.getDescription()));
}
}
Review Comment:
Happy to hear your thoughts if you feel a different behaviour would be more
suitable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
