eaolson commented on a change in pull request #4854: URL: https://github.com/apache/nifi/pull/4854#discussion_r586924459
########## File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java ########## @@ -174,6 +177,18 @@ ProtectedNiFiProperties readProtectedPropertiesFromDisk(File file) { rawProperties.load(inStream); logger.info("Loaded {} properties from {}", rawProperties.size(), file.getAbsolutePath()); + // Trim whitespace from each property. If property is multi-line, remove anything after the first line break. Review comment: Allow me to counterpoint: 1. Leading whitespace on the value is taken care of by the underlying [java.util.Properties ](https://docs.oracle.com/javase/8/docs/api/java/util/Properties.html#load-java.io.Reader-). That's already being stripped out and this pull request doesn't change that. So to support leading whitespace would mean writing a whole new reader. 2. Similarly, trailing whitespace on the _key_ is already taken care of by the Properties instance. So supporting leading whitespace would mean the possibility of "foo=bar" and "foo = bar" (which are currently equivalent) being different. And "foo =bar" would be equivalent to the former and not the latter. If nothing else, "supportLeadingWhitespace=false" should be the default, because you just know someone out there has configured their properties with spaces around the "=" and it's working now. 3. Supporting surrounding quote characters just changes the situation from "no trailing whitespace" to "no leading or trailing quote marks", though I supposed you could have a quote inside the value if you escape it somehow. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org