From 3936016eab6a7207b330ac8f0befa262b5e07096 Mon Sep 17 00:00:00 2001 From: adamfilipow92 <64852261+adamfilipow92@users.noreply.github.com> Date: Fri, 8 May 2020 15:24:55 +0200 Subject: [PATCH] #764 Fix crash on parse some control chars (#771) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * #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 Co-authored-by: Jan Góral <60390247+jan-gogo@users.noreply.github.com> --- release_notes.md | 2 +- test_runner/build.gradle.kts | 2 + test_runner/buildSrc/src/main/kotlin/Deps.kt | 5 ++ .../main/kotlin/ftl/reports/xml/JUnitXml.kt | 26 ++++--- .../xml/preprocesor/UtfControlCharRanges.kt | 13 ++++ .../xml/preprocesor/XmlPreprocessor.kt | 11 +++ .../kotlin/ftl/reports/xml/JUnitXmlTest.kt | 70 ++++++++++++++++++- 7 files changed, 115 insertions(+), 14 deletions(-) create mode 100644 test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/UtfControlCharRanges.kt create mode 100644 test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/XmlPreprocessor.kt diff --git a/release_notes.md b/release_notes.md index eaf61fdac9..ba6587ad77 100644 --- 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 index 646ad2aaf4..c37db75413 100644 --- 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 index c445def81f..2f6cbc2020 100644 --- 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 index b94676aa43..0211157adb 100644 --- a/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt +++ b/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt @@ -6,16 +6,24 @@ 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 = "\n" @@ -23,29 +31,27 @@ fun JUnitTestResult?.xmlToString(): String { } 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 new file mode 100644 index 0000000000..47948d1a68 --- /dev/null +++ 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 new file mode 100644 index 0000000000..288dba1e23 --- /dev/null +++ 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 index 9481d3b698..6fc3f97d5f 100644 --- 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) @Test fun `merge ios`() { - val merged = parseAllSuitesXml(iosPassXml).merge(parseAllSuitesXml(iosFailXml)).xmlToString().normalizeLineEnding() + val merged = + parseAllSuitesXml(iosPassXml).merge(parseAllSuitesXml(iosFailXml)).xmlToString().normalizeLineEnding() val expected = """ @@ -94,7 +96,8 @@ junit.framework.Assert.fail(Assert.java:50) @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 = """ @@ -423,7 +426,8 @@ junit.framework.Assert.fail(Assert.java:50) // * 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 = """ @@ -437,4 +441,64 @@ junit.framework.Assert.fail(Assert.java:50) """.trimIndent() assertThat(merged).isEqualTo(expected) } + + @Test + fun `parse ftl quirks in all suites`() { + val crashingAllSuitesMessage = """ + + + + + java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.() + + + + + + """.trimIndent() + + val expectedAllSuitesMessage = """ + + + + + java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.() + + + + + + """.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 = """ + + + + java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.() + + + + + """.trimIndent() + + val expectedOneSuiteMessage = """ + + + + + java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.() + + + + + + """.trimIndent() + val oneSuiteXml = parseOneSuiteXml(crashingOneSuiteMessage).xmlToString().trimIndent() + Assert.assertEquals("One Suite Messages should be the same!", expectedOneSuiteMessage, oneSuiteXml) + } }