Fix OOM issues with super large iCalendar files #92
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /* | ||
| * This file is part of bitfireAT/synctools which is released under GPLv3. | ||
| * Copyright © All Contributors. See the LICENSE and AUTHOR files in the root directory for details. | ||
| * SPDX-License-Identifier: GPL-3.0-or-later | ||
| */ | ||
|
|
||
| package at.bitfire.synctools.icalendar.validation | ||
|
|
||
| import org.junit.Test | ||
| import java.io.Reader | ||
| import java.util.UUID | ||
|
|
||
| class ICalPreprocessorInstrumentedTest { | ||
|
|
||
| class VCalendarReaderGenerator(val eventCount: Int = Int.MAX_VALUE) : Reader() { | ||
ArnyminerZ marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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:[email protected] | ||
| 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 | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun testParse_SuperLargeFiles() { | ||
|
There was a problem hiding this comment. What does this test (class) do? As I understand it, this test basically tests whether the input is put into the memory as one chunk (which would fail with OOM) or whether it's processed in streaming mode so that it doesn't load too big files into the memory. So I think there are two things that can be tested:
However, when the input is not read at all, why then generate an iCalendar at all? We could as well pass an infinite stream of the same character.
This is currently not done by the tests. I noticed it because if we pass And everything for the two cases that
There was a problem hiding this comment.
Yes, and it's purposefully placed in the instrumented test class so that it's run in an emulator, which causes the OOM. Otherwise we are not actually overflowing the emulator but our computer. That's why there's the instrumented suffix, and actually, also exists
Mmmmh yeah, good point |
||
| val preprocessor = ICalPreprocessor() | ||
| val reader = VCalendarReaderGenerator() | ||
| preprocessor.preprocessStream(reader) | ||
| // no exception called | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As there's no Android dependency, it should be possible to make this test a unit test (faster and easier to run than an instrumentation test).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above