From 86a7bd928a2080b71d3bffc80e3fc9afb082f57c Mon Sep 17 00:00:00 2001 From: Gleb Nazarov <74966728+stokado@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:01:28 +0500 Subject: [PATCH] KTOR-7470 receiveMultipart throw UnsupportedMediaTypeException (#4339) --- gradle.properties | 2 +- ktor-http/ktor-http-cio/api/ktor-http-cio.api | 4 + .../ktor-http-cio/api/ktor-http-cio.klib.api | 4 + .../common/src/io/ktor/http/cio/Multipart.kt | 10 ++- .../src/io/ktor/http/cio/internals/Errors.kt | 11 +++ .../api/ktor-server-core.klib.api | 2 +- .../src/io/ktor/server/plugins/Errors.kt | 7 +- .../ktor/server/engine/DefaultTransformJvm.kt | 23 ++++-- .../tests/server/engine/MultiPartDataTest.kt | 78 +++++++++++++++++++ 9 files changed, 127 insertions(+), 14 deletions(-) create mode 100644 ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/internals/Errors.kt create mode 100644 ktor-server/ktor-server-core/jvm/test/io/ktor/tests/server/engine/MultiPartDataTest.kt diff --git a/gradle.properties b/gradle.properties index 8be4c4ed07f..34e2c0fb084 100644 --- a/gradle.properties +++ b/gradle.properties @@ -2,7 +2,7 @@ # Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. # -# sytleguide +# styleguide kotlin.code.style=official # config diff --git a/ktor-http/ktor-http-cio/api/ktor-http-cio.api b/ktor-http/ktor-http-cio/api/ktor-http-cio.api index be316981f41..d322a02cec2 100644 --- a/ktor-http/ktor-http-cio/api/ktor-http-cio.api +++ b/ktor-http/ktor-http-cio/api/ktor-http-cio.api @@ -155,3 +155,7 @@ public final class io/ktor/http/cio/internals/MutableRange { public fun toString ()Ljava/lang/String; } +public final class io/ktor/http/cio/internals/UnsupportedMediaTypeExceptionCIO : java/io/IOException { + public fun (Ljava/lang/String;)V +} + diff --git a/ktor-http/ktor-http-cio/api/ktor-http-cio.klib.api b/ktor-http/ktor-http-cio/api/ktor-http-cio.klib.api index 2418cb5200d..39891106013 100644 --- a/ktor-http/ktor-http-cio/api/ktor-http-cio.klib.api +++ b/ktor-http/ktor-http-cio/api/ktor-http-cio.klib.api @@ -27,6 +27,10 @@ final class io.ktor.http.cio.internals/MutableRange { // io.ktor.http.cio.intern final fun toString(): kotlin/String // io.ktor.http.cio.internals/MutableRange.toString|toString(){}[0] } +final class io.ktor.http.cio.internals/UnsupportedMediaTypeExceptionCIO : kotlinx.io/IOException { // io.ktor.http.cio.internals/UnsupportedMediaTypeExceptionCIO|null[0] + constructor (kotlin/String) // io.ktor.http.cio.internals/UnsupportedMediaTypeExceptionCIO.|(kotlin.String){}[0] +} + final class io.ktor.http.cio/CIOHeaders : io.ktor.http/Headers { // io.ktor.http.cio/CIOHeaders|null[0] constructor (io.ktor.http.cio/HttpHeadersMap) // io.ktor.http.cio/CIOHeaders.|(io.ktor.http.cio.HttpHeadersMap){}[0] diff --git a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/Multipart.kt b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/Multipart.kt index 670eb13fcea..df47747efc4 100644 --- a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/Multipart.kt +++ b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/Multipart.kt @@ -130,12 +130,15 @@ private suspend fun ByteReadChannel.skipIfFoundReadCount(prefix: ByteString): Lo /** * Starts a multipart parser coroutine producing multipart events */ +@OptIn(InternalAPI::class) public fun CoroutineScope.parseMultipart( input: ByteReadChannel, headers: HttpHeadersMap, maxPartSize: Long = Long.MAX_VALUE ): ReceiveChannel { - val contentType = headers["Content-Type"] ?: throw IOException("Failed to parse multipart: no Content-Type header") + val contentType = headers["Content-Type"] ?: throw UnsupportedMediaTypeExceptionCIO( + "Failed to parse multipart: no Content-Type header" + ) val contentLength = headers["Content-Length"]?.parseDecLong() return parseMultipart(input, contentType, contentLength, maxPartSize) @@ -144,6 +147,7 @@ public fun CoroutineScope.parseMultipart( /** * Starts a multipart parser coroutine producing multipart events */ +@OptIn(InternalAPI::class) public fun CoroutineScope.parseMultipart( input: ByteReadChannel, contentType: CharSequence, @@ -151,7 +155,9 @@ public fun CoroutineScope.parseMultipart( maxPartSize: Long = Long.MAX_VALUE, ): ReceiveChannel { if (!contentType.startsWith("multipart/", ignoreCase = true)) { - throw IOException("Failed to parse multipart: Content-Type should be multipart/* but it is $contentType") + throw UnsupportedMediaTypeExceptionCIO( + "Failed to parse multipart: Content-Type should be multipart/* but it is $contentType" + ) } val boundaryByteBuffer = parseBoundaryInternal(contentType) val boundaryBytes = ByteString(boundaryByteBuffer) diff --git a/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/internals/Errors.kt b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/internals/Errors.kt new file mode 100644 index 00000000000..cb9fa810822 --- /dev/null +++ b/ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/internals/Errors.kt @@ -0,0 +1,11 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. + */ + +package io.ktor.http.cio.internals + +import io.ktor.utils.io.* +import kotlinx.io.IOException + +@InternalAPI +public class UnsupportedMediaTypeExceptionCIO(message: String) : IOException(message) diff --git a/ktor-server/ktor-server-core/api/ktor-server-core.klib.api b/ktor-server/ktor-server-core/api/ktor-server-core.klib.api index 5099f1d6820..6f3ca17e85e 100644 --- a/ktor-server/ktor-server-core/api/ktor-server-core.klib.api +++ b/ktor-server/ktor-server-core/api/ktor-server-core.klib.api @@ -699,7 +699,7 @@ final class io.ktor.server.plugins/PayloadTooLargeException : io.ktor.server.plu } final class io.ktor.server.plugins/UnsupportedMediaTypeException : io.ktor.server.plugins/ContentTransformationException, kotlinx.coroutines/CopyableThrowable { // io.ktor.server.plugins/UnsupportedMediaTypeException|null[0] - constructor (io.ktor.http/ContentType) // io.ktor.server.plugins/UnsupportedMediaTypeException.|(io.ktor.http.ContentType){}[0] + constructor (io.ktor.http/ContentType?) // io.ktor.server.plugins/UnsupportedMediaTypeException.|(io.ktor.http.ContentType?){}[0] final fun createCopy(): io.ktor.server.plugins/UnsupportedMediaTypeException // io.ktor.server.plugins/UnsupportedMediaTypeException.createCopy|createCopy(){}[0] } diff --git a/ktor-server/ktor-server-core/common/src/io/ktor/server/plugins/Errors.kt b/ktor-server/ktor-server-core/common/src/io/ktor/server/plugins/Errors.kt index 8c672d512f0..8b1393be592 100644 --- a/ktor-server/ktor-server-core/common/src/io/ktor/server/plugins/Errors.kt +++ b/ktor-server/ktor-server-core/common/src/io/ktor/server/plugins/Errors.kt @@ -85,8 +85,11 @@ public class CannotTransformContentToTypeException( */ @OptIn(ExperimentalCoroutinesApi::class) public class UnsupportedMediaTypeException( - private val contentType: ContentType -) : ContentTransformationException("Content type $contentType is not supported"), + private val contentType: ContentType? +) : ContentTransformationException( + contentType?.let { "Content type $it is not supported" } + ?: "Content-Type header is required" +), CopyableThrowable { override fun createCopy(): UnsupportedMediaTypeException = UnsupportedMediaTypeException(contentType).also { diff --git a/ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/DefaultTransformJvm.kt b/ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/DefaultTransformJvm.kt index dd3c7d43079..a5379f8e5e7 100644 --- a/ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/DefaultTransformJvm.kt +++ b/ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/DefaultTransformJvm.kt @@ -6,8 +6,10 @@ package io.ktor.server.engine import io.ktor.http.* import io.ktor.http.cio.* +import io.ktor.http.cio.internals.* import io.ktor.http.content.* import io.ktor.server.application.* +import io.ktor.server.plugins.UnsupportedMediaTypeException import io.ktor.server.request.* import io.ktor.util.pipeline.* import io.ktor.utils.io.* @@ -33,16 +35,21 @@ internal actual suspend fun PipelineContext.defaultPlatformTr @OptIn(InternalAPI::class) internal actual fun PipelineContext<*, PipelineCall>.multiPartData(rc: ByteReadChannel): MultiPartData { val contentType = call.request.header(HttpHeaders.ContentType) - ?: throw IllegalStateException("Content-Type header is required for multipart processing") + ?: throw UnsupportedMediaTypeException(null) val contentLength = call.request.header(HttpHeaders.ContentLength)?.toLong() - return CIOMultipartDataBase( - coroutineContext + Dispatchers.Unconfined, - rc, - contentType, - contentLength, - call.formFieldLimit - ) + + try { + return CIOMultipartDataBase( + coroutineContext + Dispatchers.Unconfined, + rc, + contentType, + contentLength, + call.formFieldLimit + ) + } catch (_: UnsupportedMediaTypeExceptionCIO) { + throw UnsupportedMediaTypeException(ContentType.parse(contentType)) + } } internal actual fun Source.readTextWithCustomCharset(charset: Charset): String = diff --git a/ktor-server/ktor-server-core/jvm/test/io/ktor/tests/server/engine/MultiPartDataTest.kt b/ktor-server/ktor-server-core/jvm/test/io/ktor/tests/server/engine/MultiPartDataTest.kt new file mode 100644 index 00000000000..5f3934ed78b --- /dev/null +++ b/ktor-server/ktor-server-core/jvm/test/io/ktor/tests/server/engine/MultiPartDataTest.kt @@ -0,0 +1,78 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. + */ + +package io.ktor.tests.server.engine + +import io.ktor.client.request.* +import io.ktor.http.* +import io.ktor.server.application.* +import io.ktor.server.engine.* +import io.ktor.server.plugins.* +import io.ktor.server.request.* +import io.ktor.server.response.* +import io.ktor.server.routing.* +import io.ktor.server.testing.* +import io.ktor.util.pipeline.* +import io.ktor.utils.io.* +import io.mockk.* +import kotlinx.coroutines.* +import kotlinx.coroutines.test.* +import kotlin.test.* + +class MultiPartDataTest { + private val mockContext = mockk>(relaxed = true) + private val mockRequest = mockk(relaxed = true) + private val testScope = TestScope() + + @Test + fun givenRequest_whenNoContentTypeHeaderPresent_thenUnsupportedMediaTypeException() { + // Given + every { mockContext.call.request } returns mockRequest + every { mockRequest.header(HttpHeaders.ContentType) } returns null + + // When & Then + assertFailsWith { + runBlocking { mockContext.multiPartData(ByteReadChannel("sample data")) } + } + } + + @Test + fun givenWrongContentType_whenProcessMultiPart_thenUnsupportedMediaTypeException() { + // Given + val rc = ByteReadChannel("sample data") + val contentType = "test/plain; boundary=test" + val contentLength = "123" + every { mockContext.call.request } returns mockRequest + every { mockContext.call.attributes.getOrNull(any()) } returns 0L + every { mockRequest.header(HttpHeaders.ContentType) } returns contentType + every { mockRequest.header(HttpHeaders.ContentLength) } returns contentLength + + // When & Then + testScope.runTest { + assertFailsWith { + mockContext.multiPartData(rc) + } + } + } + + @Test + fun testUnsupportedMediaTypeStatusCode() = testApplication { + routing { + post { + call.receiveMultipart() + call.respond(HttpStatusCode.OK) + } + } + + client.post { + accept(ContentType.Text.Plain) + }.apply { + assertEquals(HttpStatusCode.UnsupportedMediaType, status) + } + + client.post {}.apply { + assertEquals(HttpStatusCode.UnsupportedMediaType, status) + } + } +}