From 1cef5325adf4c277930ec5fd908ea785513d16c2 Mon Sep 17 00:00:00 2001 From: Evanston Zhou <84255167+ezhou413@users.noreply.github.com> Date: Tue, 22 Jul 2025 13:16:58 -0500 Subject: [PATCH] KAFKA-19213 Client ignores default properties (#20134) https://issues.apache.org/jira/browse/KAFKA-19213 Fixes a bug where creating a producer/consumer using a `Properties` object created using the `Properties(Properties defaults)` constructor will ignore the default properties. Reviewers: Kirk True , TaiJuWu , Jhen-Yung Hsu , Chia-Ping Tsai --- .../org/apache/kafka/common/utils/Utils.java | 23 ++- .../clients/producer/KafkaProducerTest.java | 2 +- .../apache/kafka/common/utils/UtilsTest.java | 159 ++++++++++++++++++ 3 files changed, 182 insertions(+), 2 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/utils/Utils.java b/clients/src/main/java/org/apache/kafka/common/utils/Utils.java index 8aa21f7377f..7e8b990542d 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/Utils.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/Utils.java @@ -64,6 +64,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.EnumSet; +import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -1469,7 +1470,24 @@ public final class Utils { * @return a map including all elements in properties */ public static Map propsToMap(Properties properties) { - return castToStringObjectMap(properties); + // This try catch block is to handle the case when the Properties object has non-String keys + // when calling the propertyNames() method. This is a workaround for the lack of a method that + // returns all properties including defaults and does not attempt to convert all keys to Strings. + Enumeration enumeration; + try { + enumeration = properties.propertyNames(); + } catch (ClassCastException e) { + throw new ConfigException("One or more keys is not a string."); + } + Map map = new HashMap<>(); + while (enumeration.hasMoreElements()) { + String key = (String) enumeration.nextElement(); + // properties.get(key) returns null for defaults, but properties.getProperty(key) returns null for + // non-string values. A combination of the two methods is used to cover all cases + Object value = (properties.get(key) != null) ? properties.get(key) : properties.getProperty(key); + map.put(key, value); + } + return map; } /** @@ -1479,6 +1497,9 @@ public final class Utils { * @throws ConfigException if any key is not a String */ public static Map castToStringObjectMap(Map inputMap) { + if (inputMap instanceof Properties) { + return propsToMap((Properties) inputMap); + } Map map = new HashMap<>(inputMap.size()); for (Map.Entry entry : inputMap.entrySet()) { if (entry.getKey() instanceof String) { diff --git a/clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java b/clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java index 8460d0f4c5f..24c1574f3f9 100644 --- a/clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java @@ -579,7 +579,7 @@ public class KafkaProducerTest { ConfigException ce = assertThrows( ConfigException.class, () -> new KafkaProducer<>(props, new StringSerializer(), new StringSerializer())); - assertTrue(ce.getMessage().contains("not string key"), "Unexpected exception message: " + ce.getMessage()); + assertTrue(ce.getMessage().contains("One or more keys is not a string."), "Unexpected exception message: " + ce.getMessage()); } @Test diff --git a/clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java b/clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java index 4220e84b7cc..bd8d5f73888 100755 --- a/clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java +++ b/clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java @@ -896,12 +896,171 @@ public class UtilsTest { assertValue(Collections.emptyMap()); } + @Test + public void testPropsToMapNonStringKey() { + ConfigException ce = assertThrows(ConfigException.class, () -> { + Properties props = new Properties(); + props.put(1, "value"); + Utils.propsToMap(props); + }); + assertTrue(ce.getMessage().contains("One or more keys is not a string.")); + + ce = assertThrows(ConfigException.class, () -> { + Properties props = new Properties(); + props.put(true, "value"); + props.put('a', "value"); + Utils.propsToMap(props); + }); + assertEquals("One or more keys is not a string.", ce.getMessage()); + } + + @Test + public void testPropsToMapWithDefaults() { + Properties defaultProperties = new Properties(); + defaultProperties.setProperty("DefaultKey1", "DefaultValue1"); + defaultProperties.setProperty("DefaultKey2", "DefaultValue2"); + + Properties actualProperties = new Properties(defaultProperties); + actualProperties.setProperty("ActualKey1", "ActualValue1"); + actualProperties.setProperty("ActualKey2", "ActualValue2"); + + final Map mapProperties = Utils.propsToMap(actualProperties); + + Map expectedMap = new HashMap<>(); + expectedMap.put("DefaultKey1", "DefaultValue1"); + expectedMap.put("DefaultKey2", "DefaultValue2"); + expectedMap.put("ActualKey1", "ActualValue1"); + expectedMap.put("ActualKey2", "ActualValue2"); + + assertEquals(expectedMap, mapProperties); + } + + @Test + public void testPropsToMapWithDefaultsAndSameKey() { + Properties defaultProperties = new Properties(); + defaultProperties.setProperty("DefaultKey1", "DefaultValue1"); + defaultProperties.setProperty("DefaultKey2", "DefaultValue2"); + + Properties actualProperties = new Properties(defaultProperties); + actualProperties.setProperty("DefaultKey1", "ActualValue1"); + actualProperties.setProperty("ActualKey2", "ActualValue2"); + + final Map mapProperties = Utils.propsToMap(actualProperties); + + Map expectedMap = new HashMap<>(); + expectedMap.put("DefaultKey1", "ActualValue1"); + expectedMap.put("DefaultKey2", "DefaultValue2"); + expectedMap.put("ActualKey2", "ActualValue2"); + + assertEquals(expectedMap, mapProperties); + } + private static void assertValue(Object value) { Properties props = new Properties(); props.put("key", value); assertEquals(Utils.propsToMap(props).get("key"), value); } + @Test + public void testCastToStringObjectMap() { + Map map = new HashMap<>(); + map.put("key1", "value1"); + map.put("key2", 1); + + Map expectedMap = new HashMap<>(); + expectedMap.put("key1", "value1"); + expectedMap.put("key2", 1); + + assertEquals(map, expectedMap); + } + + @Test + public void testCastToStringObjectMapNonStringKey() { + ConfigException ce = assertThrows(ConfigException.class, () -> { + Map map = new HashMap<>(); + map.put(1, "value"); + Utils.castToStringObjectMap(map); + }); + assertTrue(ce.getMessage().contains("Key must be a string.")); + + ce = assertThrows(ConfigException.class, () -> { + Map map = new HashMap<>(); + map.put(true, "value"); + map.put('a', "value"); + Utils.castToStringObjectMap(map); + }); + assertTrue(ce.getMessage().contains("Key must be a string.")); + } + + @Test + public void testCastToStringObjectMapPropertiesAsInput() { + Properties props = new Properties(); + props.put("key1", "value1"); + props.put("key2", "value2"); + + Map expectedMap = new HashMap<>(); + expectedMap.put("key1", "value1"); + expectedMap.put("key2", "value2"); + + assertEquals(expectedMap, Utils.castToStringObjectMap(props)); + assertEquals(Utils.propsToMap(props), Utils.castToStringObjectMap(props)); + } + + @Test + public void testCastToStringObjectMapPropertiesNonStringKey() { + ConfigException ce = assertThrows(ConfigException.class, () -> { + Properties props = new Properties(); + props.put(1, "value"); + Utils.castToStringObjectMap(props); + }); + assertEquals("One or more keys is not a string.", ce.getMessage()); + + ce = assertThrows(ConfigException.class, () -> { + Properties props = new Properties(); + props.put(true, "value"); + props.put('a', "value"); + Utils.castToStringObjectMap(props); + }); + assertEquals("One or more keys is not a string.", ce.getMessage()); + } + + @Test + public void testCastToStringObjectMapPropertiesWithDefaults() { + Properties defaultProperties = new Properties(); + defaultProperties.setProperty("DefaultKey1", "DefaultValue1"); + defaultProperties.setProperty("DefaultKey2", "DefaultValue2"); + + Properties actualProperties = new Properties(defaultProperties); + actualProperties.setProperty("ActualKey1", "ActualValue1"); + actualProperties.setProperty("ActualKey2", "ActualValue2"); + + Map expectedMap = new HashMap<>(); + expectedMap.put("DefaultKey1", "DefaultValue1"); + expectedMap.put("DefaultKey2", "DefaultValue2"); + expectedMap.put("ActualKey1", "ActualValue1"); + expectedMap.put("ActualKey2", "ActualValue2"); + + assertEquals(expectedMap, Utils.castToStringObjectMap(actualProperties)); + } + + @Test + public void testCastToStringObjectMapPropertiesWithDefaultsAndSameKey() { + Properties defaultProperties = new Properties(); + defaultProperties.setProperty("DefaultKey1", "DefaultValue1"); + defaultProperties.setProperty("DefaultKey2", "DefaultValue2"); + + Properties actualProperties = new Properties(defaultProperties); + actualProperties.setProperty("DefaultKey1", "ActualValue1"); + actualProperties.setProperty("ActualKey2", "ActualValue2"); + + Map expectedMap = new HashMap<>(); + expectedMap.put("DefaultKey1", "ActualValue1"); + expectedMap.put("DefaultKey2", "DefaultValue2"); + expectedMap.put("ActualKey2", "ActualValue2"); + + assertEquals(expectedMap, Utils.castToStringObjectMap(actualProperties)); + } + @Test public void testCloseAllQuietly() { AtomicReference exception = new AtomicReference<>();