From b4a21604f1aa175dc979056a0e2e367b8aa0712c Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Thu, 27 May 2021 13:53:46 -0400 Subject: [PATCH 1/2] fix reserve behavior with existing data; add new convenience wrappers --- build.gradle.kts | 4 +- .../software/aws/clientrt/io/Exceptions.kt | 19 +++++++ .../src/software/aws/clientrt/io/SdkBuffer.kt | 34 +++++++++--- .../software/aws/clientrt/io/SdkBufferTest.kt | 52 +++++++++++++++++++ .../software/aws/clientrt/io/SdkBufferJVM.kt | 11 ++++ settings.gradle.kts | 5 ++ 6 files changed, 116 insertions(+), 9 deletions(-) create mode 100644 client-runtime/io/common/src/software/aws/clientrt/io/Exceptions.kt diff --git a/build.gradle.kts b/build.gradle.kts index 65a03b1692..4572efd665 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -16,8 +16,8 @@ buildscript { } plugins { - kotlin("jvm") version "1.4.31" apply false - id("org.jetbrains.dokka") version "1.4.20" + kotlin("jvm") version "1.5.0" apply false + id("org.jetbrains.dokka") } allprojects { diff --git a/client-runtime/io/common/src/software/aws/clientrt/io/Exceptions.kt b/client-runtime/io/common/src/software/aws/clientrt/io/Exceptions.kt new file mode 100644 index 0000000000..69a57a3ae5 --- /dev/null +++ b/client-runtime/io/common/src/software/aws/clientrt/io/Exceptions.kt @@ -0,0 +1,19 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.aws.clientrt.io + +/** + * Exception thrown when a content-mutation method such as `write` is invoked upon a read-only buffer. + */ +class ReadOnlyBufferException : UnsupportedOperationException { + constructor() : super() + + constructor(message: String?) : super(message) + + constructor(message: String?, cause: Throwable?) : super(message, cause) + + constructor(cause: Throwable?) : super(cause) +} diff --git a/client-runtime/io/common/src/software/aws/clientrt/io/SdkBuffer.kt b/client-runtime/io/common/src/software/aws/clientrt/io/SdkBuffer.kt index 3d4a01f436..b426c428ea 100644 --- a/client-runtime/io/common/src/software/aws/clientrt/io/SdkBuffer.kt +++ b/client-runtime/io/common/src/software/aws/clientrt/io/SdkBuffer.kt @@ -6,7 +6,6 @@ package software.aws.clientrt.io import io.ktor.utils.io.bits.* -import io.ktor.utils.io.charsets.* import io.ktor.utils.io.core.* import software.aws.clientrt.util.InternalApi @@ -15,6 +14,9 @@ private class SdkBufferState { var readHead: Int = 0 } +@OptIn(ExperimentalIoApi::class) +internal expect fun Memory.Companion.ofByteArray(src: ByteArray, offset: Int = 0, length: Int = src.size - offset): Memory + /** * A buffer with read and write positions. Similar in spirit to `java.nio.ByteBuffer` but for use * in Kotlin Multiplatform. @@ -27,13 +29,23 @@ private class SdkBufferState { */ @OptIn(ExperimentalIoApi::class) @InternalApi -class SdkBuffer(initialCapacity: Int) { +class SdkBuffer internal constructor( + // we make use of ktor-io's `Memory` type which already implements most of the functionality in a platform + // agnostic way. We just need to wrap some methods around it + internal var memory: Memory, + val isReadOnly: Boolean = false +) { + constructor(initialCapacity: Int, readOnly: Boolean = false) : this(DefaultAllocator.alloc(initialCapacity), readOnly) + // TODO - we could implement Appendable but we would need to deal with Char as UTF-16 character // (e.g. convert code points to number of bytes and write the correct utf bytes 1..4) - // we make use of ktor-io's `Memory` type which already implements most of the functionality in a platform - // agnostic way. We just need to wrap some methods around it - internal var memory = DefaultAllocator.alloc(initialCapacity) + companion object { + /** + * Create an SdkBuffer backed by the given ByteArray + */ + fun of(src: ByteArray, offset: Int = 0, length: Int = src.size - offset): SdkBuffer = SdkBuffer(Memory.ofByteArray(src, offset, length)) + } private val state = SdkBufferState() @@ -75,8 +87,8 @@ class SdkBuffer(initialCapacity: Int) { fun reserve(count: Int) { if (writeRemaining >= count) return - val minP2 = ceilp2(count) - val currP2 = ceilp2(memory.size32 + 1) + val minP2 = ceilp2(count + writePosition) + val currP2 = ceilp2(memory.size32 + writePosition + 1) val newSize = maxOf(minP2, currP2) memory = DefaultAllocator.realloc(memory, newSize) } @@ -125,6 +137,12 @@ class SdkBuffer(initialCapacity: Int) { public inline val SdkBuffer.canRead: Boolean get() = writePosition > readPosition +/** + * Creates a new, read-only byte buffer that shares this buffer's content. + * Any attempts to write to the buffer will fail with [ReadOnlyBufferException] + */ +fun SdkBuffer.asReadOnly(): SdkBuffer = if (isReadOnly) this else SdkBuffer(memory, isReadOnly = true) + /** * Read from this buffer exactly [length] bytes and write to [dest] starting at [offset] * @throws IllegalArgumentException if there are not enough bytes available for read or the offset/length combination is invalid @@ -237,6 +255,8 @@ private inline fun SdkBuffer.read(block: (memory: Memory, readStart: Int, endExc @OptIn(ExperimentalIoApi::class) private inline fun SdkBuffer.write(block: (memory: Memory, writeStart: Int, endExclusive: Int) -> Int): Int { + if (isReadOnly) throw ReadOnlyBufferException("attempt to write to readOnly buffer at index: $writePosition") + val wc = block(memory, writePosition, capacity) commitWritten(wc) return wc diff --git a/client-runtime/io/common/test/software/aws/clientrt/io/SdkBufferTest.kt b/client-runtime/io/common/test/software/aws/clientrt/io/SdkBufferTest.kt index 9d3839d8c7..53e42b3dc1 100644 --- a/client-runtime/io/common/test/software/aws/clientrt/io/SdkBufferTest.kt +++ b/client-runtime/io/common/test/software/aws/clientrt/io/SdkBufferTest.kt @@ -5,9 +5,11 @@ package software.aws.clientrt.io +import io.ktor.utils.io.core.* import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertTrue class SdkBufferTest { @Test @@ -206,6 +208,15 @@ class SdkBufferTest { assertEquals(128, buf.capacity) } + @Test + fun testReserveExistingData() { + // https://github.com/awslabs/aws-sdk-kotlin/issues/147 + val buf = SdkBuffer(256) + buf.commitWritten(138) + buf.reserve(444) + assertEquals(1024, buf.capacity) + } + @Test fun testWriteFullyPastDestSize() { val buf = SdkBuffer(16) @@ -281,4 +292,45 @@ class SdkBufferTest { val bytes = buf.bytes() assertEquals(16, bytes.size) } + + @Test + fun testReadOnly() { + val buf = SdkBuffer(16, readOnly = true) + val data = "foo" + + assertFailsWith { + buf.write(data) + } + assertFailsWith { + buf.writeFully(data.encodeToByteArray()) + } + assertFailsWith { + val src = SdkBuffer.of(data.encodeToByteArray()) + buf.writeFully(src) + } + + assertTrue(buf.isReadOnly) + val buf2 = SdkBuffer(16).asReadOnly() + assertTrue(buf2.isReadOnly) + } + + @Test + fun testOfByteArray() { + val data = "hello world".toByteArray() + val buf = SdkBuffer.of(data) + assertEquals(data.size, buf.capacity) + + // does not automatically make the contents readable + assertEquals(0, buf.readRemaining) + buf.commitWritten(data.size) + assertEquals(data.size, buf.readRemaining) + + buf.reset() + buf.commitWritten(6) + buf.write("tests") + assertEquals("hello tests", buf.decodeToString()) + + // original buffer should have been modified + assertEquals("hello tests", data.decodeToString()) + } } diff --git a/client-runtime/io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt b/client-runtime/io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt index fc15d7fe1a..ccb32eb1b1 100644 --- a/client-runtime/io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt +++ b/client-runtime/io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt @@ -5,9 +5,20 @@ package software.aws.clientrt.io +import io.ktor.utils.io.bits.* +import java.nio.ByteBuffer + internal fun SdkBuffer.hasArray() = memory.buffer.hasArray() && !memory.buffer.isReadOnly actual fun SdkBuffer.bytes(): ByteArray = when (hasArray()) { true -> memory.buffer.array().sliceArray(readPosition until readRemaining) false -> ByteArray(readRemaining).apply { readFully(this) } } + +internal actual fun Memory.Companion.ofByteArray(src: ByteArray, offset: Int, length: Int): Memory +Memory.of(src, offset, length) + +/** + * Create a new SdkBuffer using the given [ByteBuffer] as the contents + */ +fun SdkBuffer.Companion.of(byteBuffer: ByteBuffer): SdkBuffer = SdkBuffer(Memory.of(byteBuffer)) diff --git a/settings.gradle.kts b/settings.gradle.kts index 9670b688f7..fb33321830 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -11,6 +11,11 @@ pluginManagement { google() gradlePluginPortal() } + + // configure the smithy-gradle plugin version + plugins { + id("org.jetbrains.dokka") version "1.4.32" + } } rootProject.name = "smithy-kotlin" From 1f9c2c6aae28a23c8763ec54d85d1452609513ba Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Thu, 27 May 2021 14:03:12 -0400 Subject: [PATCH 2/2] fix lint --- .../io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client-runtime/io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt b/client-runtime/io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt index ccb32eb1b1..93b93d5372 100644 --- a/client-runtime/io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt +++ b/client-runtime/io/jvm/src/software/aws/clientrt/io/SdkBufferJVM.kt @@ -15,8 +15,8 @@ actual fun SdkBuffer.bytes(): ByteArray = when (hasArray()) { false -> ByteArray(readRemaining).apply { readFully(this) } } -internal actual fun Memory.Companion.ofByteArray(src: ByteArray, offset: Int, length: Int): Memory -Memory.of(src, offset, length) +internal actual fun Memory.Companion.ofByteArray(src: ByteArray, offset: Int, length: Int): Memory = + Memory.of(src, offset, length) /** * Create a new SdkBuffer using the given [ByteBuffer] as the contents