KTOR-5410 Fix server ContentConverter not triggered (#3358)

* KTOR-5410 Fix server ContentConverter not triggered

ktor-server/ktor-server-plugins/ktor-server-content-negotiation/build.gradle.kts

@@ -3,3 +3,13 @@
  */
 
 description = ""
+
+kotlin {
+    sourceSets {
+        jvmAndNixTest {
+            dependencies {
+                implementation(project(":ktor-server:ktor-server-plugins:ktor-server-double-receive"))
+            }
+        }
+    }
+}

ktor-server/ktor-server-plugins/ktor-server-content-negotiation/jvm/test/ContentNegotiationJvmTest.kt

@@ -0,0 +1,154 @@
+/*
+ * Copyright 2014-2023 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ */
+package io.ktor.server.plugins.contentnegotiation
+
+import io.ktor.client.request.*
+import io.ktor.client.statement.*
+import io.ktor.http.*
+import io.ktor.http.content.*
+import io.ktor.serialization.*
+import io.ktor.server.application.*
+import io.ktor.server.request.*
+import io.ktor.server.response.*
+import io.ktor.server.routing.*
+import io.ktor.server.testing.*
+import io.ktor.server.testing.internal.*
+import io.ktor.util.reflect.*
+import io.ktor.utils.io.*
+import io.ktor.utils.io.charsets.*
+import io.ktor.utils.io.core.*
+import kotlinx.coroutines.*
+import java.io.*
+import kotlin.test.*
+import kotlin.text.toByteArray
+
+class ContentNegotiationJvmTest {
+    private val alwaysFailingConverter = object : ContentConverter {
+        override suspend fun serializeNullable(
+            contentType: ContentType,
+            charset: Charset,
+            typeInfo: TypeInfo,
+            value: Any?
+        ): OutgoingContent? {
+            fail("This converter should be never started for send")
+        }
+
+        override suspend fun deserialize(charset: Charset, typeInfo: TypeInfo, content: ByteReadChannel): Any? {
+            fail("This converter should be never started for receive")
+        }
+    }
+
+    @Test
+    fun testReceiveInputStreamTransformedByDefault(): Unit = testApplication {
+        application {
+            install(ContentNegotiation) {
+                // Order here matters. The first registered content type matching the Accept header will be chosen.
+                register(ContentType.Any, alwaysFailingConverter)
+            }
+
+            routing {
+                post("/input-stream") {
+                    val size = call.receive<InputStream>().readBytes().size
+                    call.respondText("bytes from IS: $size")
+                }
+                post("/multipart") {
+                    val multipart = call.receiveMultipart()
+                    val parts = multipart.readAllParts()
+                    call.respondText("parts: ${parts.map { it.name }}")
+                }
+            }
+        }
+
+        assertEquals("bytes from IS: 3", client.post("/input-stream") { setBody("123") }.bodyAsText())
+        client.post("/multipart") {
+            setBody(
+                buildMultipart(
+                    "my-boundary",
+                    listOf(
+                        PartData.FormItem(
+                            "test",
+                            {},
+                            headersOf(
+                                HttpHeaders.ContentDisposition,
+                                ContentDisposition(
+                                    "form-data",
+                                    listOf(
+                                        HeaderValueParam("name", "field1")
+                                    )
+                                ).toString()
+                            )
+                        )
+                    )
+                )
+            )
+            header(
+                HttpHeaders.ContentType,
+                ContentType.MultiPart.FormData.withParameter("boundary", "my-boundary").toString()
+            )
+        }.let { response ->
+            assertEquals("parts: [field1]", response.bodyAsText())
+        }
+    }
+
+    @Test
+    fun testRespondInputStream() = testApplication {
+        application {
+            routing {
+                install(ContentNegotiation) {
+                    register(ContentType.Application.Json, alwaysFailingConverter)
+                }
+                get("/") {
+                    call.respond(ByteArrayInputStream("""{"x": 123}""".toByteArray()))
+                }
+            }
+        }
+        val response = client.get("/").bodyAsText()
+        assertEquals("""{"x": 123}""", response)
+    }
+}
+
+@OptIn(DelicateCoroutinesApi::class)
+internal fun buildMultipart(
+    boundary: String,
+    parts: List<PartData>
+): ByteReadChannel = GlobalScope.writer(Dispatchers.IO) {
+    if (parts.isEmpty()) return@writer
+
+    try {
+        append("\r\n\r\n")
+        parts.forEach {
+            append("--$boundary\r\n")
+            for ((key, values) in it.headers.entries()) {
+                append("$key: ${values.joinToString(";")}\r\n")
+            }
+            append("\r\n")
+            append(
+                when (it) {
+                    is PartData.FileItem -> {
+                        channel.writeFully(it.provider().readBytes())
+                        ""
+                    }
+                    is PartData.BinaryItem -> {
+                        channel.writeFully(it.provider().readBytes())
+                        ""
+                    }
+                    is PartData.FormItem -> it.value
+                    is PartData.BinaryChannelItem -> {
+                        it.provider().copyTo(channel)
+                        ""
+                    }
+                }
+            )
+            append("\r\n")
+        }
+
+        append("--$boundary--\r\n")
+    } finally {
+        parts.forEach { it.dispose() }
+    }
+}.channel
+
+private suspend fun WriterScope.append(str: String, charset: Charset = Charsets.UTF_8) {
+    channel.writeFully(str.toByteArray(charset))
+}

ktor-server/ktor-server-plugins/ktor-server-content-negotiation/jvmAndNix/src/io/ktor/server/plugins/contentnegotiation/RequestConverter.kt

@@ -73,6 +73,6 @@ private suspend fun convertBody(
         convertedBody != null -> convertedBody
         !body.isClosedForRead -> body
         receiveType.kotlinType?.isMarkedNullable == true -> NullBody
-        else -> throw BadRequestException("Cannot convert request body to ${receiveType.type}")
+        else -> null
     }
 }

ktor-server/ktor-server-tests/jvmAndNix/test/io/ktor/tests/server/plugins/ContentNegotiationTest.kt → ktor-server/ktor-server-plugins/ktor-server-content-negotiation/jvmAndNix/test/ContentNegotiationTest.kt

@@ -1,17 +1,17 @@
 /*
- * Copyright 2014-2019 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ * Copyright 2014-2023 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
  */
 
-package io.ktor.tests.server.plugins
+package io.ktor.server.plugins.contentnegotiation
 
+import io.ktor.client.call.*
 import io.ktor.client.request.*
 import io.ktor.client.statement.*
 import io.ktor.http.*
 import io.ktor.http.content.*
 import io.ktor.serialization.*
 import io.ktor.server.application.*
 import io.ktor.server.plugins.*
-import io.ktor.server.plugins.contentnegotiation.*
 import io.ktor.server.plugins.doublereceive.*
 import io.ktor.server.request.*
 import io.ktor.server.response.*
@@ -25,55 +25,152 @@ import kotlin.test.*
 
 @Suppress("DEPRECATION")
 class ContentNegotiationTest {
-    private val customContentType = ContentType.parse("application/ktor")
 
-    private val customContentConverter = object : ContentConverter {
+    private val alwaysFailingConverter = object : ContentConverter {
         override suspend fun serializeNullable(
             contentType: ContentType,
             charset: Charset,
             typeInfo: TypeInfo,
             value: Any?
         ): OutgoingContent? {
-            if (value !is Wrapper) return null
-            return TextContent("[${value.value}]", contentType.withCharset(charset))
+            fail("This converter should be never started for send")
         }
 
         override suspend fun deserialize(charset: Charset, typeInfo: TypeInfo, content: ByteReadChannel): Any? {
-            if (typeInfo.type != Wrapper::class) return null
-            return Wrapper(content.readRemaining().readText().removeSurrounding("[", "]"))
+            fail("This converter should be never started for receive")
         }
     }
 
-    private val textContentConverter = object : ContentConverter {
+    @Test
+    fun testRespondByteArray() = testApplication {
+        application {
+            routing {
+                install(ContentNegotiation) {
+                    register(ContentType.Application.Json, alwaysFailingConverter)
+                }
+                get("/") {
+                    call.respond("test".toByteArray())
+                }
+            }
+        }
+        val response = client.get("/").body<ByteArray>()
+        assertContentEquals("test".toByteArray(), response)
+    }
+
+    object OK
+
+    @Test
+    fun testMultipleConverters() = testApplication {
+        var nullSerialized = false
+        var nullDeserialized = false
+        var okSerialized = false
+        var okDeserialized = false
+
+        application {
+            routing {
+                install(ContentNegotiation) {
+                    val nullConverter = object : ContentConverter {
+                        override suspend fun serializeNullable(
+                            contentType: ContentType,
+                            charset: Charset,
+                            typeInfo: TypeInfo,
+                            value: Any?
+                        ): OutgoingContent? {
+                            nullSerialized = true
+                            return null
+                        }
+
+                        override suspend fun deserialize(
+                            charset: Charset,
+                            typeInfo: TypeInfo,
+                            content: ByteReadChannel
+                        ): Any? {
+                            nullDeserialized = true
+                            return null
+                        }
+                    }
+                    val okConverter = object : ContentConverter {
+                        override suspend fun serializeNullable(
+                            contentType: ContentType,
+                            charset: Charset,
+                            typeInfo: TypeInfo,
+                            value: Any?
+                        ): OutgoingContent {
+                            okSerialized = true
+                            return TextContent("OK", contentType)
+                        }
+
+                        override suspend fun deserialize(
+                            charset: Charset,
+                            typeInfo: TypeInfo,
+                            content: ByteReadChannel
+                        ): Any {
+                            okDeserialized = true
+                            return OK
+                        }
+                    }
+
+                    register(ContentType.Application.Json, nullConverter)
+                    register(ContentType.Application.Json, okConverter)
+                }
+
+                get("/FOO") {
+                    try {
+                        call.receive<OK>()
+                        call.respond(OK)
+                    } catch (cause: Throwable) {
+                        call.respond(cause)
+                    }
+                }
+            }
+        }
+
+        val response = client.get("FOO") {
+            accept(ContentType.Application.Json)
+            contentType(ContentType.Application.Json)
+        }
+
+        assertEquals("OK", response.bodyAsText())
+
+        assertTrue(nullSerialized)
+        assertTrue(nullDeserialized)
+        assertTrue(okSerialized)
+        assertTrue(okDeserialized)
+    }
+
+    private val customContentType = ContentType.parse("application/ktor")
+
+    private val customContentConverter = object : ContentConverter {
         override suspend fun serializeNullable(
             contentType: ContentType,
             charset: Charset,
             typeInfo: TypeInfo,
             value: Any?
         ): OutgoingContent? {
             if (value !is Wrapper) return null
-            return TextContent(value.value, contentType.withCharset(charset))
+            return TextContent("[${value.value}]", contentType.withCharset(charset))
         }
 
         override suspend fun deserialize(charset: Charset, typeInfo: TypeInfo, content: ByteReadChannel): Any? {
             if (typeInfo.type != Wrapper::class) return null
-            return Wrapper(content.readRemaining().readText())
+            return Wrapper(content.readRemaining().readText().removeSurrounding("[", "]"))
         }
     }
 
-    private fun alwaysFailingConverter(ignoreString: Boolean) = object : ContentConverter {
+    private val textContentConverter = object : ContentConverter {
         override suspend fun serializeNullable(
             contentType: ContentType,
             charset: Charset,
             typeInfo: TypeInfo,
             value: Any?
         ): OutgoingContent? {
-            fail("This converter should be never started for send")
+            if (value !is Wrapper) return null
+            return TextContent(value.value, contentType.withCharset(charset))
         }
 
         override suspend fun deserialize(charset: Charset, typeInfo: TypeInfo, content: ByteReadChannel): Any? {
-            if (ignoreString && typeInfo.type == String::class) return null
-            fail("This converter should be never started for receive")
+            if (typeInfo.type != Wrapper::class) return null
+            return Wrapper(content.readRemaining().readText())
         }
     }
 
@@ -444,7 +541,7 @@ class ContentNegotiationTest {
             }
         }
 
-       client.post("/text") {
+        client.post("/text") {
             setBody("\"k=v\"")
             contentType(ContentType.Application.Json)
         }.let { response ->