You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by ex...@apache.org on 2022/06/14 16:35:56 UTC
[nifi] branch main updated: NIFI-5378 Prevent duplicate keys with different values in nifi.properties
This is an automated email from the ASF dual-hosted git repository.
exceptionfactory pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/main by this push:
new 0f5d8517b5 NIFI-5378 Prevent duplicate keys with different values in nifi.properties
0f5d8517b5 is described below
commit 0f5d8517b526f74e94b989b11c0eb68f33acf43d
Author: Mark Bean <ma...@gmail.com>
AuthorDate: Thu May 19 21:22:04 2022 +0000
NIFI-5378 Prevent duplicate keys with different values in nifi.properties
This closes #6061
Signed-off-by: David Handermann <ex...@apache.org>
---
.../nifi/properties/NiFiPropertiesLoader.java | 61 ++++++++++++++++++----
.../nifi/properties/NiFiPropertiesLoaderTest.java | 26 +++++++--
....nifi.properties => duplicates.nifi.properties} | 4 +-
.../resources/properties/conf/flow.nifi.properties | 3 ++
4 files changed, 80 insertions(+), 14 deletions(-)
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
index c2958b4acb..d893e8fd01 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
@@ -36,6 +36,7 @@ import java.nio.file.Path;
import java.nio.file.Paths;
import java.security.SecureRandom;
import java.util.Base64;
+import java.util.HashSet;
import java.util.List;
import java.util.Properties;
import java.util.Set;
@@ -122,20 +123,29 @@ public class NiFiPropertiesLoader {
}
logger.info("Loading Application Properties [{}]", file);
- final Properties properties = new Properties();
- try (final InputStream inputStream = new BufferedInputStream(new FileInputStream(file))) {
- properties.load(inputStream);
-
- final Set<String> keys = properties.stringPropertyNames();
- for (final String key : keys) {
- final String property = properties.getProperty(key);
- properties.setProperty(key, property.trim());
- }
+ final DuplicateDetectingProperties rawProperties = new DuplicateDetectingProperties();
- return new ProtectedNiFiProperties(properties);
+ try (final InputStream inputStream = new BufferedInputStream(new FileInputStream(file))) {
+ rawProperties.load(inputStream);
} catch (final Exception e) {
throw new RuntimeException(String.format("Loading Application Properties [%s] failed", file), e);
}
+
+ if (!rawProperties.redundantKeySet().isEmpty()) {
+ logger.warn("Duplicate property keys with the same value were detected in the properties file: {}", String.join(", ", rawProperties.redundantKeySet()));
+ }
+ if (!rawProperties.duplicateKeySet().isEmpty()) {
+ throw new IllegalArgumentException("Duplicate property keys with different values were detected in the properties file: " + String.join(", ", rawProperties.duplicateKeySet()));
+ }
+
+ final Properties properties = new Properties();
+ final Set<String> keys = rawProperties.stringPropertyNames();
+ for (final String key : keys) {
+ final String property = rawProperties.getProperty(key);
+ properties.setProperty(key, property.trim());
+ }
+
+ return new ProtectedNiFiProperties(properties);
}
/**
@@ -289,4 +299,35 @@ public class NiFiPropertiesLoader {
}
}
}
+
+ private static class DuplicateDetectingProperties extends Properties {
+ // Only need to retain Properties key. This will help prevent possible inadvertent exposure of sensitive Properties value
+ private final Set<String> duplicateKeys = new HashSet<>(); // duplicate key with different values
+ private final Set<String> redundantKeys = new HashSet<>(); // duplicate key with same value
+ public DuplicateDetectingProperties() {
+ super();
+ }
+
+ public Set<String> duplicateKeySet() {
+ return duplicateKeys;
+ }
+
+ public Set<String> redundantKeySet() {
+ return redundantKeys;
+ }
+
+ @Override
+ public Object put(Object key, Object value) {
+ Object existingValue = super.put(key, value);
+ if (existingValue != null) {
+ if (existingValue.toString().equals(value.toString())) {
+ redundantKeys.add(key.toString());
+ return existingValue;
+ } else {
+ duplicateKeys.add(key.toString());
+ }
+ }
+ return value;
+ }
+ }
}
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/java/org/apache/nifi/properties/NiFiPropertiesLoaderTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/java/org/apache/nifi/properties/NiFiPropertiesLoaderTest.java
index a87ef218af..835e1f7129 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/java/org/apache/nifi/properties/NiFiPropertiesLoaderTest.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/java/org/apache/nifi/properties/NiFiPropertiesLoaderTest.java
@@ -25,18 +25,17 @@ import java.net.URL;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class NiFiPropertiesLoaderTest {
private static final String NULL_PATH = null;
private static final String EMPTY_PATH = "/properties/conf/empty.nifi.properties";
-
private static final String FLOW_PATH = "/properties/conf/flow.nifi.properties";
-
private static final String PROTECTED_PATH = "/properties/conf/protected.nifi.properties";
+ private static final String DUPLICATE_PROPERTIES_PATH = "/properties/conf/duplicates.nifi.properties";
private static final String HEXADECIMAL_KEY = "12345678123456788765432187654321";
-
private static final String EXPECTED_PASSWORD = "propertyValue";
@AfterEach
@@ -44,6 +43,27 @@ class NiFiPropertiesLoaderTest {
System.clearProperty(NiFiProperties.PROPERTIES_FILE_PATH);
}
+ @Test
+ void testDuplicateProperties() {
+ final URL resource = NiFiPropertiesLoaderTest.class.getResource(DUPLICATE_PROPERTIES_PATH);
+ assertNotNull(resource);
+
+ final String path = resource.getPath();
+
+ System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, path);
+
+ final NiFiPropertiesLoader loader = NiFiPropertiesLoader.withKey(String.class.getSimpleName());
+
+ Exception exception = assertThrows(IllegalArgumentException.class, () -> {
+ final NiFiProperties properties = loader.get();
+ });
+
+ String expectedMessage = "Duplicate property keys with different values were detected in the properties file: another.duplicate, nifi.flow.configuration.file";
+ String actualMessage = exception.getMessage();
+
+ assertTrue(actualMessage.contains(expectedMessage));
+ }
+
@Test
void testGetPropertiesNotFound() {
final NiFiPropertiesLoader loader = new NiFiPropertiesLoader();
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/flow.nifi.properties b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/duplicates.nifi.properties
similarity index 90%
copy from nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/flow.nifi.properties
copy to nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/duplicates.nifi.properties
index 77241d5bee..1ad45f1617 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/flow.nifi.properties
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/duplicates.nifi.properties
@@ -14,4 +14,6 @@
# limitations under the License.
nifi.flow.configuration.file=./conf/flow.xml.gz
-nifi.flow.configuration.json.file=./conf/flow.json.gz
+nifi.flow.configuration.file=./conf/flow.json.gz
+another.duplicate=foo
+another.duplicate=bar
\ No newline at end of file
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/flow.nifi.properties b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/flow.nifi.properties
index 77241d5bee..eeac375b29 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/flow.nifi.properties
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/flow.nifi.properties
@@ -15,3 +15,6 @@
nifi.flow.configuration.file=./conf/flow.xml.gz
nifi.flow.configuration.json.file=./conf/flow.json.gz
+# duplicate with the same value intentionally added to verify this is valid
+# while bad form, it will generate only a warning, not a critical error
+nifi.flow.configuration.json.file=./conf/flow.json.gz