[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6061: NIFI-5378 prevent duplicate keys with different values in nifi.p…

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

exceptionfactory commented on code in PR #6061:
URL: https://github.com/apache/nifi/pull/6061#discussion_r879742311


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java:
##########
@@ -211,6 +220,49 @@ public NiFiProperties get() {
         return instance;
     }
 
+    private void checkForDuplicates(File file) {
+        Map<String, String> properties = new HashMap<>();
+        Map<String, Set<String>> duplicateProperties = new HashMap<>();
+        Pattern keyPattern = Pattern.compile("([^!#=][^=]*)=(.*)");
+
+        if (file == null) {
+            throw new IllegalArgumentException("NiFi properties file missing or unreadable");
+        }
+        // Scan the properties file for duplicate keys
+        try (BufferedReader bufferedReader = new BufferedReader(new FileReader(file))) {
+            String line;
+            while ((line = bufferedReader.readLine()) != null) {
+                Matcher matcher = keyPattern.matcher(line);
+
+                if (matcher.matches()) {
+                    String key = matcher.group(1);
+                    String value = matcher.group(2);
+
+                    String existingValue = properties.put(key, value);
+
+                    if (existingValue != null) {
+                        if (existingValue.equals(value)) {
+                            logger.warn("Duplicate keys found for key '{}', but values are the same.", key);
+                        } else {
+                            Set<String> dupes = duplicateProperties.computeIfAbsent(key, k -> new HashSet<>(Collections.singleton(existingValue)));
+                            dupes.add(value);
+                        }
+                    }
+                }
+            }
+        } catch (IOException ioe) {
+            throw new IllegalArgumentException("Cannot load properties file [" + file.getAbsolutePath() + "] due to "
+                    + ioe.getLocalizedMessage(), ioe);
+        }
+
+        if (!duplicateProperties.isEmpty()) {
+            duplicateProperties.keySet().forEach(
+                    k -> logger.error("Duplicate entries found for key '{}'. Conflicting values are: {}", k, duplicateProperties.get(k))

Review Comment:
   @markobean One other very important thing to note is that these property values should never be logged. Various values in `nifi.properties` are sensitive, such as keystore passwords. In addition to the parsing changes noted, property values should not be logged or included in error messages.



-- 
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: issues-unsubscribe@nifi.apache.org

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