diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 5f343eba..c51940e6 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -15,7 +15,7 @@ kotlin = "2.2.21" mockk = "1.14.6" roboelectric = "4.16" slf4j = "2.0.17" -spotbugs-annotations = "4.9.8" +spotbugs = "4.9.8" [libraries] android-desugar = { module = "com.android.tools:desugar_jdk_libs", version.ref = "android-desugar" } @@ -32,7 +32,7 @@ mockk = { module = "io.mockk:mockk", version.ref = "mockk" } mockk-android = { module = "io.mockk:mockk-android", version.ref = "mockk" } roboelectric = { module = "org.robolectric:robolectric", version.ref = "roboelectric" } slf4j-jdk = { module = "org.slf4j:slf4j-jdk14", version.ref = "slf4j" } -spotbugs-annotations = { module = "com.github.spotbugs:spotbugs-annotations", version.ref = "spotbugs-annotations" } +spotbugs-annotations = { module = "com.github.spotbugs:spotbugs-annotations", version.ref = "spotbugs" } [plugins] android-library = { id = "com.android.library", version.ref = "agp" } diff --git a/lib/build.gradle.kts b/lib/build.gradle.kts index fe061104..87ab3228 100644 --- a/lib/build.gradle.kts +++ b/lib/build.gradle.kts @@ -131,6 +131,9 @@ dependencies { // synctools.test package also provide test rules implementation(libs.androidx.test.rules) + // Useful annotations + api(libs.spotbugs.annotations) + // instrumented tests androidTestImplementation(libs.androidx.test.rules) androidTestImplementation(libs.androidx.test.runner) diff --git a/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidDayOffsetPreprocessor.kt b/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidDayOffsetPreprocessor.kt index 63cd9d00..77833180 100644 --- a/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidDayOffsetPreprocessor.kt +++ b/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidDayOffsetPreprocessor.kt @@ -6,13 +6,16 @@ package at.bitfire.synctools.icalendar.validation +import androidx.annotation.VisibleForTesting + /** * Fixes durations with day offsets with the 'T' prefix. * See also https://github.com/bitfireAT/ical4android/issues/77 */ -class FixInvalidDayOffsetPreprocessor : StreamPreprocessor() { +class FixInvalidDayOffsetPreprocessor : StreamPreprocessor { - override fun regexpForProblem() = Regex( + @VisibleForTesting + internal val regexpForProblem = Regex( // Examples: // TRIGGER:-P2DT // TRIGGER:-PT2D @@ -21,11 +24,11 @@ class FixInvalidDayOffsetPreprocessor : StreamPreprocessor() { setOf(RegexOption.MULTILINE, RegexOption.IGNORE_CASE) ) - override fun fixString(original: String): String { - var iCal: String = original + override fun repairLine(line: String): String { + var iCal: String = line // Find all instances matching the defined expression - val found = regexpForProblem().findAll(iCal).toList() + val found = regexpForProblem.findAll(iCal).toList() // ... and repair them. Use reversed order so that already replaced occurrences don't interfere with the following matches. for (match in found.reversed()) { diff --git a/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidUtcOffsetPreprocessor.kt b/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidUtcOffsetPreprocessor.kt index 89698f09..6441b4f4 100644 --- a/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidUtcOffsetPreprocessor.kt +++ b/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidUtcOffsetPreprocessor.kt @@ -6,6 +6,7 @@ package at.bitfire.synctools.icalendar.validation +import androidx.annotation.VisibleForTesting import java.util.logging.Level import java.util.logging.Logger @@ -13,21 +14,20 @@ import java.util.logging.Logger /** * Some servers modify UTC offsets in TZOFFSET(FROM,TO) like "+005730" to an invalid "+5730". * - * Rewrites values of all TZOFFSETFROM and TZOFFSETTO properties which match [TZOFFSET_REGEXP] + * Rewrites values of all TZOFFSETFROM and TZOFFSETTO properties which match [regexpForProblem] * so that an hour value of 00 is inserted. */ -class FixInvalidUtcOffsetPreprocessor: StreamPreprocessor() { +class FixInvalidUtcOffsetPreprocessor: StreamPreprocessor { private val logger get() = Logger.getLogger(javaClass.name) - private val TZOFFSET_REGEXP = Regex("^(TZOFFSET(FROM|TO):[+\\-]?)((18|19|[2-6]\\d)\\d\\d)$", + @VisibleForTesting + internal val regexpForProblem = Regex("^(TZOFFSET(FROM|TO):[+\\-]?)((18|19|[2-6]\\d)\\d\\d)$", setOf(RegexOption.MULTILINE, RegexOption.IGNORE_CASE)) - override fun regexpForProblem() = TZOFFSET_REGEXP - - override fun fixString(original: String) = - original.replace(TZOFFSET_REGEXP) { + override fun repairLine(line: String) = + line.replace(regexpForProblem) { logger.log(Level.FINE, "Applying Synology WebDAV fix to invalid utc-offset", it.value) "${it.groupValues[1]}00${it.groupValues[3]}" } diff --git a/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/ICalPreprocessor.kt b/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/ICalPreprocessor.kt index 8c6e17ab..df3f9862 100644 --- a/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/ICalPreprocessor.kt +++ b/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/ICalPreprocessor.kt @@ -7,15 +7,18 @@ package at.bitfire.synctools.icalendar.validation import androidx.annotation.VisibleForTesting +import com.google.common.io.CharSource import net.fortuna.ical4j.model.Calendar import net.fortuna.ical4j.model.Property import net.fortuna.ical4j.transform.rfc5545.CreatedPropertyRule import net.fortuna.ical4j.transform.rfc5545.DateListPropertyRule import net.fortuna.ical4j.transform.rfc5545.DatePropertyRule import net.fortuna.ical4j.transform.rfc5545.Rfc5545PropertyRule +import java.io.BufferedReader import java.io.Reader import java.util.logging.Level import java.util.logging.Logger +import javax.annotation.WillNotClose /** * Applies some rules to increase compatibility of parsed (incoming) iCalendars: @@ -40,18 +43,50 @@ class ICalPreprocessor { FixInvalidDayOffsetPreprocessor() // fix things like DURATION:PT2D ) + /** + * Applies [streamPreprocessors] to a given iCalendar [line]. + * + * @param line original line (taken from an iCalendar) + * @return the potentially repaired iCalendar line + */ + @VisibleForTesting + fun applyPreprocessors(line: String): String { + var newLine = line + for (preprocessor in streamPreprocessors) + newLine = preprocessor.repairLine(newLine) + return newLine + } + /** * Applies [streamPreprocessors] to a given [Reader] that reads an iCalendar object * in order to repair some things that must be fixed before parsing. * - * @param original original iCalendar object - * @return the potentially repaired iCalendar object + * The original reader content is processed line by line to avoid loading + * the whole content into memory at once. + * + * This method works in a streaming way, so **[original] must not be closed before + * the result of this method is consumed** like that: + * + * ~~~ + * someSource.reader().use { original -> + * val repaired = preprocessStream(original) + * // closing original here would render repaired unusable, too + * parse(repaired) + * } // use will close original + * ~~~ + * + * @param original original iCalendar object (must be closed by caller _after_ consuming the result of this method) + * @return potentially repaired iCalendar object (doesn't need to be closed separately) */ - fun preprocessStream(original: Reader): Reader { - var reader = original - for (preprocessor in streamPreprocessors) - reader = preprocessor.preprocess(reader) - return reader + fun preprocessStream(@WillNotClose original: Reader): Reader { + val repairedLines = BufferedReader(original) + .lineSequence() + .map { line -> // BufferedReader provides line without line break + val fixed = applyPreprocessors(line) + CharSource.wrap(fixed + "\r\n") // iCalendar uses CR+LF + } + .asIterable() + return CharSource.concat(repairedLines).openStream() } diff --git a/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/StreamPreprocessor.kt b/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/StreamPreprocessor.kt index 53e5cc87..b057d65d 100644 --- a/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/StreamPreprocessor.kt +++ b/lib/src/main/kotlin/at/bitfire/synctools/icalendar/validation/StreamPreprocessor.kt @@ -6,51 +6,20 @@ package at.bitfire.synctools.icalendar.validation -import java.io.IOException -import java.io.Reader -import java.io.StringReader -import java.util.Scanner - -abstract class StreamPreprocessor { - - abstract fun regexpForProblem(): Regex? +/** + * This is a pre-processor for iCalendar lines that can detect and repair errors which + * cannot be repaired on a higher level (because parsing alone would cause syntax + * or other unrecoverable errors). + */ +interface StreamPreprocessor { /** - * Fixes an iCalendar string. + * Validates and potentially repairs an iCalendar string. * - * @param original The complete iCalendar string - * @return The complete iCalendar string, but fixed + * @param line full line of an iCalendar lines to validate / fix (without line break) + * + * @return the potentially fixed version of [line] (without line break) */ - abstract fun fixString(original: String): String - - fun preprocess(reader: Reader): Reader { - var result: String? = null - - val resetSupported = try { - reader.reset() - true - } catch(_: IOException) { - false - } - - if (resetSupported) { - val regex = regexpForProblem() - // reset is supported, no need to copy the whole stream to another String (unless we have to fix the TZOFFSET) - if (regex == null || Scanner(reader).findWithinHorizon(regex.toPattern(), 0) != null) { - reader.reset() - result = fixString(reader.readText()) - } - } else - // reset not supported, always generate a new String that will be returned - result = fixString(reader.readText()) - - if (result != null) - // modified or reset not supported, return new stream - return StringReader(result) - - // not modified, return original iCalendar - reader.reset() - return reader - } + fun repairLine(line: String): String } \ No newline at end of file diff --git a/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidDayOffsetPreprocessorTest.kt b/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidDayOffsetPreprocessorTest.kt index 893a0133..3c6060f1 100644 --- a/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidDayOffsetPreprocessorTest.kt +++ b/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidDayOffsetPreprocessorTest.kt @@ -25,7 +25,7 @@ class FixInvalidDayOffsetPreprocessorTest { */ private fun assertFixedEquals(expected: String, testValue: String, parseDuration: Boolean = true) { // Fix the duration string - val fixed = processor.fixString(testValue) + val fixed = processor.repairLine(testValue) // Test the duration can now be parsed if (parseDuration) @@ -39,15 +39,15 @@ class FixInvalidDayOffsetPreprocessorTest { } @Test - fun test_FixString_NoOccurrence() { + fun test_repairLine_NoOccurrence() { assertEquals( "Some String", - processor.fixString("Some String"), + processor.repairLine("Some String"), ) } @Test - fun test_FixString_SucceedsAsValueOnCorrectProperties() { + fun test_repairLine_SucceedsAsValueOnCorrectProperties() { // By RFC 5545 the only properties allowed to hold DURATION as a VALUE are: // DURATION, REFRESH, RELATED, TRIGGER assertFixedEquals("DURATION;VALUE=DURATION:P1D", "DURATION;VALUE=DURATION:PT1D") @@ -57,18 +57,18 @@ class FixInvalidDayOffsetPreprocessorTest { } @Test - fun test_FixString_FailsAsValueOnWrongProperty() { + fun test_repairLine_FailsAsValueOnWrongProperty() { // The update from RFC 2445 to RFC 5545 disallows using DURATION as a VALUE in FREEBUSY assertFixedEquals("FREEBUSY;VALUE=DURATION:PT1D", "FREEBUSY;VALUE=DURATION:PT1D", parseDuration = false) } @Test - fun test_FixString_FailsIfNotAtStartOfLine() { + fun test_repairLine_FailsIfNotAtStartOfLine() { assertFixedEquals("xxDURATION;VALUE=DURATION:PT1D", "xxDURATION;VALUE=DURATION:PT1D", parseDuration = false) } @Test - fun test_FixString_DayOffsetFrom_Invalid() { + fun test_repairLine_DayOffsetFrom_Invalid() { assertFixedEquals("DURATION:-P1D", "DURATION:-PT1D") assertFixedEquals("TRIGGER:-P2D", "TRIGGER:-PT2D") @@ -77,38 +77,38 @@ class FixInvalidDayOffsetPreprocessorTest { } @Test - fun test_FixString_DayOffsetFrom_Valid() { + fun test_repairLine_DayOffsetFrom_Valid() { assertFixedEquals("DURATION:-PT12H", "DURATION:-PT12H") assertFixedEquals("TRIGGER:-PT12H", "TRIGGER:-PT12H") } @Test - fun test_FixString_DayOffsetFromMultiple_Invalid() { + fun test_repairLine_DayOffsetFromMultiple_Invalid() { assertFixedEquals("DURATION:-P1D\nTRIGGER:-P2D", "DURATION:-PT1D\nTRIGGER:-PT2D") assertFixedEquals("DURATION:-P1D\nTRIGGER:-P2D", "DURATION:-P1DT\nTRIGGER:-P2DT") } @Test - fun test_FixString_DayOffsetFromMultiple_Valid() { + fun test_repairLine_DayOffsetFromMultiple_Valid() { assertFixedEquals("DURATION:-PT12H\nTRIGGER:-PT12H", "DURATION:-PT12H\nTRIGGER:-PT12H") } @Test - fun test_FixString_DayOffsetFromMultiple_Mixed() { + fun test_repairLine_DayOffsetFromMultiple_Mixed() { assertFixedEquals("DURATION:-P1D\nDURATION:-PT12H\nTRIGGER:-P2D", "DURATION:-PT1D\nDURATION:-PT12H\nTRIGGER:-PT2D") assertFixedEquals("DURATION:-P1D\nDURATION:-PT12H\nTRIGGER:-P2D", "DURATION:-P1DT\nDURATION:-PT12H\nTRIGGER:-P2DT") } @Test fun test_RegexpForProblem_DayOffsetTo_Invalid() { - val regex = processor.regexpForProblem() + val regex = processor.regexpForProblem assertTrue(regex.matches("DURATION:PT2D")) assertTrue(regex.matches("TRIGGER:PT1D")) } @Test fun test_RegexpForProblem_DayOffsetTo_Valid() { - val regex = processor.regexpForProblem() + val regex = processor.regexpForProblem assertFalse(regex.matches("DURATION:-PT12H")) assertFalse(regex.matches("TRIGGER:-PT15M")) } diff --git a/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidUtcOffsetPreprocessorTest.kt b/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidUtcOffsetPreprocessorTest.kt index 9b86c27d..8c6ff8cf 100644 --- a/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidUtcOffsetPreprocessorTest.kt +++ b/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/FixInvalidUtcOffsetPreprocessorTest.kt @@ -16,46 +16,46 @@ class FixInvalidUtcOffsetPreprocessorTest { private val processor = FixInvalidUtcOffsetPreprocessor() @Test - fun test_FixString_NoOccurrence() { + fun test_repairLine_NoOccurrence() { assertEquals( "Some String", - processor.fixString("Some String")) + processor.repairLine("Some String")) } @Test - fun test_FixString_TzOffsetFrom_Invalid() { + fun test_repairLine_TzOffsetFrom_Invalid() { assertEquals("TZOFFSETFROM:+005730", - processor.fixString("TZOFFSETFROM:+5730")) + processor.repairLine("TZOFFSETFROM:+5730")) } @Test - fun test_FixString_TzOffsetFrom_Valid() { + fun test_repairLine_TzOffsetFrom_Valid() { assertEquals("TZOFFSETFROM:+005730", - processor.fixString("TZOFFSETFROM:+005730")) + processor.repairLine("TZOFFSETFROM:+005730")) } @Test - fun test_FixString_TzOffsetTo_Invalid() { + fun test_repairLine_TzOffsetTo_Invalid() { assertEquals("TZOFFSETTO:+005730", - processor.fixString("TZOFFSETTO:+5730")) + processor.repairLine("TZOFFSETTO:+5730")) } @Test - fun test_FixString_TzOffsetTo_Valid() { + fun test_repairLine_TzOffsetTo_Valid() { assertEquals("TZOFFSETTO:+005730", - processor.fixString("TZOFFSETTO:+005730")) + processor.repairLine("TZOFFSETTO:+005730")) } @Test fun test_RegexpForProblem_TzOffsetTo_Invalid() { - val regex = processor.regexpForProblem() + val regex = processor.regexpForProblem assertTrue(regex.matches("TZOFFSETTO:+5730")) } @Test fun test_RegexpForProblem_TzOffsetTo_Valid() { - val regex = processor.regexpForProblem() + val regex = processor.regexpForProblem assertFalse(regex.matches("TZOFFSETTO:+005730")) } diff --git a/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/ICalPreprocessorTest.kt b/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/ICalPreprocessorTest.kt index dab87525..a8076d96 100644 --- a/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/ICalPreprocessorTest.kt +++ b/lib/src/test/kotlin/at/bitfire/synctools/icalendar/validation/ICalPreprocessorTest.kt @@ -6,8 +6,10 @@ package at.bitfire.synctools.icalendar.validation +import com.google.common.io.CharStreams import io.mockk.junit4.MockKRule import io.mockk.mockkObject +import io.mockk.spyk import io.mockk.verify import net.fortuna.ical4j.data.CalendarBuilder import net.fortuna.ical4j.model.Component @@ -17,7 +19,10 @@ import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import java.io.InputStreamReader +import java.io.Reader import java.io.StringReader +import java.io.Writer +import java.util.UUID class ICalPreprocessorTest { @@ -26,26 +31,24 @@ class ICalPreprocessorTest { val processor = ICalPreprocessor() - @Test - fun testPreprocessStream_appliesStreamProcessors() { + fun testApplyPreprocessors_appliesStreamProcessors() { val preprocessors = processor.streamPreprocessors assertTrue(preprocessors.isNotEmpty()) processor.streamPreprocessors.forEach { mockkObject(it) } - processor.preprocessStream(StringReader("")) + processor.applyPreprocessors("") // verify that the required stream processors have been called verify { processor.streamPreprocessors.forEach { - it.preprocess(any()) + it.repairLine(any()) } } } - @Test fun testPreprocessCalendar_MsTimeZones() { javaClass.getResourceAsStream("/events/outlook1.ics").use { stream -> @@ -59,4 +62,113 @@ class ICalPreprocessorTest { } } + @Test + fun testPreprocessStream_joinsLinesCorrectly() { + val result = processor.preprocessStream(StringReader("BEGIN:VCALENDAR\nBEGIN:VEVENT")).readText() + assertEquals("BEGIN:VCALENDAR\r\nBEGIN:VEVENT\r\n", result) + } + + @Test + fun testPreprocessStream_runsApplyPreprocessors() { + val processor = spyk() + + // readText MUST be called. Otherwise the sequence is never evaluated + // there must be at least one line. Otherwise the sequence is empty + processor.preprocessStream(StringReader("\n")).use { it.readText() } + + // verify that applyPreprocessors has been called + verify { processor.applyPreprocessors(any()) } + } + + @Test + fun testPreprocessStream_LargeFiles() { + val preprocessor = ICalPreprocessor() + val reader = VCalendarReaderGenerator(eventCount = 10_000) + preprocessor.preprocessStream(reader).use { preprocessed -> + // consume preprocessed stream + val start = System.currentTimeMillis() + CharStreams.copy(preprocessed, Writer.nullWriter()) + val end = System.currentTimeMillis() + + // no exception called + System.err.println("testParse_SuperLargeFiles took ${(end - start) / 1000} seconds") + } + } + + + /** + * Reader that generates a number of VEVENTs for testing. + */ + private class VCalendarReaderGenerator(val eventCount: Int) : Reader() { + private var stage = 0 // 0 = header, 1 = events, 2 = footer, 3 = done + private var eventIdx = 0 + private var current: String? = null + private var pos = 0 + + override fun reset() { + stage = 0 + eventIdx = 0 + current = null + pos = 0 + } + + override fun read(cbuf: CharArray, off: Int, len: Int): Int { + var charsRead = 0 + while (charsRead < len) { + if (current == null || pos >= current!!.length) { + current = when (stage) { + 0 -> { + stage = 1 + """ + BEGIN:VCALENDAR + PRODID:-//xyz Corp//NONSGML PDA Calendar Version 1.0//EN + VERSION:2.0 + """.trimIndent() + "\n" + } + 1 -> { + if (eventIdx < eventCount) { + val event = """ + BEGIN:VEVENT + DTSTAMP:19960704T120000Z + UID:${UUID.randomUUID()} + ORGANIZER:mailto:jsmith@example.com + DTSTART:19960918T143000Z + DTEND:19960920T220000Z + STATUS:CONFIRMED + CATEGORIES:CONFERENCE + SUMMARY:Event $eventIdx + DESCRIPTION:Event $eventIdx description + END:VEVENT + """.trimIndent() + "\n" + eventIdx++ + event + } else { + stage = 2 + null + } + } + 2 -> { + stage = 3 + "END:VCALENDAR\n" + } + else -> return if (charsRead == 0) -1 else charsRead + } + pos = 0 + if (current == null) continue // move to next stage + } + val charsLeft = current!!.length - pos + val toRead = minOf(len - charsRead, charsLeft) + current!!.toCharArray(pos, pos + toRead).copyInto(cbuf, off + charsRead) + pos += toRead + charsRead += toRead + } + return charsRead + } + + override fun close() { + // No resources to release + current = null + } + } + } \ No newline at end of file