From e54b405fe41014f6d2152458b1382e1da5aa1de2 Mon Sep 17 00:00:00 2001 From: Abduqodiri Qurbonzoda Date: Tue, 8 Oct 2019 11:21:57 +0300 Subject: [PATCH] Fix Iterable, Sequence and String index overflow in `windowed` --- .../common/src/generated/_Collections.kt | 12 ++++--- .../stdlib/common/src/generated/_Strings.kt | 10 ++++-- .../stdlib/test/collections/IterableTests.kt | 18 +++++++++++ .../stdlib/test/collections/SequenceTest.kt | 18 +++++++++++ libraries/stdlib/test/text/StringTest.kt | 32 +++++++++++++++++++ .../src/templates/Generators.kt | 28 ++++++++++------ 6 files changed, 100 insertions(+), 18 deletions(-) diff --git a/libraries/stdlib/common/src/generated/_Collections.kt b/libraries/stdlib/common/src/generated/_Collections.kt index ad8cc0c8a7c..1d44a4ae5d6 100644 --- a/libraries/stdlib/common/src/generated/_Collections.kt +++ b/libraries/stdlib/common/src/generated/_Collections.kt @@ -2160,7 +2160,7 @@ public fun Iterable.windowed(size: Int, step: Int = 1, partialWindows: Bo val resultCapacity = thisSize / step + if (thisSize % step == 0) 0 else 1 val result = ArrayList>(resultCapacity) var index = 0 - while (index < thisSize) { + while (index in 0 until thisSize) { val windowSize = size.coerceAtMost(thisSize - index) if (windowSize < size && !partialWindows) break result.add(List(windowSize) { this[it + index] }) @@ -2197,12 +2197,14 @@ public fun Iterable.windowed(size: Int, step: Int = 1, partialWindows: checkWindowSizeStep(size, step) if (this is RandomAccess && this is List) { val thisSize = this.size - val result = ArrayList((thisSize + step - 1) / step) + val resultCapacity = thisSize / step + if (thisSize % step == 0) 0 else 1 + val result = ArrayList(resultCapacity) val window = MovingSubList(this) var index = 0 - while (index < thisSize) { - window.move(index, (index + size).coerceAtMost(thisSize)) - if (!partialWindows && window.size < size) break + while (index in 0 until thisSize) { + val windowSize = size.coerceAtMost(thisSize - index) + if (!partialWindows && windowSize < size) break + window.move(index, index + windowSize) result.add(transform(window)) index += step } diff --git a/libraries/stdlib/common/src/generated/_Strings.kt b/libraries/stdlib/common/src/generated/_Strings.kt index 71c2cd05bb7..370b89cd002 100644 --- a/libraries/stdlib/common/src/generated/_Strings.kt +++ b/libraries/stdlib/common/src/generated/_Strings.kt @@ -1377,9 +1377,9 @@ public fun CharSequence.windowed(size: Int, step: Int = 1, partialWindows: B val resultCapacity = thisSize / step + if (thisSize % step == 0) 0 else 1 val result = ArrayList(resultCapacity) var index = 0 - while (index < thisSize) { + while (index in 0 until thisSize) { val end = index + size - val coercedEnd = if (end > thisSize) { if (partialWindows) thisSize else break } else end + val coercedEnd = if (end < 0 || end > thisSize) { if (partialWindows) thisSize else break } else end result.add(transform(subSequence(index, coercedEnd))) index += step } @@ -1427,7 +1427,11 @@ public fun CharSequence.windowedSequence(size: Int, step: Int = 1, partialWindow public fun CharSequence.windowedSequence(size: Int, step: Int = 1, partialWindows: Boolean = false, transform: (CharSequence) -> R): Sequence { checkWindowSizeStep(size, step) val windows = (if (partialWindows) indices else 0 until length - size + 1) step step - return windows.asSequence().map { index -> transform(subSequence(index, (index + size).coerceAtMost(length))) } + return windows.asSequence().map { index -> + val end = index + size + val coercedEnd = if (end < 0 || end > length) length else end + transform(subSequence(index, coercedEnd)) + } } /** diff --git a/libraries/stdlib/test/collections/IterableTests.kt b/libraries/stdlib/test/collections/IterableTests.kt index 0b799f4513d..e9b97920220 100644 --- a/libraries/stdlib/test/collections/IterableTests.kt +++ b/libraries/stdlib/test/collections/IterableTests.kt @@ -199,6 +199,24 @@ abstract class OrderedIterableTests>(createFrom: (Array("size $illegalValue") { data.windowed(illegalValue, 1) } assertFailsWith("step $illegalValue") { data.windowed(1, illegalValue) } } + + // index overflow tests + for (partialWindows in listOf(true, false)) { + + val windowed1 = data.windowed(5, Int.MAX_VALUE, partialWindows) + assertEquals(data.take(5), windowed1.single()) + val windowed2 = data.windowed(Int.MAX_VALUE, 5, partialWindows) + assertEquals(if (partialWindows) listOf(data.toList(), listOf("5", "6")) else listOf(), windowed2) + val windowed3 = data.windowed(Int.MAX_VALUE, Int.MAX_VALUE, partialWindows) + assertEquals(if (partialWindows) listOf(data.toList()) else listOf(), windowed3) + + val windowedTransform1 = data.windowed(5, Int.MAX_VALUE, partialWindows) { it.joinToString("") } + assertEquals("01234", windowedTransform1.single()) + val windowedTransform2 = data.windowed(Int.MAX_VALUE, 5, partialWindows) { it.joinToString("") } + assertEquals(if (partialWindows) listOf("0123456", "56") else listOf(), windowedTransform2) + val windowedTransform3 = data.windowed(Int.MAX_VALUE, Int.MAX_VALUE, partialWindows) { it.joinToString("") } + assertEquals(if (partialWindows) listOf("0123456") else listOf(), windowedTransform3) + } } diff --git a/libraries/stdlib/test/collections/SequenceTest.kt b/libraries/stdlib/test/collections/SequenceTest.kt index 1a6cc2a5ab6..4f5555ded92 100644 --- a/libraries/stdlib/test/collections/SequenceTest.kt +++ b/libraries/stdlib/test/collections/SequenceTest.kt @@ -311,6 +311,24 @@ public class SequenceTest { } ensureIsIntermediate(source = sequenceOf(1, 2, 3)) { it.windowed(2, 1) } + + // index overflow tests + for (partialWindows in listOf(true, false)) { + + val windowed1 = seq.windowed(5, Int.MAX_VALUE, partialWindows) + assertEquals(seq.take(5).toList(), windowed1.single()) + val windowed2 = seq.windowed(Int.MAX_VALUE, 5, partialWindows) + assertEquals(if (partialWindows) listOf(seq.toList(), listOf(5, 6)) else listOf(), windowed2.toList()) + val windowed3 = seq.windowed(Int.MAX_VALUE, Int.MAX_VALUE, partialWindows) + assertEquals(if (partialWindows) listOf(seq.toList()) else listOf(), windowed3.toList()) + + val windowedTransform1 = seq.windowed(5, Int.MAX_VALUE, partialWindows) { it.joinToString("") } + assertEquals("01234", windowedTransform1.single()) + val windowedTransform2 = seq.windowed(Int.MAX_VALUE, 5, partialWindows) { it.joinToString("") } + assertEquals(if (partialWindows) listOf("0123456", "56") else listOf(), windowedTransform2.toList()) + val windowedTransform3 = seq.windowed(Int.MAX_VALUE, Int.MAX_VALUE, partialWindows) { it.joinToString("") } + assertEquals(if (partialWindows) listOf("0123456") else listOf(), windowedTransform3.toList()) + } } @Test fun zip() { diff --git a/libraries/stdlib/test/text/StringTest.kt b/libraries/stdlib/test/text/StringTest.kt index 2299011dce6..9864bbdef76 100644 --- a/libraries/stdlib/test/text/StringTest.kt +++ b/libraries/stdlib/test/text/StringTest.kt @@ -1130,6 +1130,38 @@ class StringTest { data.windowedSequence(window, step, partialWindows = true).iterator()) { iteratorBehavior() } } } + + // index overflow tests + for (partialWindows in listOf(true, false)) { + + val windowed1 = data.windowed(5, Int.MAX_VALUE, partialWindows) + assertEquals("abcde", windowed1.single()) + val windowed2 = data.windowed(Int.MAX_VALUE, 5, partialWindows) + assertEquals(if (partialWindows) listOf(data.toString(), data.substring(5, 7)) else listOf(), windowed2) + val windowed3 = data.windowed(Int.MAX_VALUE, Int.MAX_VALUE, partialWindows) + assertEquals(if (partialWindows) listOf(data.toString()) else listOf(), windowed3) + + val windowedTransform1 = data.windowed(5, Int.MAX_VALUE, partialWindows) { it } + assertEquals("abcde", windowedTransform1.single()) + val windowedTransform2 = data.windowed(Int.MAX_VALUE, 5, partialWindows) { it } + assertEquals(if (partialWindows) listOf(data.toString(), data.substring(5, 7)) else listOf(), windowedTransform2) + val windowedTransform3 = data.windowed(Int.MAX_VALUE, Int.MAX_VALUE, partialWindows) { it } + assertEquals(if (partialWindows) listOf(data.toString()) else listOf(), windowedTransform3) + + val windowedSequence1 = data.windowedSequence(5, Int.MAX_VALUE, partialWindows) + assertEquals("abcde", windowedSequence1.single()) + val windowedSequence2 = data.windowedSequence(Int.MAX_VALUE, 5, partialWindows) + assertEquals(if (partialWindows) listOf(data.toString(), data.substring(5, 7)) else listOf(), windowedSequence2.toList()) + val windowedSequence3 = data.windowedSequence(Int.MAX_VALUE, Int.MAX_VALUE, partialWindows) + assertEquals(if (partialWindows) listOf(data.toString()) else listOf(), windowedSequence3.toList()) + + val windowedSequenceTransform1 = data.windowedSequence(5, Int.MAX_VALUE, partialWindows) { it } + assertEquals("abcde", windowedSequenceTransform1.single()) + val windowedSequenceTransform2 = data.windowedSequence(Int.MAX_VALUE, 5, partialWindows) { it } + assertEquals(if (partialWindows) listOf(data.toString(), data.substring(5, 7)) else listOf(), windowedSequenceTransform2.toList()) + val windowedSequenceTransform3 = data.windowedSequence(Int.MAX_VALUE, Int.MAX_VALUE, partialWindows) { it } + assertEquals(if (partialWindows) listOf(data.toString()) else listOf(), windowedSequenceTransform3.toList()) + } } @Test fun map() = withOneCharSequenceArg { arg1 -> diff --git a/libraries/tools/kotlin-stdlib-gen/src/templates/Generators.kt b/libraries/tools/kotlin-stdlib-gen/src/templates/Generators.kt index 107f13dff85..4dc8b537407 100644 --- a/libraries/tools/kotlin-stdlib-gen/src/templates/Generators.kt +++ b/libraries/tools/kotlin-stdlib-gen/src/templates/Generators.kt @@ -655,12 +655,14 @@ object Generators : TemplateGroupBase() { checkWindowSizeStep(size, step) if (this is RandomAccess && this is List) { val thisSize = this.size - val result = ArrayList((thisSize + step - 1) / step) + val resultCapacity = thisSize / step + if (thisSize % step == 0) 0 else 1 + val result = ArrayList(resultCapacity) val window = MovingSubList(this) var index = 0 - while (index < thisSize) { - window.move(index, (index + size).coerceAtMost(thisSize)) - if (!partialWindows && window.size < size) break + while (index in 0 until thisSize) { + val windowSize = size.coerceAtMost(thisSize - index) + if (!partialWindows && windowSize < size) break + window.move(index, index + windowSize) result.add(transform(window)) index += step } @@ -681,11 +683,12 @@ object Generators : TemplateGroupBase() { """ checkWindowSizeStep(size, step) val thisSize = this.length - val result = ArrayList((thisSize + step - 1) / step) + val resultCapacity = thisSize / step + if (thisSize % step == 0) 0 else 1 + val result = ArrayList(resultCapacity) var index = 0 - while (index < thisSize) { + while (index in 0 until thisSize) { val end = index + size - val coercedEnd = if (end > thisSize) { if (partialWindows) thisSize else break } else end + val coercedEnd = if (end < 0 || end > thisSize) { if (partialWindows) thisSize else break } else end result.add(transform(subSequence(index, coercedEnd))) index += step } @@ -731,9 +734,10 @@ object Generators : TemplateGroupBase() { checkWindowSizeStep(size, step) if (this is RandomAccess && this is List) { val thisSize = this.size - val result = ArrayList>((thisSize + step - 1) / step) + val resultCapacity = thisSize / step + if (thisSize % step == 0) 0 else 1 + val result = ArrayList>(resultCapacity) var index = 0 - while (index < thisSize) { + while (index in 0 until thisSize) { val windowSize = size.coerceAtMost(thisSize - index) if (windowSize < size && !partialWindows) break result.add(List(windowSize) { this[it + index] }) @@ -785,7 +789,11 @@ object Generators : TemplateGroupBase() { """ checkWindowSizeStep(size, step) val windows = (if (partialWindows) indices else 0 until length - size + 1) step step - return windows.asSequence().map { index -> transform(subSequence(index, (index + size).coerceAtMost(length))) } + return windows.asSequence().map { index -> + val end = index + size + val coercedEnd = if (end < 0 || end > length) length else end + transform(subSequence(index, coercedEnd)) + } """ } }