diff --git a/core/src/main/java/google/registry/xml/XmlTransformer.java b/core/src/main/java/google/registry/xml/XmlTransformer.java index 6ccd5713c..41de0de95 100644 --- a/core/src/main/java/google/registry/xml/XmlTransformer.java +++ b/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)); } diff --git a/core/src/test/java/google/registry/flows/EppXmlSanitizerTest.java b/core/src/test/java/google/registry/flows/EppXmlSanitizerTest.java index 1bdad99f6..30169b318 100644 --- a/core/src/test/java/google/registry/flows/EppXmlSanitizerTest.java +++ b/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); - - String sanitizedXml = sanitizeEppXml(inputXmlBytes); - assertThat(sanitizedXml).isEqualTo(expectedXml); + assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXmlBytes)); } @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(); - - String sanitizedXml = sanitizeEppXml(inputXml.getBytes(UTF_8)); - assertThat(sanitizedXml).isEqualTo(expectedXml); + assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXml.getBytes(UTF_8))); } @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(); - - String sanitizedXml = sanitizeEppXml(inputXml.getBytes(UTF_8)); - assertThat(sanitizedXml).isEqualTo(expectedXml); + assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXml.getBytes(UTF_8))); } @Test @@ -86,9 +81,7 @@ public class EppXmlSanitizerTest { String expectedXml = UTF8_HEADER + new EppLoader(this, "contact_info_sanitized.xml", ImmutableMap.of()).getEppXml(); - - String sanitizedXml = sanitizeEppXml(inputXmlBytes); - assertThat(sanitizedXml).isEqualTo(expectedXml); + assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXmlBytes)); } @Test @@ -99,15 +92,13 @@ public class EppXmlSanitizerTest { + new EppLoader( this, "contact_info_from_create_response_sanitized.xml", ImmutableMap.of()) .getEppXml(); - - String sanitizedXml = sanitizeEppXml(inputXmlBytes); - assertThat(sanitizedXml).isEqualTo(expectedXml); + assertXmlEqualsIgnoreHeader(expectedXml, sanitizeEppXml(inputXmlBytes)); } @Test - public void testSanitize_emptyElement_transformedToLongForm() { + public void testSanitize_emptyElement_transformedToLongForm() throws Exception { byte[] inputXmlBytes = "".getBytes(UTF_8); - assertThat(sanitizeEppXml(inputXmlBytes)).isEqualTo(UTF8_HEADER + "\n"); + assertXmlEqualsIgnoreHeader("", 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 = "\u007F\u4E43x".getBytes(UTF_8); - String expectedXml = UTF8_HEADER + "C**\n"; - assertThat(sanitizeEppXml(inputXmlBytes)).isEqualTo(expectedXml); + assertXmlEqualsIgnoreHeader("C**", sanitizeEppXml(inputXmlBytes)); } @Test @@ -136,6 +126,13 @@ public class EppXmlSanitizerTest { // used, the XMLEventReader in sanitizer may resolve it to an endian-specific one. String inputXml = "

\u03bc

\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); } } diff --git a/core/src/test/java/google/registry/flows/FlowTestCase.java b/core/src/test/java/google/registry/flows/FlowTestCase.java index c2546d09f..249ef768d 100644 --- a/core/src/test/java/google/registry/flows/FlowTestCase.java +++ b/core/src/test/java/google/registry/flows/FlowTestCase.java @@ -114,7 +114,7 @@ public abstract class FlowTestCase 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 substitutions) { diff --git a/core/src/test/java/google/registry/xml/XmlTestUtils.java b/core/src/test/java/google/registry/xml/XmlTestUtils.java index dd3de65e4..d80347a45 100644 --- a/core/src/test/java/google/registry/xml/XmlTestUtils.java +++ b/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. + * + *

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. + * + *

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("")) { 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 expectedMap = toComparableJson(expected, ignoredPaths); Map actualMap = toComparableJson(actual, ignoredPaths); if (!expectedMap.equals(actualMap)) {