Make nomulus compatible with Java 11 (#649)

* Make nomulus compatible with Java 11

This fixes the double-spacing bug with logged EPP XML on App Engine that started
appearing after App Engine switching from using Java 8 to Java 11. Java 9 made
some changes to XML Transformer classes that needed a little bit of work to
accommodate.

This also fixes the unit tests that were failing in Java 11 (all of which were
related to said XML Transformer changes).

* Make code review changes

core/src/main/java/google/registry/xml/XmlTransformer.java

@@ -297,7 +297,7 @@ public class XmlTransformer {
     return marshaller;
   }
 
-  /** Pretty print xml. */
+  /** Pretty print XML. */
   public static String prettyPrint(String xmlString) {
     StringWriter prettyXml = new StringWriter();
     try {
@@ -308,13 +308,18 @@ public class XmlTransformer {
       transformer.transform(
           new StreamSource(new StringReader(xmlString)),
           new StreamResult(prettyXml));
-      return prettyXml.toString();
+
+      // Remove whitespace-only/blank lines (which the XMLTransformer in Java 9 and up sometimes
+      // adds depending on input format). Surprisingly, this is the least bad solution. See:
+      // https://stackoverflow.com/questions/58478632/how-to-avoid-extra-blank-lines-in-xml-generation-with-java
+      // Note that a simple regex replace is waaaaay more performant than using an XSLT.
+      return prettyXml.toString().replaceAll("\\n\\s*\\n", "\n");
     } catch (TransformerException e) {
       return xmlString;  // We couldn't prettify it, but that's ok; fail gracefully.
     }
   }
 
-  /** Pretty print xml bytes. */
+  /** Pretty print XML bytes. */
   public static String prettyPrint(byte[] xmlBytes) {
     return prettyPrint(new String(xmlBytes, UTF_8));
   }

core/src/test/java/google/registry/flows/EppXmlSanitizerTest.java

@@ -17,6 +17,7 @@ package google.registry.flows;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.flows.EppXmlSanitizer.sanitizeEppXml;
 import static google.registry.testing.TestDataHelper.loadBytes;
+import static google.registry.xml.XmlTestUtils.assertXmlEqualsIgnoreHeader;
 import static java.nio.charset.StandardCharsets.UTF_16LE;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
@@ -37,13 +38,11 @@ public class EppXmlSanitizerTest {
   public void testSanitize_noSensitiveData_noop() throws Exception {
     byte[] inputXmlBytes = loadBytes(getClass(), "host_create.xml").read();
     String expectedXml = UTF8_HEADER + new String(inputXmlBytes, UTF_8);
-
+    assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXmlBytes));
-    String sanitizedXml = sanitizeEppXml(inputXmlBytes);
-    assertThat(sanitizedXml).isEqualTo(expectedXml);
   }
 
   @Test
-  public void testSanitize_loginPasswords_sanitized() {
+  public void testSanitize_loginPasswords_sanitized() throws Exception {
     String inputXml =
         new EppLoader(
                 this,
@@ -57,13 +56,11 @@ public class EppXmlSanitizerTest {
                     "login_update_password.xml",
                     ImmutableMap.of("PW", "*******", "NEWPW", "*****"))
                 .getEppXml();
-
+    assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXml.getBytes(UTF_8)));
-    String sanitizedXml = sanitizeEppXml(inputXml.getBytes(UTF_8));
-    assertThat(sanitizedXml).isEqualTo(expectedXml);
   }
 
   @Test
-  public void testSanitize_loginPasswordTagWrongCase_sanitized() {
+  public void testSanitize_loginPasswordTagWrongCase_sanitized() throws Exception {
     String inputXml =
         new EppLoader(
                 this, "login_wrong_case.xml", ImmutableMap.of("PW", "oldpass", "NEWPW", "newPw"))
@@ -75,9 +72,7 @@ public class EppXmlSanitizerTest {
                     "login_wrong_case.xml",
                     ImmutableMap.of("PW", "*******", "NEWPW", "*****"))
                 .getEppXml();
-
+    assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXml.getBytes(UTF_8)));
-    String sanitizedXml = sanitizeEppXml(inputXml.getBytes(UTF_8));
-    assertThat(sanitizedXml).isEqualTo(expectedXml);
   }
 
   @Test
@@ -86,9 +81,7 @@ public class EppXmlSanitizerTest {
     String expectedXml =
         UTF8_HEADER
             + new EppLoader(this, "contact_info_sanitized.xml", ImmutableMap.of()).getEppXml();
-
+    assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXmlBytes));
-    String sanitizedXml = sanitizeEppXml(inputXmlBytes);
-    assertThat(sanitizedXml).isEqualTo(expectedXml);
   }
 
   @Test
@@ -99,15 +92,13 @@ public class EppXmlSanitizerTest {
             + new EppLoader(
                     this, "contact_info_from_create_response_sanitized.xml", ImmutableMap.of())
                 .getEppXml();
-
+    assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXmlBytes));
-    String sanitizedXml = sanitizeEppXml(inputXmlBytes);
-    assertThat(sanitizedXml).isEqualTo(expectedXml);
   }
 
   @Test
-  public void testSanitize_emptyElement_transformedToLongForm() {
+  public void testSanitize_emptyElement_transformedToLongForm() throws Exception {
     byte[] inputXmlBytes = "<pw/>".getBytes(UTF_8);
-    assertThat(sanitizeEppXml(inputXmlBytes)).isEqualTo(UTF8_HEADER + "<pw></pw>\n");
+    assertXmlEqualsIgnoreHeader("<pw></pw>", sanitizeEppXml(inputXmlBytes));
   }
 
   @Test
@@ -118,10 +109,9 @@ public class EppXmlSanitizerTest {
   }
 
   @Test
-  public void testSanitize_unicode_hasCorrectCharCount() {
+  public void testSanitize_unicode_hasCorrectCharCount() throws Exception {
     byte[] inputXmlBytes = "<pw>\u007F\u4E43x</pw>".getBytes(UTF_8);
-    String expectedXml = UTF8_HEADER + "<pw>C**</pw>\n";
+    assertXmlEqualsIgnoreHeader("<pw>C**</pw>", sanitizeEppXml(inputXmlBytes));
-    assertThat(sanitizeEppXml(inputXmlBytes)).isEqualTo(expectedXml);
   }
 
   @Test
@@ -136,6 +126,13 @@ public class EppXmlSanitizerTest {
     // used, the XMLEventReader in sanitizer may resolve it to an endian-specific one.
     String inputXml = "<?xml version=\"1.0\" encoding=\"UTF-16LE\"?><p>\u03bc</p>\n";
     String sanitizedXml = sanitizeEppXml(inputXml.getBytes(UTF_16LE));
-    assertThat(sanitizedXml).isEqualTo(inputXml);
+
+    // As of Java 9, standalone is always written to the XML header and is defaulted to "no" if not
+    // otherwise specified. We don't care about that for this specific test, and since we want to be
+    // compatible with both Java 8 and Java 11 (for now), we strip the standalone="no" attribute if
+    // it is present prior to doing the string comparison.
+    // TODO(java11): Remove this stripping and do a full assertion once we no longer care about
+    //               maintaining Java 8 build compatibility.
+    assertThat(sanitizedXml.replace(" standalone=\"no\"", "")).isEqualTo(inputXml);
   }
 }

core/src/test/java/google/registry/flows/FlowTestCase.java

@@ -114,7 +114,7 @@ public abstract class FlowTestCase<F extends Flow> extends ShardableTestCase {
   }
 
   protected void setEppInput(String inputFilename) {
-    eppLoader = new EppLoader(this, inputFilename, ImmutableMap.of());
+    eppLoader = new EppLoader(this, inputFilename);
   }
 
   protected void setEppInput(String inputFilename, Map<String, String> substitutions) {

core/src/test/java/google/registry/xml/XmlTestUtils.java

@@ -41,16 +41,40 @@ import org.json.XML;
 /** Helper class for unit tests that need XML. */
 public class XmlTestUtils {
 
-  public static void assertXmlEquals(
+  /**
-      String expected, String actual, String... ignoredPaths) throws Exception {
+   * Asserts that the two XML strings match.
+   *
+   * <p>Note that the actual XML must start with a UTF-8 standalone XML header, but the expected
+   * XML has no such restriction (and typically lacks the header entirely).
+   */
+  public static void assertXmlEquals(String expected, String actual, String... ignoredPaths)
+      throws Exception {
     assertXmlEqualsWithMessage(expected, actual, "", ignoredPaths);
   }
 
+  /**
+   * Asserts that the two XML strings match, but ignoring the XML header.
+   *
+   * <p>Do NOT use this for assertions about results of EPP flows, as the XML header is required per
+   * the EPP spec for those. Rather, use this for raw operations on EPP XMLs, in situations where
+   * the header may be absent or incorrect (e.g. because you did operations on raw EPP XML directly
+   * loaded from a file without passing it through an EPP flow).
+   */
+  public static void assertXmlEqualsIgnoreHeader(
+      String expected, String actual, String... ignoredPaths) throws Exception {
+    assertXmlEqualsWithMessageHelper(expected, actual, "", ignoredPaths);
+  }
+
   public static void assertXmlEqualsWithMessage(
       String expected, String actual, String message, String... ignoredPaths) throws Exception {
     if (!actual.startsWith("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>")) {
       assertWithMessage("XML declaration not found at beginning:\n%s", actual).fail();
     }
+    assertXmlEqualsWithMessageHelper(expected, actual, message, ignoredPaths);
+  }
+
+  private static void assertXmlEqualsWithMessageHelper(
+      String expected, String actual, String message, String... ignoredPaths) throws Exception {
     Map<String, Object> expectedMap = toComparableJson(expected, ignoredPaths);
     Map<String, Object> actualMap = toComparableJson(actual, ignoredPaths);
     if (!expectedMap.equals(actualMap)) {