#764 Fix crash on parse some control chars (#771)

* #764 Fix crash on parse some control chars * #764 detekt issues fix detekt issues fix * Asserts in tests Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8 * Add documentation of magicial numers * set name of UtfControlChars enum to UtfControlCharRanges * #764 Fix crash on parse some control chars * #764 detekt issues fix detekt issues fix * Asserts in tests Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8 * Add documentation of magicial numers * set name of UtfControlChars enum to UtfControlCharRanges * Detekt suggestions * Create should_exists.txt * #764 Fix crash on parse some control chars * #764 detekt issues fix detekt issues fix * Asserts in tests Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8 * Add documentation of magicial numers * set name of UtfControlChars enum to UtfControlCharRanges * Fail fast when results-dir is incorrect (#772) * Fail fast when results-dir is incorrect * #764 Change XmlPreprocessor more functional and remove should_exists * Add info about issue to release notes Co-authored-by: Adam <[email protected]> Co-authored-by: Jan Góral <[email protected]>
Flank · May 13, 2020 · 3936016 · 3936016
1 parent 5f0158c
commit 3936016
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 14 deletions.
diff --git a/release_notes.md b/release_notes.md
@@ -1,5 +1,5 @@
 ## next (unreleased)
-- [#656](https://github.com/Flank/flank/issues/656) Improve error message reporting. ([adamfilipow92](https://github.com/adamfilipow92))
+- [#764](https://github.com/Flank/flank/pull/771) Fix crash on parse some control chars. ([adamfilipow92](https://github.com/adamfilipow92))
 - [#772](https://github.com/Flank/flank/pull/772) Fail fast when results-dir is incorrect. ([jan-gogo](https://github.com/jan-gogo))
 - [#757](https://github.com/Flank/flank/pull/767) Reduce memory usage by using Reader and Writer instead of ByteArrays. ([jan-gogo](https://github.com/jan-gogo))
 - [#763](https://github.com/Flank/flank/pull/763) Use "localhost" as default for hostname to fix backward compatibility. ([jan-gogo](https://github.com/jan-gogo))

diff --git a/test_runner/build.gradle.kts b/test_runner/build.gradle.kts
@@ -213,6 +213,8 @@ dependencies {
     implementation(Libs.SYSTEM_RULES)
     testImplementation(Libs.TRUTH)
     testImplementation(Libs.MOCKK)
+
+    implementation(Libs.COMMON_TEXT)
 }
 
 // Fix Exception in thread "main" java.lang.NoSuchMethodError: com.google.common.hash.Hashing.crc32c()Lcom/google/common/hash/HashFunction;

diff --git a/test_runner/buildSrc/src/main/kotlin/Deps.kt b/test_runner/buildSrc/src/main/kotlin/Deps.kt
@@ -74,6 +74,9 @@ object Versions {
 
     // https://github.com/mockk/mockk
     const val MOCKK = "1.9.3"
+
+    //https://commons.apache.org/proper/commons-text/
+    const val COMMON_TEXT = "1.8"
 }
 
 object Libs {
@@ -123,4 +126,6 @@ object Libs {
     const val TRUTH = "com.google.truth:truth:${Versions.TRUTH}"
     const val MOCKK = "io.mockk:mockk:${Versions.MOCKK}"
     //endregion
+
+    const val COMMON_TEXT = "org.apache.commons:commons-text:${Versions.COMMON_TEXT}"
 }
diff --git a/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt b/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt
@@ -6,46 +6,52 @@ import com.fasterxml.jackson.module.kotlin.KotlinModule
 import com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES
 import ftl.reports.xml.model.JUnitTestResult
 import ftl.reports.xml.model.JUnitTestSuite
+import ftl.reports.xml.preprocesor.fixHtmlCodes
 import java.io.File
+import java.nio.file.Files
 import java.nio.file.Path
 
 private val xmlModule = JacksonXmlModule().apply { setDefaultUseWrapper(false) }
+
 private val xmlMapper = XmlMapper(xmlModule)
     .registerModules(KotlinModule())
     .configure(FAIL_ON_UNKNOWN_PROPERTIES, false)
 
 internal val xmlPrettyWriter = xmlMapper.writerWithDefaultPrettyPrinter()
 
+private fun xmlText(path: Path): String {
+    if (!path.toFile().exists()) throw RuntimeException("$path doesn't exist!")
+    return String(Files.readAllBytes(path))
+}
+
 fun JUnitTestResult?.xmlToString(): String {
     if (this == null) return ""
     val prefix = "<?xml version='1.0' encoding='UTF-8' ?>\n"
     return prefix + xmlPrettyWriter.writeValueAsString(this)
 }
 
 fun parseOneSuiteXml(path: Path): JUnitTestResult {
-    return parseOneSuiteXml(path.toFile())
+    return parseOneSuiteXml(xmlText(path))
 }
 
-fun parseOneSuiteXml(file: File): JUnitTestResult {
-    if (!file.exists()) throw RuntimeException("$file doesn't exist!")
-    return JUnitTestResult(mutableListOf(xmlMapper.readValue(file, JUnitTestSuite::class.java)))
+fun parseOneSuiteXml(path: File): JUnitTestResult {
+    return parseOneSuiteXml(xmlText(path.toPath()))
 }
 
 fun parseOneSuiteXml(data: String): JUnitTestResult {
-    return JUnitTestResult(mutableListOf(xmlMapper.readValue(data, JUnitTestSuite::class.java)))
+    return JUnitTestResult(mutableListOf(xmlMapper.readValue(fixHtmlCodes(data), JUnitTestSuite::class.java)))
 }
 
 // --
 
 fun parseAllSuitesXml(path: Path): JUnitTestResult {
-    return parseAllSuitesXml(path.toFile())
+    return parseAllSuitesXml(xmlText(path))
 }
 
-fun parseAllSuitesXml(file: File): JUnitTestResult {
-    if (!file.exists()) throw RuntimeException("$file doesn't exist!")
-    return xmlMapper.readValue(file, JUnitTestResult::class.java)
+fun parseAllSuitesXml(path: File): JUnitTestResult {
+    return parseAllSuitesXml(path.toPath())
 }
 
 fun parseAllSuitesXml(data: String): JUnitTestResult {
-    return xmlMapper.readValue(data, JUnitTestResult::class.java)
+    return xmlMapper.readValue(fixHtmlCodes(data), JUnitTestResult::class.java)
 }
diff --git a/test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/UtfControlCharRanges.kt b/test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/UtfControlCharRanges.kt
@@ -0,0 +1,13 @@
+package ftl.reports.xml.preprocesor
+
+/**
+ *   Numbers come from ascii table https://www.utf8-chartable.de.
+ *   and represents control chars. We need to avoid characters in ranges CONTROL_TOP_START..CONTROL_TOP_END and
+ *   CONTROL_BOTTOM_START..CONTROL_BOTTOM_END because chars from that range escaped to html causing parsing errors.
+ * */
+enum class UtfControlCharRanges(val charValue: Int) {
+    CONTROL_TOP_START(0x00),
+    CONTROL_TOP_END(0x1F),
+    CONTROL_BOTTOM_START(0x7F),
+    CONTROL_BOTTOM_END(0x9F)
+}
diff --git a/test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/XmlPreprocessor.kt b/test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/XmlPreprocessor.kt
@@ -0,0 +1,11 @@
+package ftl.reports.xml.preprocesor
+
+import org.apache.commons.text.StringEscapeUtils
+
+fun fixHtmlCodes(data: String): String = listOf(
+    UtfControlCharRanges.CONTROL_TOP_START.charValue..UtfControlCharRanges.CONTROL_TOP_END.charValue,
+    UtfControlCharRanges.CONTROL_BOTTOM_START.charValue..UtfControlCharRanges.CONTROL_BOTTOM_END.charValue
+).flatten()
+    .map { StringEscapeUtils.escapeXml11(it.toChar().toString()) }
+    .filter { it.startsWith("&#") }
+    .fold(data) { fixedStr: String, isoControlCode: String -> fixedStr.replace(isoControlCode, "") }
diff --git a/test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt b/test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt
@@ -2,6 +2,7 @@ package ftl.reports.xml
 
 import com.google.common.truth.Truth.assertThat
 import ftl.test.util.TestHelper.normalizeLineEnding
+import org.junit.Assert
 import java.nio.file.Paths
 import org.junit.Test
 
@@ -73,7 +74,8 @@ junit.framework.Assert.fail(Assert.java:50)</failure>
 
     @Test
     fun `merge ios`() {
-        val merged = parseAllSuitesXml(iosPassXml).merge(parseAllSuitesXml(iosFailXml)).xmlToString().normalizeLineEnding()
+        val merged =
+            parseAllSuitesXml(iosPassXml).merge(parseAllSuitesXml(iosFailXml)).xmlToString().normalizeLineEnding()
         val expected = """
 <?xml version='1.0' encoding='UTF-8' ?>
 <testsuites>
@@ -94,7 +96,8 @@ junit.framework.Assert.fail(Assert.java:50)</failure>
 
     @Test
     fun `Merge iOS large time`() {
-        val merged = parseAllSuitesXml(iosLargeNum).merge(parseAllSuitesXml(iosLargeNum)).xmlToString().normalizeLineEnding()
+        val merged =
+            parseAllSuitesXml(iosLargeNum).merge(parseAllSuitesXml(iosLargeNum)).xmlToString().normalizeLineEnding()
 
         val expected = """
 <?xml version='1.0' encoding='UTF-8' ?>
@@ -423,7 +426,8 @@ junit.framework.Assert.fail(Assert.java:50)</failure>
         // * c() failed in newRun and passed in oldRun. timing info copied over from oldRun
         // * d() was skipped in newRun and successful in oldRun. d() is excluded from the merged result
 
-        val merged = parseAllSuitesXml(newRun).mergeTestTimes(parseAllSuitesXml(oldRun)).xmlToString().normalizeLineEnding()
+        val merged =
+            parseAllSuitesXml(newRun).mergeTestTimes(parseAllSuitesXml(oldRun)).xmlToString().normalizeLineEnding()
         val expected = """
 <?xml version='1.0' encoding='UTF-8' ?>
 <testsuites>
@@ -437,4 +441,64 @@ junit.framework.Assert.fail(Assert.java:50)</failure>
         """.trimIndent()
         assertThat(merged).isEqualTo(expected)
     }
+
+    @Test
+    fun `parse ftl quirks in all suites`() {
+        val crashingAllSuitesMessage = """
+            <?xml version='1.0' encoding='UTF-8' ?>
+            <testsuites>
+              <testsuite name="EarlGreyExampleSwiftTests" tests="3" failures="0" errors="0" skipped="0" time="10.0" hostname="localhost">
+                <testcase name="a()" classname="a" time="1.0">
+                <failure> java.net.ConnectException: Failed to connect to ... at &#8;&#8;&#8;(Coroutine boundary.&#8;(&#8;)</failure>
+                </testcase>
+                <testcase name="b()" classname="b" time="2.0"/>
+                <testcase name="c()" classname="c" time="7.0"/>
+              </testsuite>
+            </testsuites>
+        """.trimIndent()
+
+        val expectedAllSuitesMessage = """
+            <?xml version='1.0' encoding='UTF-8' ?>
+            <testsuites>
+              <testsuite name="EarlGreyExampleSwiftTests" tests="3" failures="0" errors="0" skipped="0" time="10.0" hostname="localhost">
+                <testcase name="a()" classname="a" time="1.0">
+                  <failure> java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.()</failure>
+                </testcase>
+                <testcase name="b()" classname="b" time="2.0"/>
+                <testcase name="c()" classname="c" time="7.0"/>
+              </testsuite>
+            </testsuites>
+        """.trimIndent()
+        val allSuitesXml = parseAllSuitesXml(crashingAllSuitesMessage).xmlToString().trimIndent()
+        Assert.assertEquals("All Suite Messages should be the same!", expectedAllSuitesMessage, allSuitesXml)
+    }
+
+    @Test
+    fun `parse ftl quirks in on suite`() {
+        val crashingOneSuiteMessage = """
+            <?xml version='1.0' encoding='UTF-8' ?>
+              <testsuite name="EarlGreyExampleSwiftTests" tests="3" failures="0" errors="0" skipped="0" time="10.0" hostname="localhost">
+                <testcase name="a()" classname="a" time="1.0">
+                <failure> java.net.ConnectException: Failed to connect to ... at &#8;&#8;&#8;(Coroutine boundary.&#8;(&#8;)</failure>
+                </testcase>
+                <testcase name="b()" classname="b" time="2.0"/>
+                <testcase name="c()" classname="c" time="7.0"/>
+              </testsuite>
+        """.trimIndent()
+
+        val expectedOneSuiteMessage = """
+            <?xml version='1.0' encoding='UTF-8' ?>
+            <testsuites>
+              <testsuite name="EarlGreyExampleSwiftTests" tests="3" failures="0" errors="0" skipped="0" time="10.0" hostname="localhost">
+                <testcase name="a()" classname="a" time="1.0">
+                  <failure> java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.()</failure>
+                </testcase>
+                <testcase name="b()" classname="b" time="2.0"/>
+                <testcase name="c()" classname="c" time="7.0"/>
+              </testsuite>
+            </testsuites>
+        """.trimIndent()
+        val oneSuiteXml = parseOneSuiteXml(crashingOneSuiteMessage).xmlToString().trimIndent()
+        Assert.assertEquals("One Suite Messages should be the same!", expectedOneSuiteMessage, oneSuiteXml)
+    }
 }