From d937a1e461723e579122eb446411269b23116016 Mon Sep 17 00:00:00 2001 From: Sergey Mashkov Date: Fri, 3 Jan 2020 22:01:13 +0300 Subject: [PATCH 1/3] ~ Fix chunked encoding case with specified content length --- .../ktor/http/cio/ChunkedTransferEncoding.kt | 32 +++++++++++++++++-- .../common/src/io/ktor/http/cio/HttpBody.kt | 7 ++-- .../io/ktor/tests/http/cio/ChunkedTest.kt | 4 +++ .../io/ktor/tests/http/cio/MultipartTest.kt | 2 +- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt index dc30dca868..abfebfa144 100644 --- a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt +++ b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt @@ -32,15 +32,30 @@ typealias DecoderJob = WriterJob /** * Start a chunked stream decoder coroutine */ -fun CoroutineScope.decodeChunked(input: ByteReadChannel): DecoderJob = writer(coroutineContext) { - decodeChunked(input, channel) +@Deprecated("Specify content length if known or pass -1L", + ReplaceWith("decodeChunked(input, -1L)")) +fun CoroutineScope.decodeChunked(input: ByteReadChannel): DecoderJob = + decodeChunked(input, -1L) + +/** + * Start a chunked stream decoder coroutine + */ +fun CoroutineScope.decodeChunked(input: ByteReadChannel, contentLength: Long): DecoderJob = writer(coroutineContext) { + decodeChunked(input, channel, contentLength) +} + +@Deprecated("Specify contentLength if provided or pass -1L", + ReplaceWith("decodeChunked(input, out, -1L)")) +suspend fun decodeChunked(input: ByteReadChannel, out: ByteWriteChannel) { + return decodeChunked(input, out, -1L) } /** * Chunked stream decoding loop */ -suspend fun decodeChunked(input: ByteReadChannel, out: ByteWriteChannel) { +suspend fun decodeChunked(input: ByteReadChannel, out: ByteWriteChannel, contentLength: Long) { val chunkSizeBuffer = ChunkSizeBufferPool.borrow() + var totalBytesCopied = 0L try { while (true) { @@ -55,9 +70,15 @@ suspend fun decodeChunked(input: ByteReadChannel, out: ByteWriteChannel) { if (chunkSizeBuffer.length == 1 && chunkSizeBuffer[0] == '0') 0 else chunkSizeBuffer.parseHexLong() + if (contentLength != -1L && chunkSize > (contentLength - totalBytesCopied)) { + input.cancel() + throw ParserException("Invalid chunk: chunk-encoded content is larger than expected $contentLength") + } + if (chunkSize > 0) { input.copyTo(out, chunkSize) out.flush() + totalBytesCopied += chunkSize } chunkSizeBuffer.clear() @@ -70,6 +91,11 @@ suspend fun decodeChunked(input: ByteReadChannel, out: ByteWriteChannel) { if (chunkSize == 0L) break } + + if (contentLength != -1L && totalBytesCopied != contentLength) { + input.cancel() + throw EOFException("Corrupted chunk-encoded content: steam ended before the expected number of bytes ($contentLength) were decoded.") + } } catch (t: Throwable) { out.close(t) throw t diff --git a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpBody.kt b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpBody.kt index 1af961ba67..e79486240f 100644 --- a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpBody.kt +++ b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpBody.kt @@ -68,6 +68,10 @@ fun expectHttpBody(request: Request): Boolean = expectHttpBody( /** * Parse HTTP request or response body using [contentLength], [transferEncoding] and [connectionOptions] * writing it to [out]. Usually doesn't fail but closing [out] channel with error. + * + * @param contentLength from the corresponding header or -1 + * @param transferEncoding header or `null` + * @param */ @KtorExperimentalAPI suspend fun parseHttpBody( @@ -79,12 +83,11 @@ suspend fun parseHttpBody( ) { if (transferEncoding != null) { when { - transferEncoding.equalsLowerCase(other = "chunked") -> return decodeChunked(input, out) + transferEncoding.equalsLowerCase(other = "chunked") -> return decodeChunked(input, out, contentLength) transferEncoding.equalsLowerCase(other = "identity") -> { // do nothing special } else -> out.close(IllegalStateException("Unsupported transfer-encoding $transferEncoding")) - // TODO: combined transfer encodings } } diff --git a/ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt b/ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt index 01c6a15a9a..9af915789b 100644 --- a/ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt +++ b/ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt @@ -191,4 +191,8 @@ class ChunkedTest { assertEquals(first, second) } + + private suspend fun decodeChunked(input: ByteReadChannel, out: ByteWriteChannel) { + return decodeChunked(input, out, -1L) + } } diff --git a/ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/MultipartTest.kt b/ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/MultipartTest.kt index c46b1791ba..da98b99065 100644 --- a/ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/MultipartTest.kt +++ b/ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/MultipartTest.kt @@ -244,7 +244,7 @@ class MultipartTest { val ch = ByteReadChannel(body.toByteArray()) val request = parseRequest(ch)!! - val decoded = GlobalScope.decodeChunked(ch) + val decoded = GlobalScope.decodeChunked(ch, -1L) val mp = GlobalScope.parseMultipart(decoded.channel, request.headers) val allEvents = ArrayList() From c1bd1ab9cbc935dbff25f20bff2c1f05bfb95c85 Mon Sep 17 00:00:00 2001 From: Sergey Mashkov Date: Wed, 25 Dec 2019 23:53:53 +0300 Subject: [PATCH 2/3] Add header line length overrun check --- .../ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt index d27eb87b53..cc79be3988 100644 --- a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt +++ b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt @@ -105,8 +105,10 @@ internal suspend fun parseHeaders( } range.end = builder.length + val rangeLength = range.end - range.start - if (range.start == range.end) break + if (rangeLength == 0) break + if (rangeLength >= HTTP_LINE_LIMIT) error("Header line length limit exceeded") val nameStart = range.start val nameEnd = parseHeaderName(builder, range) From 007f2d2c870f7b62cf30a301c034bd2a51b0e342 Mon Sep 17 00:00:00 2001 From: Sergey Mashkov Date: Sun, 5 Jan 2020 20:52:39 +0300 Subject: [PATCH 3/3] ~ binary --- .../reference-public-api/ktor-http-cio.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/binary-compatibility-validator/reference-public-api/ktor-http-cio.txt b/binary-compatibility-validator/reference-public-api/ktor-http-cio.txt index 769b8b4d16..27daaef9bf 100644 --- a/binary-compatibility-validator/reference-public-api/ktor-http-cio.txt +++ b/binary-compatibility-validator/reference-public-api/ktor-http-cio.txt @@ -19,8 +19,10 @@ public final class io/ktor/http/cio/CIOMultipartDataBase : io/ktor/http/content/ } public final class io/ktor/http/cio/ChunkedTransferEncodingKt { + public static final fun decodeChunked (Lio/ktor/utils/io/ByteReadChannel;Lio/ktor/utils/io/ByteWriteChannel;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public static final fun decodeChunked (Lio/ktor/utils/io/ByteReadChannel;Lio/ktor/utils/io/ByteWriteChannel;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public static final fun decodeChunked (Lkotlinx/coroutines/CoroutineScope;Lio/ktor/utils/io/ByteReadChannel;)Lio/ktor/utils/io/WriterJob; + public static final fun decodeChunked (Lkotlinx/coroutines/CoroutineScope;Lio/ktor/utils/io/ByteReadChannel;J)Lio/ktor/utils/io/WriterJob; public static final fun encodeChunked (Lio/ktor/utils/io/ByteWriteChannel;Lio/ktor/utils/io/ByteReadChannel;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public static final fun encodeChunked (Lio/ktor/utils/io/ByteWriteChannel;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; }