chihsuan commented on code in PR #10840:
URL: https://github.com/apache/ozone/pull/10840#discussion_r3630364333
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/conf/ConfigurationFieldsTests.java:
##########
@@ -668,6 +677,35 @@ public void testDefaultValueCollision() {
LOG.info("Checked {} default values for collision.", valuesChecked);
}
+ }
+
+ /**
+ * Verifies no default value in the XML embeds a line break, which would
+ * corrupt the runtime string (see HDDS-8046). Legitimate cases opt out via
+ * {@link #xmlPropsAllowedToContainNewline}.
+ */
+ @Test
+ public void testXmlValuesHaveNoEmbeddedNewlines() {
+ Set<String> xmlValuesWithNewlines = new HashSet<>();
+ for (Map.Entry<String, String> entry : xmlKeyValueMap.entrySet()) {
+ String value = entry.getValue();
+ if (value == null) {
+ continue;
+ }
+ if (xmlPropsAllowedToContainNewline.contains(entry.getKey())) {
+ LOG.info("XML Property: {} AllowedToContainNewline", entry.getKey());
+ continue;
+ }
+ if (value.indexOf('\n') != -1 || value.indexOf('\r') != -1) {
+ xmlValuesWithNewlines.add(entry.getKey());
+ }
+ }
+ assertTrue(xmlValuesWithNewlines.isEmpty(),
+ "These properties in " + xmlFilename + " have an embedded line break
in "
+ + "their <value>, which corrupts the runtime string: " +
xmlValuesWithNewlines
Review Comment:
nit: There is an extra space after `runtime string:`.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/conf/ConfigurationFieldsTests.java:
##########
@@ -668,6 +677,35 @@ public void testDefaultValueCollision() {
LOG.info("Checked {} default values for collision.", valuesChecked);
}
+ }
+
+ /**
+ * Verifies no default value in the XML embeds a line break, which would
+ * corrupt the runtime string (see HDDS-8046). Legitimate cases opt out via
+ * {@link #xmlPropsAllowedToContainNewline}.
+ */
+ @Test
+ public void testXmlValuesHaveNoEmbeddedNewlines() {
+ Set<String> xmlValuesWithNewlines = new HashSet<>();
+ for (Map.Entry<String, String> entry : xmlKeyValueMap.entrySet()) {
Review Comment:
Would it make sense to check newlines before applying
`xmlPropsToSkipCompare` and `xmlPrefixToSkipCompare`? therwise, properties
excluded from field comparison also skip this check, even if they are not
listed in `xmlPropsAllowedToContainNewline`.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/conf/ConfigurationFieldsTests.java:
##########
@@ -668,6 +677,35 @@ public void testDefaultValueCollision() {
LOG.info("Checked {} default values for collision.", valuesChecked);
}
+ }
+
+ /**
+ * Verifies no default value in the XML embeds a line break, which would
+ * corrupt the runtime string (see HDDS-8046). Legitimate cases opt out via
+ * {@link #xmlPropsAllowedToContainNewline}.
+ */
+ @Test
+ public void testXmlValuesHaveNoEmbeddedNewlines() {
+ Set<String> xmlValuesWithNewlines = new HashSet<>();
Review Comment:
nit: Would a `TreeSet` be useful here to keep the failure output
deterministic? The sibling checks in this class also use it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
