From 4ce136ad039f4d84fec5437a4d05cb27246cf9da Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 4 Dec 2024 14:39:40 -0500 Subject: [PATCH 1/3] Refine Source|Buffer.indexOf(ByteBuffer) contracts Closes #422 --- core/common/src/ByteStrings.kt | 27 ++++++++++++++++++++------- core/common/test/CommonBufferTest.kt | 16 ++++++++++++++++ core/jvm/test/BufferTest.kt | 1 + 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/core/common/src/ByteStrings.kt b/core/common/src/ByteStrings.kt index 8dd269c23..7cfb45a0d 100644 --- a/core/common/src/ByteStrings.kt +++ b/core/common/src/ByteStrings.kt @@ -85,6 +85,8 @@ public fun Source.readByteString(byteCount: Int): ByteString { * expands the source's buffer as necessary until [byteString] is found. This reads an unbounded number of * bytes into the buffer. Returns `-1` if the stream is exhausted before the requested bytes are found. * + * For empty byte strings this function always return `0`. + * * @param byteString the sequence of bytes to find within the source. * @param startIndex the index into the source to start searching from. * @@ -96,11 +98,9 @@ public fun Source.readByteString(byteCount: Int): ByteString { */ @OptIn(InternalIoApi::class, UnsafeByteStringApi::class) public fun Source.indexOf(byteString: ByteString, startIndex: Long = 0): Long { - require(startIndex >= 0) { "startIndex: $startIndex" } + require(startIndex >= 0) { "startIndex should be non-negative, but was: $startIndex" } - if (byteString.isEmpty()) { - return 0 - } + if (byteString.isEmpty()) return 0 var offset = startIndex while (request(offset + byteString.size)) { @@ -117,11 +117,24 @@ public fun Source.indexOf(byteString: ByteString, startIndex: Long = 0): Long { return -1 } +/** + * Returns the index of the first match for [byteString] in the buffer at or after [startIndex]. + * Returns `-1` if the stream is exhausted before the requested bytes are found. + * + * For empty byte strings this function always return `0`. + * + * @param byteString the sequence of bytes to find within the buffer. + * @param startIndex the index into the buffer to start searching from. + * + * @throws IllegalArgumentException if [startIndex] is negative or if it is greater of equal to buffer's [Buffer.size]. + * + * @sample kotlinx.io.samples.ByteStringSamples.indexOfByteString + */ @OptIn(UnsafeByteStringApi::class) public fun Buffer.indexOf(byteString: ByteString, startIndex: Long = 0): Long { - require(startIndex <= size) { - "startIndex ($startIndex) should not exceed size ($size)" - } + require(startIndex <= size) { "startIndex ($startIndex) should not exceed size ($size)" } + require(startIndex >= 0) { "startIndex should be non-negative, but was: $startIndex" } + if (byteString.isEmpty()) return 0 if (startIndex > size - byteString.size) return -1L diff --git a/core/common/test/CommonBufferTest.kt b/core/common/test/CommonBufferTest.kt index d009c5f62..4648bab31 100644 --- a/core/common/test/CommonBufferTest.kt +++ b/core/common/test/CommonBufferTest.kt @@ -24,6 +24,7 @@ import kotlinx.io.bytestring.ByteString import kotlinx.io.bytestring.encodeToByteString import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFails import kotlin.test.assertFailsWith import kotlin.test.assertTrue @@ -618,4 +619,19 @@ class CommonBufferTest { assertEquals(null, dst.head?.prev) assertEquals(null, dst.tail?.next) } + + @Test + fun indexOfByteString() { + val buffer = Buffer() + buffer.writeString("hello") + + assertFailsWith { buffer.indexOf(ByteString(1, 2, 3), -1) } + assertFailsWith { buffer.indexOf(ByteString(1, 2, 3), 10) } + + assertEquals(2, buffer.indexOf("ll".encodeToByteString())) + assertEquals(2, buffer.indexOf("ll".encodeToByteString(), 2)) + assertEquals(-1, buffer.indexOf("ll".encodeToByteString(), 3)) + assertEquals(-1, buffer.indexOf("hello world".encodeToByteString())) + assertEquals(0, buffer.indexOf(ByteString())) + } } diff --git a/core/jvm/test/BufferTest.kt b/core/jvm/test/BufferTest.kt index 693cf67ca..d1a7c4889 100644 --- a/core/jvm/test/BufferTest.kt +++ b/core/jvm/test/BufferTest.kt @@ -20,6 +20,7 @@ */ package kotlinx.io +import kotlinx.io.bytestring.ByteString import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.InputStream From c39b214a779551fb0eebd967965798b77a9c215f Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Thu, 5 Dec 2024 13:48:29 -0500 Subject: [PATCH 2/3] Removed unused import --- core/jvm/test/BufferTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/core/jvm/test/BufferTest.kt b/core/jvm/test/BufferTest.kt index d1a7c4889..693cf67ca 100644 --- a/core/jvm/test/BufferTest.kt +++ b/core/jvm/test/BufferTest.kt @@ -20,7 +20,6 @@ */ package kotlinx.io -import kotlinx.io.bytestring.ByteString import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.InputStream From 63630b8e8e3e569a644f356bf24777288e635066 Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Tue, 10 Dec 2024 16:21:27 -0500 Subject: [PATCH 3/3] Align indexOf(empty bytestring) behavior with CS.indexOf("") Closes #423 --- core/common/src/ByteStrings.kt | 25 ++++++++++++++----------- core/common/test/AbstractSourceTest.kt | 14 ++++++-------- core/common/test/CommonBufferTest.kt | 9 +++++++-- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/core/common/src/ByteStrings.kt b/core/common/src/ByteStrings.kt index 7cfb45a0d..a1468956c 100644 --- a/core/common/src/ByteStrings.kt +++ b/core/common/src/ByteStrings.kt @@ -10,6 +10,7 @@ import kotlinx.io.bytestring.isEmpty import kotlinx.io.bytestring.unsafe.UnsafeByteStringApi import kotlinx.io.bytestring.unsafe.UnsafeByteStringOperations import kotlinx.io.unsafe.UnsafeBufferOperations +import kotlin.math.max import kotlin.math.min /** @@ -85,12 +86,14 @@ public fun Source.readByteString(byteCount: Int): ByteString { * expands the source's buffer as necessary until [byteString] is found. This reads an unbounded number of * bytes into the buffer. Returns `-1` if the stream is exhausted before the requested bytes are found. * - * For empty byte strings this function always return `0`. + * For empty byte strings this function returns [startIndex] if it lays within underlying buffer's bounds, + * `0` if [startIndex] was negative and the size of the underlying buffer if [startIndex] exceeds its size. + * If the [startIndex] value was greater than the underlying buffer's size, the data will be fetched and buffered + * despite the [byteString] is empty. * * @param byteString the sequence of bytes to find within the source. * @param startIndex the index into the source to start searching from. * - * @throws IllegalArgumentException if [startIndex] is negative. * @throws IllegalStateException if the source is closed. * @throws IOException when some I/O error occurs. * @@ -98,9 +101,12 @@ public fun Source.readByteString(byteCount: Int): ByteString { */ @OptIn(InternalIoApi::class, UnsafeByteStringApi::class) public fun Source.indexOf(byteString: ByteString, startIndex: Long = 0): Long { - require(startIndex >= 0) { "startIndex should be non-negative, but was: $startIndex" } + val startIndex = max(0, startIndex) - if (byteString.isEmpty()) return 0 + if (byteString.isEmpty()) { + request(startIndex) + return min(startIndex, buffer.size) + } var offset = startIndex while (request(offset + byteString.size)) { @@ -119,23 +125,20 @@ public fun Source.indexOf(byteString: ByteString, startIndex: Long = 0): Long { /** * Returns the index of the first match for [byteString] in the buffer at or after [startIndex]. - * Returns `-1` if the stream is exhausted before the requested bytes are found. * - * For empty byte strings this function always return `0`. + * For empty byte strings this function returns [startIndex] if it lays within buffer's bounds, + * `0` if [startIndex] was negative and [Buffer.size] if it was greater or equal to [Buffer.size]. * * @param byteString the sequence of bytes to find within the buffer. * @param startIndex the index into the buffer to start searching from. * - * @throws IllegalArgumentException if [startIndex] is negative or if it is greater of equal to buffer's [Buffer.size]. - * * @sample kotlinx.io.samples.ByteStringSamples.indexOfByteString */ @OptIn(UnsafeByteStringApi::class) public fun Buffer.indexOf(byteString: ByteString, startIndex: Long = 0): Long { - require(startIndex <= size) { "startIndex ($startIndex) should not exceed size ($size)" } - require(startIndex >= 0) { "startIndex should be non-negative, but was: $startIndex" } + val startIndex = max(0, min(startIndex, size)) - if (byteString.isEmpty()) return 0 + if (byteString.isEmpty()) return startIndex if (startIndex > size - byteString.size) return -1L UnsafeByteStringOperations.withByteArrayUnsafe(byteString) { byteStringData -> diff --git a/core/common/test/AbstractSourceTest.kt b/core/common/test/AbstractSourceTest.kt index 71347afc0..867d6cd3b 100644 --- a/core/common/test/AbstractSourceTest.kt +++ b/core/common/test/AbstractSourceTest.kt @@ -1723,12 +1723,15 @@ abstract class AbstractBufferedSourceTest internal constructor( sink.writeString("flop flip flop") sink.emit() assertEquals(10, source.indexOf("flop".encodeToByteString(), 1)) + assertEquals(0, source.indexOf("flop".encodeToByteString(), -1)) source.readString() // Clear stream - // Make sure we backtrack and resume searching after partial match. + // Make sure we backtrack and resume searching after the partial match. sink.writeString("hi hi hi hi hey") sink.emit() assertEquals(6, source.indexOf("hi hi hey".encodeToByteString(), 1)) + + assertEquals(-1, source.indexOf("ho ho ho".encodeToByteString(), 9001)) } @Test @@ -1738,13 +1741,8 @@ abstract class AbstractBufferedSourceTest internal constructor( sink.writeString("blablabla") sink.emit() assertEquals(0, source.indexOf(ByteString())) - } - - @Test - fun indexOfByteStringInvalidArgumentsThrows() { - assertFailsWith { - source.indexOf("hi".encodeToByteString(), -1) - } + assertEquals(0, source.indexOf(ByteString(), -1)) + assertEquals(9, source.indexOf(ByteString(), 100000)) } /** diff --git a/core/common/test/CommonBufferTest.kt b/core/common/test/CommonBufferTest.kt index 4648bab31..d2b00cbd0 100644 --- a/core/common/test/CommonBufferTest.kt +++ b/core/common/test/CommonBufferTest.kt @@ -625,13 +625,18 @@ class CommonBufferTest { val buffer = Buffer() buffer.writeString("hello") - assertFailsWith { buffer.indexOf(ByteString(1, 2, 3), -1) } - assertFailsWith { buffer.indexOf(ByteString(1, 2, 3), 10) } + assertEquals(-1, buffer.indexOf(ByteString(1, 2, 3), -1)) + assertEquals(-1, buffer.indexOf(ByteString(1, 2, 3), 10)) assertEquals(2, buffer.indexOf("ll".encodeToByteString())) assertEquals(2, buffer.indexOf("ll".encodeToByteString(), 2)) + assertEquals(2, buffer.indexOf("ll".encodeToByteString(), -2)) assertEquals(-1, buffer.indexOf("ll".encodeToByteString(), 3)) assertEquals(-1, buffer.indexOf("hello world".encodeToByteString())) + assertEquals(0, buffer.indexOf(ByteString())) + assertEquals(buffer.size, buffer.indexOf(ByteString(), 1000)) + assertEquals(1, buffer.indexOf(ByteString(), 1)) + assertEquals(0, buffer.indexOf(ByteString(), -1)) } }