KTOR-8015 CIO: Do not accept CR as line delimiter in requests #4668
Conversation
...ver/ktor-server-test-suites/jvm/src/io/ktor/server/testing/suites/SustainabilityTestSuite.kt
Outdated
Show resolved
Hide resolved
| @@ -395,6 +396,32 @@ public suspend fun ByteReadChannel.discard(max: Long = Long.MAX_VALUE): Long { | |||
| private const val CR: Byte = '\r'.code.toByte() | |||
| private const val LF: Byte = '\n'.code.toByte() | |||
|
|
|||
| @JvmInline | |||
| public value class LineEndingMode private constructor(private val mode: Int) { | |||
There was a problem hiding this comment.
I could fix the problem only for chunked body, but it seems we should also fix line endings in request/response parsing.
...ver/ktor-server-test-suites/jvm/src/io/ktor/server/testing/suites/SustainabilityTestSuite.kt
Outdated
Show resolved
Hide resolved
| private fun LineEndingMode.assertIncludes(other: LineEndingMode) { | ||
| if (other !in this) { | ||
| throw EOFException("Unexpected line ending $other, while expected $this") | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think EOF is technically correct since it's malformed data but the input hasn't ended, maybe just an IOException would do.
Could there be scenarios here where you'd simply want to ignore the partial line ending?
There was a problem hiding this comment.
I don't think EOF is technically correct
Changed to IOException
Could there be scenarios here where you'd simply want to ignore the partial line ending?
That is how Netty works. It just ignores everything until the LF line ending. It makes a server more tolerant to malformed requests. The question is, do we want to make it tolerant?
There was a problem hiding this comment.
I think it's better to throw an exception as Jetty and Tomcat do it. Also, a user says that the behavior of Google Load Balancer (in Netty it's the same: parse input as one request with two chunks) is incorrect for them
osipxd
force-pushed
the
osipxd/kmp-engine-test-base
branch
3 times, most recently
from
8a50989 to
4231ba6
Compare
osipxd
changed the title
osipxd
force-pushed
the
osipxd/kmp-engine-test-base
branch
3 times, most recently
from
9e166eb to
148dc2c
Compare
| } | ||
|
|
||
| @OptIn(InternalAPI::class, InternalIoApi::class) | ||
| public suspend fun ByteReadChannel.readUTF8LineTo( |
There was a problem hiding this comment.
I will add KDoc comments if this solution with the new API is acceptable
osipxd
force-pushed
the
osipxd/kmp-engine-test-base
branch
from
148dc2c to
ef09f08
Compare
Subsystem
Server (CIO)
Motivation
KTOR-8015 Server accepts \r without a following \n as a valid line terminator in chunked transfer encoding
Solution
Add a parameter
lineEndingtoreadUTF8Lineto make it possible to ensure that a proper line ending is used.