[GitHub] [kafka] dengziming commented on a diff in pull request #12109: KAFKA-13863: Prevent null config value when create topic in KRaft mode

[GitBox <gi...@apache.org>]

dengziming commented on code in PR #12109:
URL: https://github.com/apache/kafka/pull/12109#discussion_r873158984


##########
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java:
##########
@@ -773,10 +773,18 @@ static void validateNewTopicNames(Map<String, ApiError> topicErrors,
         for (CreatableTopic topic : topics) {
             if (topicErrors.containsKey(topic.name())) continue;
             Map<String, Entry<OpType, String>> topicConfigs = new HashMap<>();
+            List<String> nullConfigs = new ArrayList<>();
             for (CreateTopicsRequestData.CreateableTopicConfig config : topic.configs()) {
-                topicConfigs.put(config.name(), new SimpleImmutableEntry<>(SET, config.value()));
+                if (config.value() == null) {
+                    nullConfigs.add(config.name());
+                } else {
+                    topicConfigs.put(config.name(), new SimpleImmutableEntry<>(SET, config.value()));
+                }
             }
-            if (!topicConfigs.isEmpty()) {
+            if (!nullConfigs.isEmpty()) {
+                topicErrors.put(topic.name(), new ApiError(Errors.INVALID_REQUEST,
+                    "Null value not supported for topic configs : " + String.join(",", nullConfigs)));

Review Comment:
   This is copied from `ZkAdminManager`, I think it's better to optimize it so I also changed `ZkAdminManager`.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org