Skip to content

Commit c978b86

Browse files
garyrussellartembilan
authored andcommitted
GH-2345: Fix Possible NPE in KafkaAdmin
Resolves #2345 I haven't been able to reproduce it since, but I saw one occasion where the existing config property returned null. Protect against an NPE in that case and add the property to the change candidates. Also move the map entry out of the for loop. **cherry-pick to 2.9.x, 2.8.x**
1 parent 1026edd commit c978b86

File tree

2 files changed

+23
-8
lines changed

2 files changed

+23
-8
lines changed

spring-kafka/src/main/java/org/springframework/kafka/core/KafkaAdmin.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,12 @@ private Map<ConfigResource, List<ConfigEntry>> checkTopicsForConfigMismatches(
301301
if (topicOptional.isPresent() && topicOptional.get().configs() != null) {
302302
for (Map.Entry<String, String> desiredConfigParameter : topicOptional.get().configs().entrySet()) {
303303
ConfigEntry actualConfigParameter = topicConfig.getValue().get(desiredConfigParameter.getKey());
304-
if (!actualConfigParameter.value().equals(desiredConfigParameter.getValue())) {
304+
if (!desiredConfigParameter.getValue().equals(actualConfigParameter.value())) {
305305
configMismatchesEntries.add(actualConfigParameter);
306306
}
307-
308-
if (configMismatchesEntries.size() > 0) {
309-
configMismatches.put(topicConfig.getKey(), configMismatchesEntries);
310-
}
307+
}
308+
if (configMismatchesEntries.size() > 0) {
309+
configMismatches.put(topicConfig.getKey(), configMismatchesEntries);
311310
}
312311
}
313312
}

spring-kafka/src/test/java/org/springframework/kafka/core/KafkaAdminTests.java

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,17 +152,25 @@ public void testAddTopicsAndAddPartitions() throws Exception {
152152
&& configResourceConfigMap.get(new ConfigResource(Type.TOPIC, "mismatchconfig")).get("retention.ms").value().equals("11");
153153
});
154154

155-
this.admin.createOrModifyTopics(mismatchconfig);
155+
this.admin.createOrModifyTopics(mismatchconfig,
156+
TopicBuilder.name("noConfigAddLater")
157+
.partitions(2)
158+
.replicas(1)
159+
.config("retention.ms", "1000")
160+
.build());
156161

157162
await().until(() -> {
158163
DescribeConfigsResult describeConfigsResult = this.adminClient
159-
.describeConfigs(List.of(new ConfigResource(Type.TOPIC, "mismatchconfig")));
164+
.describeConfigs(List.of(new ConfigResource(Type.TOPIC, "mismatchconfig"),
165+
new ConfigResource(Type.TOPIC, "noConfigAddLater")));
160166
Map<ConfigResource, org.apache.kafka.clients.admin.Config> configResourceConfigMap = describeConfigsResult.all()
161167
.get();
162168
return configResourceConfigMap.get(new ConfigResource(Type.TOPIC, "mismatchconfig"))
163169
.get("retention.bytes").value().equals("1024")
164170
&& configResourceConfigMap.get(new ConfigResource(Type.TOPIC, "mismatchconfig"))
165-
.get("retention.ms").value().equals("1111");
171+
.get("retention.ms").value().equals("1111")
172+
&& configResourceConfigMap.get(new ConfigResource(Type.TOPIC, "noConfigAddLater"))
173+
.get("retention.ms").value().equals("1000");
166174
});
167175
}
168176

@@ -309,6 +317,14 @@ public NewTopic mismatchconfig() {
309317
.build();
310318
}
311319

320+
@Bean
321+
public NewTopic noConfigAddLater() {
322+
return TopicBuilder.name("noConfigAddLater")
323+
.partitions(2)
324+
.replicas(1)
325+
.build();
326+
}
327+
312328
@Bean
313329
public NewTopics topics456() {
314330
return new NewTopics(

0 commit comments

Comments
 (0)