Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KTOR-7470 receiveMultipart throw UnsupportedMediaTypeException #4339

Merged
merged 16 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Copyright 2014-2020 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
Expand Down
11 changes: 11 additions & 0 deletions ktor-http/ktor-http-cio/jvm/src/io/ktor/http/cio/Errors.kt
Original file line number Diff line number Diff line change
@@ -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

import io.ktor.utils.io.*
import kotlinx.io.IOException

@InternalAPI
public class UnsupportedMediaTypeExceptionCIO(message: String) : IOException(message)
10 changes: 8 additions & 2 deletions ktor-http/ktor-http-cio/jvm/src/io/ktor/http/cio/Multipart.kt
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,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<MultipartEvent> {
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)
Expand All @@ -147,14 +150,17 @@ public fun CoroutineScope.parseMultipart(
/**
* Starts a multipart parser coroutine producing multipart events
*/
@OptIn(InternalAPI::class)
public fun CoroutineScope.parseMultipart(
input: ByteReadChannel,
contentType: CharSequence,
contentLength: Long?,
maxPartSize: Long = Long.MAX_VALUE,
): ReceiveChannel<MultipartEvent> {
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)
Expand Down
2 changes: 1 addition & 1 deletion ktor-server/ktor-server-core/api/ktor-server-core.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,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> { // io.ktor.server.plugins/UnsupportedMediaTypeException|null[0]
constructor <init>(io.ktor.http/ContentType) // io.ktor.server.plugins/UnsupportedMediaTypeException.<init>|<init>(io.ktor.http.ContentType){}[0]
constructor <init>(io.ktor.http/ContentType?) // io.ktor.server.plugins/UnsupportedMediaTypeException.<init>|<init>(io.ktor.http.ContentType?){}[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this change is not binary compatible. Could you add a constructor with an old signature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added constructor with an old signature
How can I run an apiDump task now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not run apiDump task on my Windows PC because ios subtasks are failing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed it with @osipxd and it's binary compatible, I reverted the last commit and will merge the PR


final fun createCopy(): io.ktor.server.plugins/UnsupportedMediaTypeException // io.ktor.server.plugins/UnsupportedMediaTypeException.createCopy|createCopy(){}[0]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<UnsupportedMediaTypeException> {

override fun createCopy(): UnsupportedMediaTypeException = UnsupportedMediaTypeException(contentType).also {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import io.ktor.http.*
import io.ktor.http.cio.*
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.*
Expand All @@ -33,16 +34,21 @@ internal actual suspend fun PipelineContext<Any, PipelineCall>.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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also one more place, where exception is thrown - CIO:
1)

val contentType = headers["Content-Type"] ?: throw IOException("Failed to parse multipart: no Content-Type header")

2)
throw IOException("Failed to parse multipart: Content-Type should be multipart/* but it is $contentType")

Can we please fix it too?


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 =
Expand Down
Original file line number Diff line number Diff line change
@@ -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<PipelineContext<*, PipelineCall>>(relaxed = true)
private val mockRequest = mockk<PipelineRequest>(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<UnsupportedMediaTypeException> {
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<Long>(any()) } returns 0L
every { mockRequest.header(HttpHeaders.ContentType) } returns contentType
every { mockRequest.header(HttpHeaders.ContentLength) } returns contentLength

// When & Then
testScope.runTest {
assertFailsWith<UnsupportedMediaTypeException> {
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)
}
}
}