ojasvajain commented on code in PR #23696:
URL: https://github.com/apache/kafka/pull/23696#discussion_r4228707172
##########
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:
To support backward compatibility, I have gone ahead with the following
behaviour - If user updates an existing subscription, without updating the
"match" field, the user should not get an error if any of the patterns is not
supported in RE2. This is why we need the old configs of the subscription, to
check if "match" field was changed in the new config. If yes, we compile the
new patterns using RE2, if not, we skip it.
--
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]
