KTOR-9171 Redesign ByteReadChannel.readUTF8Line API #5236
Conversation
WalkthroughReplaces legacy UTF‑8 line readers with a new line-reading API ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
osipxd
force-pushed
the
osipxd/optimize-read-line
branch
from
20e4128 to
e396244
Compare
| } | ||
|
|
||
| @Test | ||
| fun `test reading long lines completes in a reasonable time`() = runTest(timeout = 5.seconds) { |
There was a problem hiding this comment.
This test times out. It seems too low timeout for CI agent.
Probably we should remove these two tests and just run io-benchmarks 🤔
There was a problem hiding this comment.
Yeah, the CI has proven itself to be unreasonable 😅
| val lfIndex = if (lineEnding != LineEndingMode.CR) readBuffer.indexOf(LF) else -1 | ||
| val crIndex = if (allowsCR) { | ||
| // Index of lone CR delimiter. | ||
| // We're interested in it only if the requested line ending mode allows CR as a line delimiter. | ||
| val crEndIndex = if (lfIndex >= 0L) (lfIndex - crEndIndexCorrection).coerceAtLeast(0) else Long.MAX_VALUE | ||
| readBuffer.indexOf(CR, endIndex = crEndIndex) |
There was a problem hiding this comment.
Handling different line ending modes turned out to be quite annoying. I hope the code is clear with comments. Please point out any unclear moments.
There was a problem hiding this comment.
Kinda looks like it may be a bit cleaner if we just had when(lineEnding) { ... } with separate functions for each strategy, then we can use the direct calls internally for an optimisation where relevant.
There was a problem hiding this comment.
Do you mean create a when with 7 branches? (CR + LF + CRLF, CR + LF, CR + CRLF, LF + CRLF, CR, LF, CRLF). It's sad that I've introduced such a flexible parameter, I think some of these combinations don't make any sense 😅
There was a problem hiding this comment.
Oh yeah that would be a bit unwieldy. I thought it might be like 4 branches 😅
Maybe it would make sense to just have a function for CRLF + LF, then keep this more generous implementation for all other branches - then we call the CRLF + LF one directly for the better performance.
osipxd
force-pushed
the
osipxd/optimize-read-line
branch
from
e396244 to
ab9e968
Compare
| fun transferString(count: Long) { | ||
| if (count > 0L) { | ||
| val string = readBuffer.readString(count) | ||
| out.append(string) | ||
| consumed += string.length | ||
| } | ||
| } |
There was a problem hiding this comment.
We could avoid one extra copy here if we introduce a Source.readTo(Appendable) function in kotlinx-io. Maybe we can make a contribution for the next release. If we wanted to do this without changing kotlinx-io it seems that would require a fair bit of duplication because most of their functions are internal in this area.
There was a problem hiding this comment.
Yes, also thought about it. My plan was to implement readStringTo inside Ktor and measure it's performance compared to readString. Then go to kotlinx-io guys with this data
There was a problem hiding this comment.
Created an issue for further improvements: KTOR-9178
| val lfIndex = readBuffer.indexOf(LF, endIndex = limitLeft) | ||
|
|
||
| if (lenientLineEnding) { | ||
| val crEndIndex = when (lfIndex) { | ||
| // Subtract 1 from source.remaining to ignore the last byte, | ||
| // when RC might be part of a CRLF sequence split into two buffers | ||
| -1L -> minOf(limitLeft, readBuffer.remaining - 1) | ||
| 0L -> 0 | ||
| // Subtract 1 from lfIndex to ignore the case when CR is part of the CRLF sequence | ||
| else -> lfIndex - 1 | ||
| } | ||
| if (lineBuffer.size >= max) { | ||
| throw TooLongLineException("Line exceeds limit of $max characters") | ||
| val crIndex = readBuffer.indexOf(CR, endIndex = crEndIndex) | ||
|
|
||
| // Sole CR in the buffer | ||
| if (crIndex >= 0) { | ||
| transferString(count = crIndex) | ||
| readBuffer.discard(1) | ||
| return consumed | ||
| } | ||
| } |
There was a problem hiding this comment.
This would be performing two scans of the buffer here... Maybe it would be better to implement a custom Source.indexOf() for the two different bytes for the CR / LF case. I think it would be cleaner to just have two separate functions instead of including the CR only case inside this one. There'll be a bit of duplication, but it should be easier to follow.
There was a problem hiding this comment.
Yes, initially I made two different functions, but it was hard to fix bugs in two places 😅 And in this case we would need to duplicate all tests for both line ending modes to verify these functions actually behave similarly.
So I thought that if this extra logic for CR handling was guarded with a simple if it would be easy to follow the logic ignoring these 'if's for no-CR case.
Do you think we should split these functions?
There was a problem hiding this comment.
Maybe it would be better to implement a custom Source.indexOf() for the two different bytes for the CR / LF case
Good idea! I'll try to do it in the scope of KTOR-9178
There was a problem hiding this comment.
Do you think we should split these functions?
Maybe if splitting them out isn't viable, you could try function composition?
Something like...
@OptIn(InternalAPI::class, InternalIoApi::class)
public suspend fun ByteReadChannel.readLineStrictTo(
out: Appendable,
limit: Long = Long.MAX_VALUE,
lineEnding: LineEnding = LineEnding.Default,
): Long {
require(limit >= 0) { "Limit ($limit) should be non-negative" }
return when(lineEnding) {
LineEnding.Default -> internalReadLineTo(
out,
limit,
throwOnIncompleteLine = true,
scanForCr = DefaultScan,
discardCr = DefaultDiscard,
)
LineEnding.Lenient -> internalReadLineTo(
out,
limit,
throwOnIncompleteLine = true,
scanForCr = LenientScan,
discardCr = LenientDiscard,
)
}
}
@OptIn(InternalAPI::class, InternalIoApi::class)
private suspend fun ByteReadChannel.internalReadLineTo(
out: Appendable,
limit: Long,
throwOnIncompleteLine: Boolean,
scanForCr: Source.(Long, Long) -> Long,
discardCr: Source.() -> Boolean,
): Long
Then you could use inline / crossinline for slightly better performance 🤷
There was a problem hiding this comment.
Do you mean something like this?
// Default
scanForCr = { _, _ -> -1L }
discardCr = { false }
// Lenient
scanForCr = { lfIndex, limitLeft ->
val endIndex = when (lfIndex) {
// Subtract 1 from source.remaining to ignore the last byte,
// when RC might be part of a CRLF sequence split into two buffers
-1L -> minOf(limitLeft, remaining - 1)
0L -> 0
// Subtract 1 from lfIndex to ignore the case when CR is part of the CRLF sequence
else -> lfIndex - 1
}
indexOf(CR, endIndex = endIndex)
}
discardCr = {
discard(1)
true
}Maybe it will be enough to just introduce an inner function scanForCr?
fun Source.scanForCr(lfIndex: Long, limitLeft: Long): Long {
// Don't scan for CR in default mode
if (!lenientLineEnding) return -1L
val endIndex = ...
return indexOf(CR, endIndex = endIndex)
}There was a problem hiding this comment.
You can check the variant with an inner function in this commit: a9ac7a8
There was a problem hiding this comment.
Yeah, that's what I was thinking. Just to shift the branching up a higher level. It's fine with the checks inside the function, it just adds a little more complexity.
There was a problem hiding this comment.
I've failed to make this function inline as inline functions don't support inner functions 😢
So I'd proceed with an inner scanForCr function. WDYT?
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
Outdated
Show resolved
Hide resolved
| fun transferString(count: Long) { | ||
| if (count > 0L) { | ||
| val string = readBuffer.readString(count) | ||
| out.append(string) | ||
| consumed += string.length | ||
| } | ||
| } |
There was a problem hiding this comment.
Yes, also thought about it. My plan was to implement readStringTo inside Ktor and measure it's performance compared to readString. Then go to kotlinx-io guys with this data
| val lfIndex = readBuffer.indexOf(LF, endIndex = limitLeft) | ||
|
|
||
| if (lenientLineEnding) { | ||
| val crEndIndex = when (lfIndex) { | ||
| // Subtract 1 from source.remaining to ignore the last byte, | ||
| // when RC might be part of a CRLF sequence split into two buffers | ||
| -1L -> minOf(limitLeft, readBuffer.remaining - 1) | ||
| 0L -> 0 | ||
| // Subtract 1 from lfIndex to ignore the case when CR is part of the CRLF sequence | ||
| else -> lfIndex - 1 | ||
| } | ||
| if (lineBuffer.size >= max) { | ||
| throw TooLongLineException("Line exceeds limit of $max characters") | ||
| val crIndex = readBuffer.indexOf(CR, endIndex = crEndIndex) | ||
|
|
||
| // Sole CR in the buffer | ||
| if (crIndex >= 0) { | ||
| transferString(count = crIndex) | ||
| readBuffer.discard(1) | ||
| return consumed | ||
| } | ||
| } |
There was a problem hiding this comment.
Yes, initially I made two different functions, but it was hard to fix bugs in two places 😅 And in this case we would need to duplicate all tests for both line ending modes to verify these functions actually behave similarly.
So I thought that if this extra logic for CR handling was guarded with a simple if it would be easy to follow the logic ignoring these 'if's for no-CR case.
Do you think we should split these functions?
| val lfIndex = readBuffer.indexOf(LF, endIndex = limitLeft) | ||
|
|
||
| if (lenientLineEnding) { | ||
| val crEndIndex = when (lfIndex) { | ||
| // Subtract 1 from source.remaining to ignore the last byte, | ||
| // when RC might be part of a CRLF sequence split into two buffers | ||
| -1L -> minOf(limitLeft, readBuffer.remaining - 1) | ||
| 0L -> 0 | ||
| // Subtract 1 from lfIndex to ignore the case when CR is part of the CRLF sequence | ||
| else -> lfIndex - 1 | ||
| } | ||
| if (lineBuffer.size >= max) { | ||
| throw TooLongLineException("Line exceeds limit of $max characters") | ||
| val crIndex = readBuffer.indexOf(CR, endIndex = crEndIndex) | ||
|
|
||
| // Sole CR in the buffer | ||
| if (crIndex >= 0) { | ||
| transferString(count = crIndex) | ||
| readBuffer.discard(1) | ||
| return consumed | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe it would be better to implement a custom Source.indexOf() for the two different bytes for the CR / LF case
Good idea! I'll try to do it in the scope of KTOR-9178
| if (consumed == 0L && isClosedForRead) return -1 | ||
|
|
||
| // Defensive check. Normally the consumed count should never exceed the limit | ||
| if (consumed > limit) throwTooLongLineException(limit) |
There was a problem hiding this comment.
Question for review: Maybe IllegalStateException should be thrown here, I'm not sure
There was a problem hiding this comment.
Yeah, looks like a good place for a check().
| public suspend fun ByteReadChannel.readLineTo( | ||
| out: Appendable, | ||
| lineEnding: LineEnding = LineEnding.Default | ||
| ): Long { |
There was a problem hiding this comment.
I've removed limit from non-strict readLineTo as it was not obvious what to do when we reach the limit. We shouldn't throw, but returning a truncated string doesn't make sense. It also aligns with kotlinx-io.
| @OptIn(InternalAPI::class, InternalIoApi::class) | ||
| public suspend fun ByteReadChannel.readLineStrictTo( | ||
| out: Appendable, | ||
| limit: Long = Long.MAX_VALUE, |
There was a problem hiding this comment.
Question for review: Changed the type of limit to Long as kotlinx-io operates with long. However I'm not sure if it's good or bad for the API. In one hand we don't limit users with Int.MAX_VALUE, but in other hand this might require adding .toLong() in existing code.
There was a problem hiding this comment.
I'm ok with Long here. Best to keep it consistent, though I can't imagine anyone using a value outside the max integer range.
osipxd
force-pushed
the
osipxd/optimize-read-line
branch
4 times, most recently
from
aed91ae to
f963802
Compare
osipxd
changed the title
osipxd
changed the title
osipxd
force-pushed
the
osipxd/optimize-read-line
branch
2 times, most recently
from
1e181b7 to
f8cfa5f
Compare
| /** | ||
| * Reads a line of UTF-8 characters to the specified [out] buffer. | ||
| * It recognizes CR, LF and CRLF as a line delimiter. | ||
| * It recognizes CR, LF, and CRLF as a line delimiter. | ||
| * | ||
| * ## Deprecation Notes | ||
| * | ||
| * This function is deprecated in favor of [readLineStrictTo] and [readLineTo]. | ||
| * | ||
| * Use [readLineStrictTo] when: | ||
| * - Need to limit line length and throw [TooLongLineException] if the limit is reached | ||
| * - Expect an explicit line break at the end of a line. [EOFException] will be thrown | ||
| * if the channel is closed before the line break | ||
| * | ||
| * Use [readLineTo] when: | ||
| * - No need to limit line length | ||
| * - An explicit line break at the end of the channel is not mandatory | ||
| * | ||
| * Other changes: | ||
| * - New functions recognize LF and CRLF as a line delimiter by default. This default comes with better performance. | ||
| * To keep current behavior and recognize all line delimiters (CR, LF, and CRLF), specify [LineEnding.Lenient]. | ||
| * - New functions return number of appended characters instead of [Boolean] or `-1` if the channel is empty. | ||
| * - [readLineStrictTo] accepts [Long] instead of [Int] as a limit parameter. | ||
| * | ||
| * The direct equivalent of `readUTF8LineTo` would be: | ||
| * ``` | ||
| * // Before | ||
| * val success = channel.readUTF8LineTo(out, max = 1024) | ||
| * | ||
| * // After | ||
| * val success = try { | ||
| * channel.readLineStrictTo(out, limit = 1024, lineEnding = LineEnding.Lenient) >= 0 | ||
| * } catch (_: EOFException) { | ||
| * true | ||
| * } | ||
| * ``` | ||
| * However, we recommend to use [LineEnding.Default] if possible and verify if the case with | ||
| * the unexpected end of line should actually be ignored. We expect the following code to be correct in most cases: | ||
| * ``` | ||
| * val success = channel.readLineTo(out, limit = 1024) >= 0 | ||
| * ``` |
There was a problem hiding this comment.
@bjhham, @vnikolova, could you check KDocs here? I suspect they're not actionable enough, but I don't how to improve yet keep it simple
There was a problem hiding this comment.
It seems like enough information for me. It might be good to link to the relevant kotlinx-io function docs too.
There was a problem hiding this comment.
I suggested some small improvements for added clarity and actionability, but it generally looks good as it is!
| /** | ||
| * Reads a line of UTF-8 characters from the `ByteReadChannel`. | ||
| * It recognizes CR, LF, and CRLF as a line delimiter. | ||
| * | ||
| * ## Deprecation Notes | ||
| * | ||
| * This function is deprecated in favor of [readLineStrict] and [readLine]. | ||
| * | ||
| * Use [readLineStrict] when: | ||
| * - Need to limit line length and throw [TooLongLineException] if the limit is reached | ||
| * - Expect an explicit line break at the end of a line. [EOFException] will be thrown | ||
| * if the channel is closed before the line break | ||
| * | ||
| * Use [readLine] when: | ||
| * - No need to limit line length | ||
| * - An explicit line break at the end of the channel is not mandatory | ||
| * | ||
| * Other changes: | ||
| * - New functions recognize LF and CRLF as a line delimiter by default. This default comes with better performance. | ||
| * To keep current behavior and recognize all line delimiters (CR, LF, and CRLF), specify [LineEnding.Lenient]. | ||
| * - [readLineStrict] accepts [Long] instead of [Int] as a limit parameter. | ||
| * | ||
| * The direct equivalent of `readUTF8Line` would be: | ||
| * ``` | ||
| * // Before | ||
| * val line = channel.readUTF8Line(max = 1024) | ||
| * | ||
| * // After | ||
| * val buffer = StringBuilder() | ||
| * val success = try { | ||
| * channel.readLineStrictTo(out, limit = 1024, lineEnding = LineEnding.Lenient) >= 0 | ||
| * } catch (_: EOFException) { | ||
| * true | ||
| * } | ||
| * val line = if (success) buffer.toString() else null | ||
| * ``` | ||
| * However, we recommend to use [LineEnding.Default] if possible and verify if the case with | ||
| * the unexpected end of line should actually be ignored. We expect the following code to be correct in most cases: | ||
| * ``` | ||
| * val line = channel.readLineStrict(limit = 1024) | ||
| * ``` | ||
| * | ||
| * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.utils.io.readUTF8Line) | ||
| * | ||
| * @param max the maximum number of characters to read. Default is [Int.MAX_VALUE]. | ||
| * @return a string containing the line read, or null if channel is closed | ||
| * @throws TooLongLineException if max is reached before encountering a line delimiter or end of input | ||
| */ | ||
| @Suppress("DEPRECATION") | ||
| @Deprecated("Use readLineStrict instead. See deprecation notes for more details.") | ||
| public suspend fun ByteReadChannel.readUTF8Line(max: Int = Int.MAX_VALUE): String? { |
There was a problem hiding this comment.
These KDoc are almost similar to the first one, but for a function returning String.
| /** | ||
| * Reads a line of UTF-8 characters to the specified [out] buffer. | ||
| * | ||
| * Appends UTF-8 characters until a line break is found or the channel is exhausted. Implicit line break | ||
| * is assumed if the channel ends without a line break. Line break characters are not included in the result. | ||
| * | ||
| * @param out the buffer to append line to (line break characters are not appended) | ||
| * @param lineEnding line ending mode. Accepts LF and CRLF by default. | ||
| * | ||
| * @return number of characters appended to [out], or `-1` if can't read from the channel | ||
| */ |
| /** | ||
| * Reads a line of UTF-8 characters to the specified [out] buffer. | ||
| * Throws an exception if the line exceeds [limit] or doesn't end with a line break. | ||
| * | ||
| * The [limit] counts **characters** after UTF-8 decoding, not bytes. | ||
| * Line break characters are not included in the result and not counted towards the [limit]. | ||
| * So this function may scan up to `limit + 2` characters (when line ends with CRLF after limit). | ||
| * | ||
| * **Important**: This function consumes bytes from the channel and appends them to [out] | ||
| * even if it fails to complete. | ||
| * | ||
| * @param out the buffer to append line to (line break characters are not appended) | ||
| * @param limit maximum characters to append to [out]. Unlimited by default. | ||
| * @param lineEnding line ending mode. Accepts LF and CRLF by default. | ||
| * | ||
| * @return number of characters appended to [out], or `-1` if can't read from the channel | ||
| * @throws TooLongLineException if the line exceeds [limit] | ||
| * @throws EOFException if the channel is closed before a line break is found | ||
| */ | ||
| @OptIn(InternalAPI::class, InternalIoApi::class) | ||
| public suspend fun ByteReadChannel.readLineStrictTo( |
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ktor-test-dispatcher/common/src/TestCommon.kt (1)
41-50: Avoidcontextname shadowing + consider preserving non-dispatcher context elements.
- Line 46 shadows the
contextparameter withval context = ..., which makes this helper harder to reason about.- Also, the current logic discards the caller’s context entirely unless the dispatcher is a
TestDispatcher. If the caller passedCoroutineName, MDC context, etc., those get dropped unexpectedly.Suggested tweak (rename + keep extras while normalizing dispatcher):
public fun runTestWithRealTime( context: CoroutineContext = EmptyCoroutineContext, timeout: Duration = 60.seconds, testBody: suspend CoroutineScope.() -> Unit ): TestResult { - val context = if (context[CoroutineDispatcher] is TestDispatcher) context else EmptyCoroutineContext - return runTest(context, timeout) { + val runTestContext = + if (context[CoroutineDispatcher] is TestDispatcher) context else context.minusKey(CoroutineDispatcher) + return runTest(runTestContext, timeout) { withContext(Dispatchers.Default.limitedParallelism(1), testBody) } }If you intended to hard-reset all context elements (not just dispatcher), please add a short comment stating why. (Based on coding guidelines: keep API behavior intentional and well-documented.)
♻️ Duplicate comments (2)
ktor-io/common/test/ReadLineTest.kt (1)
17-73: Potential CI flakiness ifwriter {}escapesrunTestscheduler (realdelay(100)).
Given earlier feedback about timeouts in this area, please double-check these tests don’t sleep in real time on CI. If they do, prefer advancing virtual time (or structuring the test without time-based delays).Also applies to: 165-186
ktor-io/common/test/ReadUtf8LineTest.kt (1)
37-43: Watch for CI timeouts if thesedelay(100)calls become real-time.
Given prior review feedback on timeouts in this area, please confirmrunTestvirtual time is actually driving these writers on CI; otherwise restructure to avoid wall-clock delays.Also applies to: 66-80
🧹 Nitpick comments (5)
ktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.kt (1)
93-96:readLineStrict()usage is appropriate, but this test now assumes the 100-Continue response has no headers.
If aDate/other header ever gets added to the100 Continueresponse,assertEquals("", readChannel.readLineStrict())will fail because the next line wouldn’t be blank. Consider either (a) reading until the first blank line, or (b) explicitly asserting there are no headers (if that’s a protocol/implementation guarantee you want to lock in).ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.kt (1)
21-23: Keep kotlin.test imports consistent with the module’s conventions
If this repo/module typically usesimport kotlin.test.*, consider matching that to reduce churn; otherwise this is fine.ktor-io/common/test/ReadLineTest.kt (1)
312-327:assertLines()assumes returned count matches KotlinString.length(UTF-16 code units).
If the API contract is “number of read symbols” (per PR motivation), consider adding at least one non-ASCII line (e.g.,€, emoji, combining marks) to pin down the intended counting semantics.ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt (1)
40-46: Consider looping onreadLineStrict()result instead ofisClosedForRead(avoids edge-case spin/hang).
Optional hardening:- while (!channel.isClosedForRead) { - channel.readLineStrict() - } + while (channel.readLineStrict() != null) { + // consume + }ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (1)
555-565: Fragile error detection in deprecated wrapper.The check
cause.message?.startsWith("Unexpected end of stream after reading")is brittle—if the message wording changes, this will break silently. Consider using a more robust detection mechanism or a specific exception subtype.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt(2 hunks)ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.kt(3 hunks)ktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.kt(2 hunks)ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.kt(2 hunks)ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.kt(3 hunks)ktor-client/ktor-client-okhttp/jvm/test/io/ktor/client/engine/okhttp/OkHttpHttp2Test.kt(2 hunks)ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/HttpTimeoutTest.kt(3 hunks)ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.kt(1 hunks)ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt(1 hunks)ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt(2 hunks)ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt(3 hunks)ktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.kt(1 hunks)ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt(6 hunks)ktor-io/api/ktor-io.api(2 hunks)ktor-io/api/ktor-io.klib.api(2 hunks)ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt(4 hunks)ktor-io/common/src/io/ktor/utils/io/LineEnding.kt(1 hunks)ktor-io/common/src/io/ktor/utils/io/LineEndingMode.kt(1 hunks)ktor-io/common/test/ByteReadChannelOperationsTest.kt(2 hunks)ktor-io/common/test/ReadLineTest.kt(1 hunks)ktor-io/common/test/ReadUtf8LineTest.kt(4 hunks)ktor-io/jvm/test/ByteReadChannelOperationsJvmTest.kt(1 hunks)ktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.kt(4 hunks)ktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.kt(2 hunks)ktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.kt(1 hunks)ktor-network/jvm/test/io/ktor/network/sockets/tests/ServerSocketTest.kt(1 hunks)ktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.kt(2 hunks)ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.kt(2 hunks)ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.kt(2 hunks)ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt(2 hunks)ktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.kt(5 hunks)ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt(2 hunks)ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.kt(3 hunks)ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.kt(3 hunks)ktor-test-dispatcher/common/src/TestCommon.kt(1 hunks)ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.ktktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-io/common/src/io/ktor/utils/io/LineEnding.ktktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.ktktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.ktktor-network/jvm/test/io/ktor/network/sockets/tests/ServerSocketTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-test-dispatcher/common/src/TestCommon.ktktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.ktktor-io/common/src/io/ktor/utils/io/LineEndingMode.ktktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.ktktor-client/ktor-client-okhttp/jvm/test/io/ktor/client/engine/okhttp/OkHttpHttp2Test.ktktor-io/common/test/ReadLineTest.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-client/ktor-client-tests/common/test/io/ktor/client/tests/HttpTimeoutTest.ktktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.ktktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.ktktor-io/common/test/ReadUtf8LineTest.kt
**/test/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Follow test naming pattern:
DescribeWhatIsBeingTested
Files:
ktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.ktktor-network/jvm/test/io/ktor/network/sockets/tests/ServerSocketTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.ktktor-client/ktor-client-okhttp/jvm/test/io/ktor/client/engine/okhttp/OkHttpHttp2Test.ktktor-io/common/test/ReadLineTest.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-client/ktor-client-tests/common/test/io/ktor/client/tests/HttpTimeoutTest.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.ktktor-io/common/test/ReadUtf8LineTest.kt
🧠 Learnings (27)
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.
Applied to files:
ktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-test-dispatcher/common/src/TestCommon.ktktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-io/common/test/ReadLineTest.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-27T10:07:26.686Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Applied to files:
ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-27T10:07:51.060Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/libcurl/ports/curl/dependencies.patch:156-160
Timestamp: 2025-11-27T10:07:51.060Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/libcurl/ports/*` as these are vcpkg port files copied from the vcpkg repository and maintained upstream, not by the Ktor team.
Applied to files:
ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.kt
📚 Learning: 2025-09-30T07:52:14.769Z
Learnt from: yschimke
Repo: ktorio/ktor PR: 4013
File: ktor-client/ktor-client-android/jvm/src/io/ktor/client/engine/android/Android14URLConnectionFactory.kt:24-26
Timestamp: 2025-09-30T07:52:14.769Z
Learning: In the Ktor Android HTTP client engine (ktor-client-android), prefer using `URI.create(urlString).toURL()` over the `URL(urlString)` constructor when opening connections with Android's HttpEngine, as it avoids deprecated APIs and the different exception behavior (IllegalArgumentException vs MalformedURLException) is acceptable.
Applied to files:
ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.kt
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Error handling should follow Kotlin conventions and use specific Ktor exceptions.
Applied to files:
ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Error handling follows Kotlin conventions with specific Ktor exceptions
Applied to files:
ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Use star imports for `io.ktor.*` packages
Applied to files:
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.ktktor-test-dispatcher/common/src/TestCommon.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-test-dispatcher/common/src/TestCommon.ktktor-io/common/src/io/ktor/utils/io/LineEndingMode.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-io/api/ktor-io.klib.apiktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.ktktor-io/common/test/ReadUtf8LineTest.ktktor-io/api/ktor-io.api
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-network/jvm/test/io/ktor/network/sockets/tests/ServerSocketTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/test/**/*.kt : Follow test naming pattern: `DescribeWhatIsBeingTested`
Applied to files:
ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-test-dispatcher/common/src/TestCommon.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-io/api/ktor-io.klib.apiktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-io/api/ktor-io.klib.apiktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/api/ktor-io.api
📚 Learning: 2025-06-09T07:08:35.085Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Applied to files:
ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.ktktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.ktktor-client/ktor-client-okhttp/jvm/test/io/ktor/client/engine/okhttp/OkHttpHttp2Test.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-08-14T15:17:11.466Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts:12-12
Timestamp: 2025-08-14T15:17:11.466Z
Learning: The Cargo plugin (dev.gobley.cargo) used in the Ktor WebRTC RS module depends on the kotlin("plugin.atomicfu") plugin, so atomicfu should not be removed even if there are no direct kotlinx.atomicfu imports in the module's source code.
Applied to files:
ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.kt
📚 Learning: 2025-08-14T15:17:11.466Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts:12-12
Timestamp: 2025-08-14T15:17:11.466Z
Learning: The Gobley Cargo plugin (dev.gobley.cargo) used in the Ktor WebRTC RS module requires the kotlin("plugin.atomicfu") plugin as a dependency, so the atomicfu plugin should not be removed even if there are no direct kotlinx.atomicfu imports in the module's source code.
Applied to files:
ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.kt
📚 Learning: 2025-09-05T12:47:49.016Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:35-52
Timestamp: 2025-09-05T12:47:49.016Z
Learning: JettyWebsocketConnection in ktor-server-jetty-jakarta implements Closeable interface (has close() method), so connection.use { } is valid Kotlin syntax and will compile correctly.
Applied to files:
ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-05-14T18:17:03.059Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt:37-38
Timestamp: 2025-05-14T18:17:03.059Z
Learning: When implementing CoroutineScope in Ktor components like DependencyRegistry, always ensure the scope is properly cancelled during application shutdown to prevent memory leaks and resource issues. In the DI plugin, this is handled by calling registry.cancel() in the ApplicationStopped event handler after all dependency cleanup is complete.
Applied to files:
ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.kt
📚 Learning: 2025-09-05T12:46:14.074Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:90-100
Timestamp: 2025-09-05T12:46:14.074Z
Learning: The ByteReadChannel.readAvailable(ByteBuffer) method in Ktor IO automatically calls awaitContent() internally when the read buffer is exhausted. When it returns 0, it has already suspended and waited for data to become available, so adding explicit awaitContent() calls is redundant and incorrect.
Applied to files:
ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.ktktor-network/jvm/test/io/ktor/network/sockets/tests/ServerSocketTest.ktktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.ktktor-io/api/ktor-io.klib.apiktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.ktktor-io/common/test/ReadUtf8LineTest.ktktor-io/api/ktor-io.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-14T14:11:30.292Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5195
File: ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/JavaHttp2Test.kt:13-15
Timestamp: 2025-11-14T14:11:30.292Z
Learning: In ktor-client-java tests, Java's HttpClient does support h2c (HTTP/2 cleartext) when configured with `protocolVersion = HttpClient.Version.HTTP_2` on Java 11+. The JavaHttp2Test successfully passes with the default useH2c=true setting, connecting to http://localhost:8084.
Applied to files:
ktor-client/ktor-client-okhttp/jvm/test/io/ktor/client/engine/okhttp/OkHttpHttp2Test.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.kt
📚 Learning: 2025-06-16T17:13:12.657Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/api/ktor-utils.api:498-506
Timestamp: 2025-06-16T17:13:12.657Z
Learning: The user tre3p removed ZstdInputStream/ZstdOutputStream from their ZstdEncoder implementation to address blocking I/O concerns in coroutine contexts.
Applied to files:
ktor-io/jvm/test/ByteReadChannelOperationsJvmTest.kt
📚 Learning: 2025-09-05T12:47:49.016Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:35-52
Timestamp: 2025-09-05T12:47:49.016Z
Learning: Jetty's AbstractConnection class implements Closeable interface, so any class extending AbstractConnection (like JettyWebsocketConnection) can be used with Kotlin's use { } extension function for resource management.
Applied to files:
ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ByteReadChannelOperationsTest.ktktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Prefix commit messages with KTOR-{NUM} (YouTrack issue), and write them in present tense, imperative mood.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Prefix commit messages with KTOR-{NUM} (YouTrack issue number)
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-07-01T10:54:53.751Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4970
File: build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts:67-72
Timestamp: 2025-07-01T10:54:53.751Z
Learning: In build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts, the KT-70915 and KT-77609 workarounds that limit parallelism for KotlinNativeLink and CInteropCommonizerTask should remain unconditional (not gated behind CI flag) as these limits are beneficial for both local and CI builds.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-io/api/ktor-io.api
🧬 Code graph analysis (7)
ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.kt (1)
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (1)
readLineStrict(624-630)
ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.kt (1)
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (1)
readLineStrict(624-630)
ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt (1)
ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt (2)
decodeChunked(51-55)decodeChunked(66-100)
ktor-io/common/test/ReadLineTest.kt (2)
ktor-server/ktor-server-test-suites/jvm/src/io/ktor/server/testing/suites/Utils.kt (1)
assertFailsWith(94-104)ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (3)
readLineTo(595-606)readLineStrictTo(651-665)exhausted(32-36)
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.kt (1)
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (1)
readLine(577-582)
ktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.kt (3)
ktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/HttpParserTestUtils.kt (1)
test(12-27)ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt (1)
parseResponse(75-97)ktor-server/ktor-server-test-suites/jvm/src/io/ktor/server/testing/suites/Utils.kt (1)
assertFailsWith(94-104)
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (4)
ktor-io/common/src/io/ktor/utils/io/ByteChannel.kt (1)
awaitContent(69-80)ktor-io/common/src/io/ktor/utils/io/CountedByteReadChannel.kt (1)
awaitContent(43-53)ktor-io/jvm/src/io/ktor/utils/io/jvm/javaio/Reading.kt (1)
awaitContent(66-87)ktor-io/common/src/io/ktor/utils/io/core/ByteReadPacket.kt (1)
discard(69-75)
🔇 Additional comments (51)
ktor-test-dispatcher/common/src/TestCommon.kt (1)
7-16: Imports-only change is fine, but verify it matches the repo’s coroutines-test version.This is “just” import hygiene, but it can surface mismatches if
TestResult/runTestoverloads differ across kotlinx-coroutines-test versions.ktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.kt (1)
2-2: Copyright header year bump looks correct.ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.kt (1)
203-208: VerifyreadLine()behavior when the body has no trailing newline
Here"bodyContent"doesn’t include\n. Please confirmcall.receiveChannel().readLine()returns"bodyContent"(vsnull, exception, or suspending waiting for a line ending) under the new API contract; otherwise this test may become flaky/hang depending on implementation and line-ending mode defaults.ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.kt (1)
332-332: API migration looks correct.The change from
readUTF8Line()toreadLineStrict()aligns with the PR's API redesign objectives. For this test's simple input ("Hello world!\n"), the stricter API should work identically while benefiting from the performance improvements mentioned in the PR.ktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.kt (2)
48-51: SecondreadLineStrict()usage is consistent with test data and avoids nullable handling.
No additional concerns beyond verifying the strict EOF/terminator behavior matches the test’s assumptions.
39-41: Migration toreadLineStrict()is correct and safe. The test writes\n-terminated lines with explicitflush()before closing the write channel, ensuring data is available whenreadLineStrict()is called. SincereadLineStrict()requires a line ending and will throwEOFExceptionif the channel closes before finding one, and the test data guarantees the line ending, there is no risk of null return or exceptions from premature closure.ktor-network/jvm/test/io/ktor/network/sockets/tests/ServerSocketTest.kt (1)
74-82: The test code is correct as-is;readLine()treats EOF as an implicit line terminator.The
readLine()function explicitly handles EOF as an implicit line break (seeByteReadChannelOperations.kt:readLineTowiththrowOnIncompleteLine = false). Sending"123"without\nworks fine withreadLine(). The distinction is thatreadLineStrict()requires an explicit line break and will throwEOFExceptionif the channel closes without one—but this test usesreadLine(), notreadLineStrict().ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt (1)
21-21: Ensure KTOR-9179 is properly tracked before disabling the entire test class.The
@Ignoreannotation disables all tests inDeflaterReadChannelTest. While referencing a JIRA ticket (KTOR-9179) for traceability is good practice, confirm that this ticket exists, is public, and has a defined resolution timeline. If this is the only ignored test suite in the project with race conditions, ensure the team has a plan to address and re-enable it.ktor-client/ktor-client-okhttp/jvm/test/io/ktor/client/engine/okhttp/OkHttpHttp2Test.kt (2)
81-85: Exception-path read:readLineStrict()is a good match, but ensure it throws on EOF/close as expectedThis test assumes the second
out.readLineStrict()(Line 84) throws under duplex writer failure; if the new strict API returnsnull/empty on close instead of throwing, thefail(...)will trigger and the assertion will flip. Worth confirming the newreadLineStrict()contract for closed channel/error propagation.
39-47: The concern about potential double-newline/delimiter mismatch is unfounded. ThereadLineStrictTo(buffer)function appends only the line content (without the line ending delimiter) to the buffer and returns the number of characters appended. The manualbuffer.append('\n')at line 44 is correct and necessary to reconstruct proper line separation in the accumulated output. The test code works as intended with no double-newlines or delimiter mismatches.Likely an incorrect or invalid review comment.
ktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.kt (1)
91-93: No action needed—readLine()correctly handles EOF without a line terminator. The API returns accumulated bytes when the channel closes, making the test assertion valid. ThereadLine()method is specifically designed for cases where "an explicit line break at the end of the channel is not mandatory," unlikereadLineStrict()which throwsEOFExceptionin such scenarios.Likely an incorrect or invalid review comment.
ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.kt (3)
2-3: Copyright header update looks fine.
12-23: Imports are now explicit; keep an eye on coroutines API compatibility.
If this repo pins a specific kotlinx-coroutines version, ensurekotlinx.coroutines.future.awaitis available for that version.
58-58:ByteReadChannel.readLine()returnsString?and can return null on EOF. The test directly uses this result inassertEquals()without null-guarding, which will produce a clear failure message ("ktor" != null) if EOF is unexpectedly reached. For a unit test with known, controlled input, this is acceptable; however, if you prefer explicit null-handling for clarity, assign to a variable first and assert non-null before use.ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt (1)
216-216: Migration looks good. The change fromreadUTF8Line()toreadLine()is the correct API update. The defaultLineEndingparameter recognizes both LF and CRLF, which is appropriate for parsing HTTP responses.ktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.kt (3)
2-2: LGTM: Copyright year updated.Standard maintenance update to include 2025.
16-20: LGTM: Import refinements follow guidelines.The explicit imports for
kotlinx.coroutines.testandkotlin.testare appropriate, whileio.ktor.*packages correctly use star imports above.
154-154: The API migration fromreadUTF8Line()toreadLine()is correct and properly tested.The three test methods correctly validate
applyLimit()behavior across different scenarios:
- channelApplyLimitTooLarge: Verifies
PayloadTooLargeExceptionis thrown when payload exceeds the 5-byte limit- channelApplyLimitNormal: Verifies successful read when payload (10 bytes) equals the limit
- channelApplyLimitEmptyOnRead: Verifies null return when the channel closes after the first availability check (KTOR-7254)
The
readLine()API properly replaces the deprecatedreadUTF8Line(). Both returnString?and propagate exceptions from wrapped channels likeapplyLimit()without suppression. Test naming follows theDescribeWhatIsBeingTestedconvention as required.ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.kt (1)
7-15: Star imports forio.ktor.*look correct per repo conventions.
This aligns with the “use star imports forio.ktor.*packages” guideline.ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.kt (1)
85-87: Assertions assumereadLineStrict()normalizes CRLF (no trailing\r) and returns non-null here.
IfreadLineStrict()can surface a trailing\rfor CRLF, these will intermittently fail (e.g.,"HTTP/1.1 200 OK\r"). If its contract is “CRLF → strip both”, then this is solid.Also applies to: 120-122
ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt (1)
114-114: LGTM! SSE line reading updated to new API.The migration from
readUTF8Line()toreadLineStrict()is appropriate for SSE parsing, which benefits from strict line ending enforcement.Also applies to: 118-118
ktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.kt (1)
225-244: LGTM! Cache deserialization updated to new line-reading API.The migration to
readLineStrict()with null-assertion operators is appropriate here. The structured binary format guarantees data availability through pre-read counts (headersCount,varyKeysCount), making the!!operators safe. The refactoring torepeat()blocks also improves readability.ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt (1)
45-45: LGTM! HTTP parsing migrated to new line-reading API.The migration from
readUTF8LineTo()toreadLineStrictTo()correctly adapts to the new return semantics (bytes read asLongwith-1Lfor EOF). The use of strict line endings aligns with HTTP standards (RFC 9112), and line limit enforcement is preserved.Also applies to: 81-81, 122-122
ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/HttpTimeoutTest.kt (1)
225-225: LGTM! Timeout tests updated to new line-reading API.The migration from
readUTF8Line()toreadLine()is appropriate for these timeout tests. The lenient variant (readLine()) is suitable here since the tests focus on cancellation behavior rather than line ending validation.Also applies to: 246-246, 265-265
ktor-io/jvm/test/ByteReadChannelOperationsJvmTest.kt (1)
6-15: LGTM! Test cleanup and import refinement.The explicit imports improve code clarity, and the removal of the deprecated
readUTF8LineTotest aligns with the API migration strategy.ktor-io/common/src/io/ktor/utils/io/LineEnding.kt (1)
1-26: LGTM! New line ending mode API is well-designed.The
LineEndingenum provides a clean, type-safe API for configuring line ending behavior. The documentation clearly explains when to use each mode, and the recommendation to preferDefaultfor efficiency is helpful.ktor-io/common/src/io/ktor/utils/io/LineEndingMode.kt (1)
18-19: LGTM! Deprecation path established.The deprecation annotation appropriately guides users to the new
LineEndingAPI. The@Suppress("DEPRECATION")is necessary for internal self-references within the deprecated class.ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.kt (1)
138-138: LGTM! Multipart parsing updated to strict line reading.The migration to
readLineStrict()is appropriate for multipart boundary and header parsing, which requires strict line ending handling per the multipart format specification. The null checks correctly handle end-of-stream scenarios.Also applies to: 145-145
ktor-io/common/test/ReadLineTest.kt (1)
16-310: Great coverage of CR/LF boundary + limit edge cases (esp. CRLF straddling buffers).
These scenarios map well to the PR’s correctness goals (limit handling, delimiter parsing, EOF vs strict).ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt (1)
71-74: Switch toreadLineStrict()in assertion looks consistent with the new API.ktor-io/common/test/ReadUtf8LineTest.kt (2)
14-18: Nice separation of responsibilities between legacyreadUTF8LineToand new line APIs.
This makes it clearer what behavior is being preserved vs newly specified.
52-64: Good additions: CRLF-only flush case + “no line ending” regression coverage.
TheLineEndingMode.CRLFtest is especially valuable for preserving legacy behavior under the new implementation.Also applies to: 82-93
ktor-io/api/ktor-io.klib.api (1)
50-59: ABI dump includes the newLineEnding+readLine*APIs — please verify all ABI checks + docs are in place.
Per repo guidelines, ensurecheckLegacyAbiis clean and the new public APIs are documented (params/return/throws) in source.Also applies to: 503-506
ktor-io/api/ktor-io.api (2)
319-325: NewLineEndingenum API looks well-structured.The enum with
DefaultandLeniententries aligns with the PR objectives.Default(LF+CRLF only) enables the performance optimization, whileLenient(CR+LF+CRLF) maintains backward compatibility.
88-95: New line-reading APIs properly tracked in ABI.The new
readLine,readLineStrict,readLineStrictTo, andreadLineTofunctions with their$defaultsynthetic variants are correctly declared. This maintains binary compatibility while introducing the new API surface. Based on learnings, API changes must be intentional and well-documented, and binary compatibility is enforced.ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (4)
420-474: Comprehensive deprecation documentation.The detailed migration guide in the deprecation notice is helpful. It clearly explains when to use
readLineStrictvsreadLineand provides code examples for direct equivalents. This helps users migrate their code correctly.
595-606: Clean delegation to internal implementation.The
readLineTofunction correctly delegates withthrowOnIncompleteLine = false, matching the documented behavior of accepting implicit line breaks at EOF.
651-665: Proper validation and clear semantics.The
require(limit >= 0)validation follows Kotlin conventions. Documentation correctly notes that the limit counts characters after UTF-8 decoding, which is important for users.
783-789: Helper functions correctly returnNothing.The exception helper functions
throwTooLongLineExceptionandthrowEndOfStreamExceptioncorrectly haveNothingreturn type, making the control flow clear to the compiler.ktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.kt (2)
58-58: Appropriate use ofreadLineStrict().Using
readLineStrict()is correct here since the server explicitly writes\n-terminated lines. The strict variant will throw if the connection closes unexpectedly without a line break, which is the desired behavior for these tests.
310-316: Consistent migration in autoFlush test.The test correctly uses
readLineStrict()for reading server responses with explicit line terminators.ktor-io/common/test/ByteReadChannelOperationsTest.kt (1)
291-291: Correct use of non-strictreadLine().Using
readLine()instead ofreadLineStrict()is appropriate here because the remaining content "skipIfFound" has no trailing newline, andreadLine()correctly handles implicit line breaks at EOF.ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt (1)
523-526: Migration toreadLineStrict()is appropriate.The server sends
\n-terminated lines, soreadLineStrict()is correct. The?: breakhandles the case wherereadLineStrict()returnsnull(channel empty), though with proper line terminators this shouldn't occur beforeisClosedForReadbecomes true.ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.kt (1)
38-40: LGTM! API migration is correct.The change from
readUTF8Line()toreadLineStrict()correctly migrates to the new line-reading API. Both functions returnString?and the nullable result is appropriately discarded since the intent is to skip the status line as a side effect.ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt (2)
55-55: LGTM! Test code simplified.The change from
ByteReadChannel(bodyText.toByteArray())toByteReadChannel(bodyText)is a good simplification that leverages the String convenience constructor while maintaining identical functionality.Also applies to: 66-66, 76-76, 88-88, 101-101, 112-112, 123-123, 138-138
106-106: LGTM! API migration is correct.The change from
readUTF8Line()toreadLine()correctly migrates to the new line-reading API as intended by this PR.Also applies to: 117-117, 128-128
ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt (2)
72-72: LGTM! Correct API migration.The change from
readUTF8LineTo()(returns Boolean) toreadLineStrictTo()(returns Long) with a>= 0check correctly adapts to the new API semantics where -1 indicates EOF and non-negative values indicate successful line reads.
86-89: The end-of-chunk line validation semantics are correct.Verification confirms that
readLineStrictTo(chunkSizeBuffer, limit = 0):
- Returns 0 when an empty line (just CRLF or LF) is encountered
- Throws
TooLongLineExceptionif any content characters appear before the line ending- Returns -1L only on EOF
This properly validates that the chunk trailer contains no content, enforcing the HTTP chunked transfer encoding format where each chunk must be followed by an empty line.
ktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.kt (3)
9-11: LGTM! Explicit imports improve clarity.The change from wildcard import to explicit imports for
kotlin.testfunctions follows Kotlin best practices and makes the test dependencies clear.
15-15: LGTM! Consistent use of test helper.The migration to the
testhelper function (fromHttpParserTestUtils.kt) provides cleaner syntax for suspend test functions and is consistent with other tests in this package.Also applies to: 26-26, 39-39, 46-46
17-18: LGTM! Explicit line endings improve test clarity.Adding explicit
\r\nto test data makes the line-ending expectations clear, which is particularly important given this PR's focus on redesigning line-reading APIs with stricter line-ending handling.Also applies to: 28-28, 31-31, 34-34, 41-41, 51-51
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.kt
Show resolved
Hide resolved
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.kt
Outdated
Show resolved
Hide resolved
osipxd
force-pushed
the
osipxd/optimize-read-line
branch
from
3784489 to
6c65b2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.kt (1)
234-237: SSE parsing still lacks explicit line-length limits (memory/DoS risk).The migration to
readLine(lineEnding = LineEnding.Lenient)preserves the unbounded line-length behavior (internally useslimit = Long.MAX_VALUE). For network-facing SSE parsing, this remains a potential DoS/memory exhaustion vector.Consider adding a configurable maximum line length (e.g., 64KB default) and using
readLineStrict(limit = maxLineLength, lineEnding = LineEnding.Lenient)to fail fast withTooLongLineException.ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (1)
742-753: Potential IndexOutOfBoundsException when buffer becomes empty after CR handling.After
transferString(1)at line 747, the buffer may be empty. The loop continues withoutawaitContent(), and at line 744,buffer[count - 1]withcount == 0causesbuffer[-1]access.This was flagged before and marked as addressed, but the current code still appears vulnerable:
- Buffer has only CR (
remaining = 1)discardCrlfOrCr()returns false (default mode, CR not followed by LF)transferString(1)transfers CR, buffer now empty- Loop continues,
count = minOf(limitLeft, 0) = 0buffer[count - 1]→buffer[-1]→ crashApply this diff to add content await after transferring lone CR:
if (readBuffer.buffer[count - 1] == CR) { transferString(count = count - 1) if (readBuffer.discardCrlfOrCr()) return consumed transferString(1) // transfer the CR + if (consumed < limit && !awaitContent()) break } else { // No new line separator transferString(count) if (consumed < limit && !awaitContent()) break }ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.kt (1)
71-72: Make the status-line assertion clearer and avoidBoolean?
statusLine?.contains("200 OK")produces aBoolean?, soassertEquals(true, ...)is a bit awkward and hides the difference between EOF and a non-200 status. It would be clearer to assert non-null explicitly and then assert the condition, for example:- val statusLine = input.readLineStrict() - assertEquals(true, statusLine?.contains("200 OK"), "Expected HTTP 200 OK, got: $statusLine") + val statusLine = input.readLineStrict() + kotlin.test.assertNotNull(statusLine, "Expected status line, got EOF") + kotlin.test.assertTrue(statusLine.contains("200 OK"), "Expected HTTP 200 OK, got: $statusLine")(or use
requireNotNullinstead ofassertNotNullif you prefer a hard precondition failure).This keeps the intent of the test while making failures easier to interpret.
🧹 Nitpick comments (4)
ktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.kt (1)
149-156: readUTF8Line → readLine migration preserves test intent for applyLimit semanticsThe three tests now use
readLine()afterapplyLimit, covering:
- Line overflow path throwing
PayloadTooLargeException(Line 154).- Normal in‑limit read returning the full string (Line 163).
- Empty/closing channel returning
null(Line 176), matching the KTOR‑7254 regression scenario.These usages align with the new line‑reading API while still validating the key behaviors of
applyLimit. If CR/LF edge cases are being exercised elsewhere in the suite, this file is in good shape as-is.Also applies to: 158-165, 169-178
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.kt (1)
21-23: Explicitkotlin.testimports are consistent with guidelinesSwitching from a wildcard to explicit
kotlin.testimports keeps only the needed symbols (Test,assertEquals,assertFailsWith) and leaves theio.ktor.*imports as wildcards as preferred. This is consistent with the guidance to reserve star imports forio.ktor.*packages.As per coding guidelines, this narrowed import set looks good and keeps dependencies clear.
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (1)
555-565: Fragile exception message check in deprecated wrapper.The check
cause.message?.startsWith("Unexpected end of stream after reading")is brittle—ifthrowEndOfStreamExceptionmessage changes, this silently breaks. Consider extracting a constant or using a custom exception type. However, since this is a deprecated wrapper, this may be acceptable for the transition period.ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.kt (1)
29-33: Simplify temporary Unix socket file deletion
File.createTempFilealready creates the file, so theexists()check is redundant here –delete()is safe to call even if the file is already gone. You can simplify this block without changing behavior:- val socketFile = File.createTempFile("ktor-unix-socket-test", ".sock") - if (socketFile.exists()) { - socketFile.delete() - } + val socketFile = File.createTempFile("ktor-unix-socket-test", ".sock") + socketFile.delete()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt(2 hunks)ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.kt(3 hunks)ktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.kt(2 hunks)ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.kt(2 hunks)ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.kt(3 hunks)ktor-client/ktor-client-okhttp/jvm/test/io/ktor/client/engine/okhttp/OkHttpHttp2Test.kt(2 hunks)ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/HttpTimeoutTest.kt(3 hunks)ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.kt(1 hunks)ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt(1 hunks)ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt(2 hunks)ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt(3 hunks)ktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.kt(1 hunks)ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt(6 hunks)ktor-io/api/ktor-io.api(2 hunks)ktor-io/api/ktor-io.klib.api(2 hunks)ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt(4 hunks)ktor-io/common/src/io/ktor/utils/io/LineEnding.kt(1 hunks)ktor-io/common/src/io/ktor/utils/io/LineEndingMode.kt(1 hunks)ktor-io/common/test/ByteReadChannelOperationsTest.kt(2 hunks)ktor-io/common/test/ReadLineTest.kt(1 hunks)ktor-io/common/test/ReadUtf8LineTest.kt(4 hunks)ktor-io/jvm/test/ByteReadChannelOperationsJvmTest.kt(1 hunks)ktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.kt(4 hunks)ktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.kt(2 hunks)ktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.kt(1 hunks)ktor-network/jvm/test/io/ktor/network/sockets/tests/ServerSocketTest.kt(1 hunks)ktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.kt(2 hunks)ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.kt(2 hunks)ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.kt(2 hunks)ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt(2 hunks)ktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.kt(5 hunks)ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt(2 hunks)ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.kt(3 hunks)ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.kt(3 hunks)ktor-test-dispatcher/common/src/TestCommon.kt(1 hunks)ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- ktor-server/ktor-server-cio/common/test/io/ktor/tests/server/cio/CIOEngineTest.kt
- ktor-client/ktor-client-curl/desktop/src/io/ktor/client/engine/curl/CurlClientEngine.kt
- ktor-client/ktor-client-okhttp/jvm/test/io/ktor/client/engine/okhttp/OkHttpHttp2Test.kt
- ktor-network/common/test/io/ktor/network/sockets/tests/TCPSocketTest.kt
- ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/PartialContentTest.kt
- ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/ChunkedTransferEncoding.kt
- ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt
- ktor-io/common/src/io/ktor/utils/io/LineEndingMode.kt
- ktor-client/ktor-client-tests/jvm/src/io/ktor/client/tests/HttpClientTest.kt
- ktor-io/common/test/ByteReadChannelOperationsTest.kt
- ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/HttpTimeoutTest.kt
- ktor-network/jvm/test/io/ktor/network/sockets/tests/ServerSocketTest.kt
- ktor-server/ktor-server-netty/jvm/test/io/ktor/tests/server/netty/NettySpecificTest.kt
- ktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5HttpClientTest.kt
- ktor-http/ktor-http-cio/common/test/io/ktor/tests/http/cio/ResponseParserTest.kt
- ktor-http/ktor-http-cio/jvm/test/io/ktor/tests/http/cio/ChunkedTest.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.ktktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-test-dispatcher/common/src/TestCommon.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.ktktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.ktktor-io/common/test/ReadLineTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/src/io/ktor/utils/io/LineEnding.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-io/common/test/ReadUtf8LineTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
**/test/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Follow test naming pattern:
DescribeWhatIsBeingTested
Files:
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.ktktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.ktktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.ktktor-io/common/test/ReadLineTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-io/common/test/ReadUtf8LineTest.kt
🧠 Learnings (24)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.
Applied to files:
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.ktktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-test-dispatcher/common/src/TestCommon.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.ktktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Use star imports for `io.ktor.*` packages
Applied to files:
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-test-dispatcher/common/src/TestCommon.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-test-dispatcher/common/src/TestCommon.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-io/common/test/ReadUtf8LineTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.ktktor-io/api/ktor-io.api
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Error handling should follow Kotlin conventions and use specific Ktor exceptions.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/test/ReadUtf8LineTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-09-05T12:46:14.074Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:90-100
Timestamp: 2025-09-05T12:46:14.074Z
Learning: The ByteReadChannel.readAvailable(ByteBuffer) method in Ktor IO automatically calls awaitContent() internally when the read buffer is exhausted. When it returns 0, it has already suspended and waited for data to become available, so adding explicit awaitContent() calls is redundant and incorrect.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.ktktor-io/api/ktor-io.klib.apiktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/jvm/test/ByteReadChannelOperationsJvmTest.ktktor-io/common/test/ReadUtf8LineTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.ktktor-io/api/ktor-io.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/api/ktor-io.api
📚 Learning: 2025-11-27T10:07:26.686Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Applied to files:
ktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/test/**/*.kt : Follow test naming pattern: `DescribeWhatIsBeingTested`
Applied to files:
ktor-test-dispatcher/common/src/TestCommon.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Error handling follows Kotlin conventions with specific Ktor exceptions
Applied to files:
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/test/ReadUtf8LineTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-09-05T12:47:49.016Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:35-52
Timestamp: 2025-09-05T12:47:49.016Z
Learning: JettyWebsocketConnection in ktor-server-jetty-jakarta implements Closeable interface (has close() method), so connection.use { } is valid Kotlin syntax and will compile correctly.
Applied to files:
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-io/api/ktor-io.klib.apiktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-09-30T07:52:14.769Z
Learnt from: yschimke
Repo: ktorio/ktor PR: 4013
File: ktor-client/ktor-client-android/jvm/src/io/ktor/client/engine/android/Android14URLConnectionFactory.kt:24-26
Timestamp: 2025-09-30T07:52:14.769Z
Learning: In the Ktor Android HTTP client engine (ktor-client-android), prefer using `URI.create(urlString).toURL()` over the `URL(urlString)` constructor when opening connections with Android's HttpEngine, as it avoids deprecated APIs and the different exception behavior (IllegalArgumentException vs MalformedURLException) is acceptable.
Applied to files:
ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.kt
📚 Learning: 2025-09-05T12:47:49.016Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:35-52
Timestamp: 2025-09-05T12:47:49.016Z
Learning: Jetty's AbstractConnection class implements Closeable interface, so any class extending AbstractConnection (like JettyWebsocketConnection) can be used with Kotlin's use { } extension function for resource management.
Applied to files:
ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-io/common/test/ReadUtf8LineTest.kt
📚 Learning: 2025-11-14T14:11:30.292Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5195
File: ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/JavaHttp2Test.kt:13-15
Timestamp: 2025-11-14T14:11:30.292Z
Learning: In ktor-client-java tests, Java's HttpClient does support h2c (HTTP/2 cleartext) when configured with `protocolVersion = HttpClient.Version.HTTP_2` on Java 11+. The JavaHttp2Test successfully passes with the default useH2c=true setting, connecting to http://localhost:8084.
Applied to files:
ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.kt
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-06-09T07:08:35.085Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Applied to files:
ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-06-16T17:13:12.657Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/api/ktor-utils.api:498-506
Timestamp: 2025-06-16T17:13:12.657Z
Learning: The user tre3p removed ZstdInputStream/ZstdOutputStream from their ZstdEncoder implementation to address blocking I/O concerns in coroutine contexts.
Applied to files:
ktor-io/jvm/test/ByteReadChannelOperationsJvmTest.kt
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Prefix commit messages with KTOR-{NUM} (YouTrack issue), and write them in present tense, imperative mood.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Prefix commit messages with KTOR-{NUM} (YouTrack issue number)
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-07-01T10:54:53.751Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4970
File: build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts:67-72
Timestamp: 2025-07-01T10:54:53.751Z
Learning: In build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts, the KT-70915 and KT-77609 workarounds that limit parallelism for KotlinNativeLink and CInteropCommonizerTask should remain unconditional (not gated behind CI flag) as these limits are beneficial for both local and CI builds.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
🧬 Code graph analysis (4)
ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/CIOUnixSocketServerTest.kt (2)
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/HttpRequestRetry.kt (1)
delay(243-245)ktor-network/common/src/io/ktor/network/sockets/Builders.kt (1)
aSocket(14-14)
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/sse/DefaultClientSSESession.kt (1)
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (1)
readLine(577-582)
ktor-io/common/test/ReadLineTest.kt (3)
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/HttpRequestRetry.kt (1)
delay(243-245)ktor-server/ktor-server-test-suites/jvm/src/io/ktor/server/testing/suites/Utils.kt (1)
assertFailsWith(94-104)ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (3)
readLineTo(595-606)readLineStrictTo(651-665)exhausted(32-36)
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (4)
ktor-io/common/src/io/ktor/utils/io/ByteChannel.kt (1)
awaitContent(69-80)ktor-io/common/src/io/ktor/utils/io/CountedByteReadChannel.kt (1)
awaitContent(43-53)ktor-io/jvm/src/io/ktor/utils/io/jvm/javaio/Reading.kt (1)
awaitContent(66-87)ktor-io/common/src/io/ktor/utils/io/core/ByteReadPacket.kt (1)
discard(69-75)
🔇 Additional comments (27)
ktor-test-dispatcher/common/src/TestCommon.kt (1)
7-16: Imports are consistent and minimalExplicit imports correctly cover all used coroutine/time types, with no unused entries and no API/behavior changes. This aligns with the guideline to only use star imports for
io.ktor.*packages.ktor-server/ktor-server-cio/jvm/test/io/ktor/tests/server/cio/ServerPipelineTest.kt (1)
85-87: LGTM! API migration is correct.The migration from
readUTF8Line()toreadLineStrict()is correct and aligns with the PR's goal to replace deprecated UTF-8 line readers with the new line-reading API. The test assertions remain unchanged and the new API correctly handles CRLF line endings used in these HTTP responses.Also applies to: 120-122
ktor-server/ktor-server-plugins/ktor-server-body-limit/common/test/io/ktor/server/plugins/bodylimit/RequestBodyLimitTest.kt (1)
2-2: Header year bump and explicit test imports look goodUpdating the copyright year and switching to explicit imports for
runTestandkotlin.testassertions while keeping star imports only forio.ktor.*is consistent with the stated style guidelines. No further changes needed here.Also applies to: 16-20
ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/LoggingMockedTests.kt (1)
326-333: Switch toreadLineStrictis appropriate hereGiven that this test deliberately writes a single newline‑terminated line (
"Hello world!\n") and closes the source channel after the first collected element, usingreadLineStrict()instead of the old nullablereadUTF8Line()is consistent with the new API and avoids propagating a nullable line value through the flow. The surroundingwhile (!ch.isClosedForRead) { ch.awaitContent(); ... }pattern remains valid under the stricter semantics, since EOF without a newline would rightly surface as an error rather than quietly producingnull.ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt (1)
523-526: No changes needed—code correctly usesreadLineStrict().The return type is
String?and the function returns null (rather than throws) when the channel is empty at the start of a read attempt.EOFExceptionis only thrown if a line is partially read but never terminates with a line break before the channel closes. In this test, the server writes all 3 lines with proper line endings, so the?: breakpattern correctly handles the normal EOF case where no more data is available.ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/http/ApplicationRequestContentTest.kt (2)
2-2: License header year range update is appropriateExtending the copyright range to 2025 matches the current development period and has no behavioral impact.
207-207: UsingreadLine()here aligns the test with the new line-reading APIReplacing
readUTF8Line()withreadLine()keeps the assertion semantics the same for this case (no line terminator in"bodyContent"), while exercising the new line-reading implementation inByteReadChannel. This helps validate the new API under DoubleReceive caching.Please double-check that the new
readLinebehavior for “EOF without terminator” matches the oldreadUTF8Linecontract (returning the collected line rather thannull), since this test depends on that behavior to pass.ktor-io/jvm/test/ByteReadChannelOperationsJvmTest.kt (2)
6-8: LGTM! Import changes follow Kotlin conventions.The explicit imports for
kotlinx.coroutinesandkotlin.testimprove clarity while correctly maintaining the star import forio.ktor.utils.io.*as required by the coding guidelines.Also applies to: 14-15
17-117: Tests for the new line-reading APIs already exist elsewhere. Comprehensive test coverage forreadLine,readLineStrict,readLineTo, andreadLineStrictTois maintained inktor-io/common/test/ReadLineTest.kt, with the deprecatedreadUTF8Line*APIs tested separately inktor-io/common/test/ReadUtf8LineTest.kt. The removal of thereadUTF8LineTotest fromByteReadChannelOperationsJvmTest.ktis appropriate and does not create a coverage gap.ktor-client/ktor-client-java/jvm/test/io/ktor/client/engine/java/ResponseConsumerTest.kt (3)
2-3: Header year update is fineBumping the copyright range to 2014–2025 and keeping the standard header format looks correct; no further changes needed here.
12-22: Imports are consistent with usage and styleThe added explicit imports for coroutines,
java.net.http, andkotlin.testmatch the usages in this file, whileio.ktor.*packages remain star-imported as recommended. No issues here.As per coding guidelines, keeping star imports only for
io.ktor.*is appropriate.
58-58: Switch toreadLine()aligns with new line-reading APIUsing
(responseBody.body as ByteReadChannel).readLine()insiderunBlockingcorrectly migrates this test from the deprecatedreadUTF8Line()to the new API, and the assertion still verifies that the full body ("ktor") is read from the channel. Behavior and intent of the test remain unchanged.ktor-network/jvm/test/io/ktor/network/sockets/tests/ClientSocketTest.kt (1)
92-92: The migration fromreadUTF8Line()toreadLine()is appropriate. The API documentation confirms thatreadLine()explicitly treats EOF as an implicit line boundary: "Appends UTF-8 characters until a line break is found or the channel is exhausted. Implicit line break is assumed if the channel ends without a line break." This is exactly the scenario the test verifies—data ending at EOF without an explicit line terminator. The test is valid and the behavior is preserved.ktor-network/common/test/io/ktor/network/sockets/tests/UnixSocketTest.kt (1)
40-40: LGTM! Test migration to new line-reading API is correct.The migration from
readUTF8Line()toreadLineStrict()is appropriate for this test. Both test messages include newline terminators, so the strict variant will correctly read the lines.Also applies to: 49-49
ktor-http/ktor-http-cio/common/src/io/ktor/http/cio/HttpParser.kt (1)
45-45: LGTM! HTTP parser correctly migrated to new line-reading API.The migration preserves existing behavior:
- EOF detection now checks for
-1Lreturn value instead of boolean- The 8192-byte line limit is maintained
- Error handling and parsing logic remain intact
Also applies to: 81-81, 122-122
ktor-io/common/src/io/ktor/utils/io/LineEnding.kt (1)
1-26: LGTM! Well-designed public API for line-ending configuration.The new
LineEndingenum is:
- Clearly documented with usage guidance
- Follows Kotlin and Ktor conventions
- Provides sensible defaults (Default mode recommended, Lenient for compatibility)
- Properly integrated with the feedback system
ktor-io/common/test/ReadLineTest.kt (1)
1-328: Excellent test coverage for new line-reading APIs.This comprehensive test suite effectively validates:
- Buffer boundary handling (CR/LF split across buffers)
- LineEnding modes (Default vs Lenient)
- EOF semantics (strict throws EOFException, lenient returns partial)
- Limit enforcement (exact limits, exceeding limits, TooLongLineException)
- Edge cases (empty channels, position-0 endings, mixed line endings)
The data-driven approach at lines 245-310 provides good coverage of various line-ending combinations.
ktor-io/common/test/ReadUtf8LineTest.kt (2)
14-18: LGTM! Proper approach to maintain legacy API compatibility tests.The comment clearly explains that these tests verify the deprecated
readUTF8LineTobehavior is preserved, while the newReadLineTestcovers the new API surface. The@Suppress("DEPRECATION")annotation is appropriate.
22-93: LGTM! Test naming and coverage improvements.The migration to backtick-quoted natural language test names improves readability and follows Ktor test conventions. The new test cases for flush scenarios and lines without endings expand edge-case coverage.
ktor-io/api/ktor-io.klib.api (1)
50-59: LGTM! API surface properly expanded with new line-reading capabilities.The new public API is well-designed:
LineEndingenum provides clear line-ending semantics- Line-reading functions (
readLine,readLineStrict,readLineTo,readLineStrictTo) follow consistent naming- Default parameters ensure backward compatibility
- Binary compatibility is preserved (deprecated APIs at lines 513-515 remain available)
The changes are properly tracked in the ABI signature file as required by Ktor's binary compatibility enforcement.
As per coding guidelines, remember to run:
./gradlew checkLegacyAbito validate ABI compatibility./gradlew updateLegacyAbiif needed after review approvalAlso applies to: 503-506
ktor-io/api/ktor-io.api (1)
88-95: LGTM! New public API surface is well-structured.The new
LineEndingenum andreadLine*family of functions are properly exposed with correct default parameter handling. The API maintains binary compatibility by keeping the deprecatedreadUTF8Line*functions. As per coding guidelines, public API changes are tracked in the/api/directory as required.Also applies to: 319-325
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (6)
420-474: Excellent deprecation documentation.The migration guide is comprehensive, covering both the direct equivalent and the recommended approach. The examples clearly show before/after patterns.
567-606: LGTM! Clean API design.The
readLineandreadLineTofunctions correctly delegate to the internal implementation with appropriate defaults. The removal of thelimitparameter from non-strict variants is intentional and aligns with kotlinx-io conventions.
608-665: LGTM! Strict variants are well-designed.The
require(limit >= 0)validation and clear exception semantics (TooLongLineException, EOFException) are properly documented and implemented.
755-781: LGTM! Post-loop handling is robust.The distinction between EOF (incomplete line) and TooLongLineException (limit exceeded) is correctly implemented. The
check()guard at line 757 provides good defensive programming against implementation bugs.
783-789: LGTM! Helper functions correctly returnNothing.Both
throwTooLongLineExceptionandthrowEndOfStreamExceptionhave the correctNothingreturn type for non-returning functions.
281-281: LGTM! Documentation update.The reference to
Buffer.remainingis accurate for the current API.
ktor-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.kt
Show resolved
Hide resolved
osipxd
force-pushed
the
osipxd/optimize-read-line
branch
from
6c65b2d to
fd1747c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt (1)
21-23: Class-level@Ignorelooks fine; consider narrowing or adding explicit re‑enable noteUsing
@Ignore("KTOR-9179 Fix races in DeflaterReadChannelTest")at class level is appropriate to park a flaky/racy suite and the reason is clear. If possible, consider:
- Restricting the ignore to only the specific flaky test(s), or
- Adding a short comment/TODO near the annotation about re‑enabling once KTOR‑9179 is resolved, so this doesn’t become a permanent skip.
ktor-io/common/test/ReadLineTest.kt (1)
220-224: Test name is misleading - it tests lenient EOF handling, not limit.The test is named
limit - exact limit at end of stream succeeds in lenient mode, but it callsassertLineswithout a limit (defaulting to-1L), which means it usesreadLineTo(unlimited). The test actually verifies that lenient mode accepts content without a trailing line break, not limit behavior.Consider renaming to
lenient mode - line without trailing line break succeedsor similar.@Test - fun `limit - exact limit at end of stream succeeds in lenient mode`() = runTest { + fun `lenient mode - line without trailing line break succeeds`() = runTest { val channel = ByteReadChannel("12345") channel.assertLines("12345", lineEnding = LineEnding.Lenient) }ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (1)
555-565: Fragile exception message check in deprecated wrapper.Line 562 checks
cause.message?.startsWith("Unexpected end of stream after reading")to distinguish between differentEOFExceptionscenarios. This is fragile because:
- The message text could change (see line 789)
- Other
EOFExceptionsources might have different messagesConsider using a dedicated exception subclass or a more robust detection mechanism for the deprecation compatibility layer.
One approach is to define a marker in the exception or use a specific exception type for this case:
- throw EOFException("Unexpected end of stream after reading $consumed characters") + // Consider creating a more specific exception or adding a propertyAlternatively, document this coupling clearly between line 562 and line 789.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ktor-io/api/ktor-io.api(2 hunks)ktor-io/api/ktor-io.klib.api(2 hunks)ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt(4 hunks)ktor-io/common/test/ReadLineTest.kt(1 hunks)ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-io/common/test/ReadLineTest.ktktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
**/test/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Follow test naming pattern:
DescribeWhatIsBeingTested
Files:
ktor-io/common/test/ReadLineTest.ktktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt
🧠 Learnings (14)
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-io/api/ktor-io.apiktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-io/api/ktor-io.klib.apiktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-io/api/ktor-io.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-io/api/ktor-io.apiktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-09-05T12:46:14.074Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:90-100
Timestamp: 2025-09-05T12:46:14.074Z
Learning: The ByteReadChannel.readAvailable(ByteBuffer) method in Ktor IO automatically calls awaitContent() internally when the read buffer is exhausted. When it returns 0, it has already suspended and waited for data to become available, so adding explicit awaitContent() calls is redundant and incorrect.
Applied to files:
ktor-io/api/ktor-io.apiktor-io/api/ktor-io.klib.apiktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.
Applied to files:
ktor-io/common/test/ReadLineTest.ktktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/test/**/*.kt : Follow test naming pattern: `DescribeWhatIsBeingTested`
Applied to files:
ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.kt
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-utils/jvm/test/io/ktor/tests/utils/DeflaterReadChannelTest.ktktor-io/api/ktor-io.klib.api
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Prefix commit messages with KTOR-{NUM} (YouTrack issue), and write them in present tense, imperative mood.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Error handling should follow Kotlin conventions and use specific Ktor exceptions.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Prefix commit messages with KTOR-{NUM} (YouTrack issue number)
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-06-09T07:08:35.085Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Error handling follows Kotlin conventions with specific Ktor exceptions
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
📚 Learning: 2025-07-01T10:54:53.751Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4970
File: build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts:67-72
Timestamp: 2025-07-01T10:54:53.751Z
Learning: In build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts, the KT-70915 and KT-77609 workarounds that limit parallelism for KotlinNativeLink and CInteropCommonizerTask should remain unconditional (not gated behind CI flag) as these limits are beneficial for both local and CI builds.
Applied to files:
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
🧬 Code graph analysis (2)
ktor-io/common/test/ReadLineTest.kt (1)
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (3)
readLineTo(595-606)readLineStrictTo(651-665)exhausted(32-36)
ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (4)
ktor-io/common/src/io/ktor/utils/io/ByteChannel.kt (1)
awaitContent(69-80)ktor-io/common/src/io/ktor/utils/io/CountedByteReadChannel.kt (1)
awaitContent(43-53)ktor-io/jvm/src/io/ktor/utils/io/jvm/javaio/Reading.kt (1)
awaitContent(66-87)ktor-io/common/src/io/ktor/utils/io/core/ByteReadPacket.kt (1)
discard(69-75)
🔇 Additional comments (13)
ktor-io/common/test/ReadLineTest.kt (2)
1-11: LGTM on test file setup.The test file correctly follows Ktor conventions by omitting the package declaration. Imports are appropriate and the test framework usage with
runTestis correct.
312-327: Helper function handles both strict and non-strict modes correctly.The
assertLineshelper elegantly switches betweenreadLineToandreadLineStrictTobased on the limit parameter. The exhausted check at line 326 ensures all expected lines are consumed.ktor-io/api/ktor-io.klib.api (2)
50-59: NewLineEndingenum API is correctly exposed.The enum with
DefaultandLeniententries along with standard enum utilities is properly declared in the public API surface.
503-506: New line-reading APIs are properly declared.The new suspend functions (
readLine,readLineStrict,readLineStrictTo,readLineTo) with theirLineEndingparameters are correctly added to the public API surface. As per coding guidelines, API changes must be intentional and well-documented - the corresponding KDoc in the implementation provides this documentation.ktor-io/api/ktor-io.api (2)
88-95: JVM API additions for line-reading are correctly declared.The new
readLine,readLineStrict,readLineStrictTo, andreadLineTomethods along with their$defaultsynthetic overloads are properly exposed in the JVM public API. The signatures correctly includeLineEndingandContinuationparameters.
319-325:LineEndingenum is correctly exposed in JVM API.The enum extends
java.lang.EnumwithDefaultandLenientfields, and includes standard Kotlin enum utilities (getEntries,valueOf,values).ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt (7)
420-474: Excellent deprecation documentation.The KDoc thoroughly explains the migration path from
readUTF8Lineto the new APIs, including code examples showing both direct equivalents and recommended alternatives. This helps users understand the behavioral differences between Default and Lenient line ending modes.
742-754: Edge case for empty buffer after CR handling is properly addressed.Line 753 correctly handles the scenario where the buffer becomes empty after transferring a lone CR. The check
readBuffer.remaining == 0L && !awaitContent()ensures we either get more data or exit the loop before the next iteration attempts to access buffer indices.
584-606: NewreadLineToAPI is well-designed.The function correctly:
- Defaults to
LineEnding.Defaultfor optimal performance- Returns the number of characters appended (or
-1for empty channel)- Doesn't throw on missing line break at EOF (unlike strict variant)
The KDoc clearly documents the behavior difference from the deprecated
readUTF8LineTo.
632-665: NewreadLineStrictToAPI is well-designed with clear exception semantics.The function:
- Validates that
limit >= 0(line 657)- Throws
TooLongLineExceptionif limit exceeded- Throws
EOFExceptionif channel closes before line break- Documents that limit counts characters, not bytes
The KDoc includes the important note that bytes are consumed even on failure.
758-778: Good defensive check and corner case handling.Line 758-760 includes a
check()assertion to catch implementation bugs. Lines 768-778 correctly handle the corner case where the line ending appears immediately after the limit is reached, ensuring such lines are accepted without throwingTooLongLineException.
281-281: Minor KDoc improvement.The reference to
Buffer.remainingwas updated to align with API changes. The documentation is clear and accurate.
688-701: The edge case concern is invalid. The loop maintains an invariant thatremaining >= 1whenscanForSoleCris called.At line 720,
remainingis guaranteed to be at least 1 because line 753's guard (readBuffer.remaining == 0L && !awaitContent()) breaks the loop if the buffer is exhausted and no more content is available; otherwise,awaitContent()returning true refills the buffer with at least 1 byte. Therefore, line 695's computationminOf(limitLeft, remaining - 1)always produces a non-negativeendIndex(at worst 0 whenremaining == 1), and theindexOf(CR, endIndex = endIndex)call is safe. Similarly, the buffer access at line 744 (buffer[count - 1]) is safe becausecount >= 1(guaranteed bylimitLeft > 0from the loop condition andremaining >= 1).
| transferString(count) | ||
| } | ||
|
|
||
| if (consumed < limit && readBuffer.remaining == 0L && !awaitContent()) break |
There was a problem hiding this comment.
Is it okay to check readBuffer.remaining == 0L or should I call exhuasted() instead?
There was a problem hiding this comment.
Looks like readBuffer.remaining == 0 could return a false positive when using a raw source with SourceByteReadChannel, but this is mitigated by calling awaitContent() afterwards.
| transferString(count) | ||
| } | ||
|
|
||
| if (consumed < limit && readBuffer.remaining == 0L && !awaitContent()) break |
There was a problem hiding this comment.
Looks like readBuffer.remaining == 0 could return a false positive when using a raw source with SourceByteReadChannel, but this is mitigated by calling awaitContent() afterwards.
| * | ||
| * ## Deprecation Notes | ||
| * | ||
| * This function is deprecated in favor of [readLineStrictTo] and [readLineTo]. |
There was a problem hiding this comment.
| * This function is deprecated in favor of [readLineStrictTo] and [readLineTo]. | |
| * This function is deprecated. Use [readLineStrictTo] or [readLineTo] instead. |
| * It recognizes CR, LF and CRLF as a line delimiter. | ||
| * It recognizes CR, LF, and CRLF as a line delimiter. | ||
| * | ||
| * ## Deprecation Notes |
There was a problem hiding this comment.
| * ## Deprecation Notes | |
| * ## Deprecation |
| * This function is deprecated in favor of [readLineStrictTo] and [readLineTo]. | ||
| * | ||
| * Use [readLineStrictTo] when: | ||
| * - Need to limit line length and throw [TooLongLineException] if the limit is reached |
There was a problem hiding this comment.
| * - Need to limit line length and throw [TooLongLineException] if the limit is reached | |
| * - You need to limit the line length and throw [TooLongLineException] when the limit is reached. |
| * | ||
| * Use [readLineStrictTo] when: | ||
| * - Need to limit line length and throw [TooLongLineException] if the limit is reached | ||
| * - Expect an explicit line break at the end of a line. [EOFException] will be thrown |
There was a problem hiding this comment.
| * - Expect an explicit line break at the end of a line. [EOFException] will be thrown | |
| * - You expect an explicit line break at the end of a line. [EOFException] will be thrown |
| * if the channel is closed before the line break | ||
| * | ||
| * Use [readLineTo] when: | ||
| * - No need to limit line length |
There was a problem hiding this comment.
| * - No need to limit line length | |
| * - You do not need to limit line length. |
| * - No need to limit line length | ||
| * - An explicit line break at the end of the channel is not mandatory | ||
| * | ||
| * Other changes: |
There was a problem hiding this comment.
| * Other changes: | |
| * Compared to `readUTF8LineTo`, the new functions differ in the following ways: |
| * - An explicit line break at the end of the channel is not mandatory | ||
| * | ||
| * Other changes: | ||
| * - New functions recognize LF and CRLF as a line delimiter by default. This default comes with better performance. |
There was a problem hiding this comment.
| * - New functions recognize LF and CRLF as a line delimiter by default. This default comes with better performance. | |
| * - They recognize LF and CRLF as line delimiters by default, which provides better performance. |
| * Other changes: | ||
| * - New functions recognize LF and CRLF as a line delimiter by default. This default comes with better performance. | ||
| * To keep current behavior and recognize all line delimiters (CR, LF, and CRLF), specify [LineEnding.Lenient]. | ||
| * - New functions return number of appended characters instead of [Boolean] or `-1` if the channel is empty. |
There was a problem hiding this comment.
| * - New functions return number of appended characters instead of [Boolean] or `-1` if the channel is empty. | |
| * - They return the number of appended characters, or`-1` if the channel is empty, instead of returning [Boolean]. |
| * true | ||
| * } | ||
| * ``` | ||
| * However, we recommend to use [LineEnding.Default] if possible and verify if the case with |
There was a problem hiding this comment.
Suggestion for added clarity and conciseness:
" However, we recommend using the default line ending behavior ([LineEnding.Default]) and verifying unexpected end-of-input explicitly:"
| /** | ||
| * Reads a line of UTF-8 characters to the specified [out] buffer. | ||
| * It recognizes CR, LF and CRLF as a line delimiter. | ||
| * It recognizes CR, LF, and CRLF as a line delimiter. | ||
| * | ||
| * ## Deprecation Notes | ||
| * | ||
| * This function is deprecated in favor of [readLineStrictTo] and [readLineTo]. | ||
| * | ||
| * Use [readLineStrictTo] when: | ||
| * - Need to limit line length and throw [TooLongLineException] if the limit is reached | ||
| * - Expect an explicit line break at the end of a line. [EOFException] will be thrown | ||
| * if the channel is closed before the line break | ||
| * | ||
| * Use [readLineTo] when: | ||
| * - No need to limit line length | ||
| * - An explicit line break at the end of the channel is not mandatory | ||
| * | ||
| * Other changes: | ||
| * - New functions recognize LF and CRLF as a line delimiter by default. This default comes with better performance. | ||
| * To keep current behavior and recognize all line delimiters (CR, LF, and CRLF), specify [LineEnding.Lenient]. | ||
| * - New functions return number of appended characters instead of [Boolean] or `-1` if the channel is empty. | ||
| * - [readLineStrictTo] accepts [Long] instead of [Int] as a limit parameter. | ||
| * | ||
| * The direct equivalent of `readUTF8LineTo` would be: | ||
| * ``` | ||
| * // Before | ||
| * val success = channel.readUTF8LineTo(out, max = 1024) | ||
| * | ||
| * // After | ||
| * val success = try { | ||
| * channel.readLineStrictTo(out, limit = 1024, lineEnding = LineEnding.Lenient) >= 0 | ||
| * } catch (_: EOFException) { | ||
| * true | ||
| * } | ||
| * ``` | ||
| * However, we recommend to use [LineEnding.Default] if possible and verify if the case with | ||
| * the unexpected end of line should actually be ignored. We expect the following code to be correct in most cases: | ||
| * ``` | ||
| * val success = channel.readLineTo(out, limit = 1024) >= 0 | ||
| * ``` |
There was a problem hiding this comment.
I suggested some small improvements for added clarity and actionability, but it generally looks good as it is!
| * @param out the buffer to append line to (line break characters are not appended) | ||
| * @param lineEnding line ending mode. Accepts LF and CRLF by default. | ||
| * | ||
| * @return number of characters appended to [out], or `-1` if can't read from the channel |
There was a problem hiding this comment.
| * @return number of characters appended to [out], or `-1` if can't read from the channel | |
| * @return number of characters appended to [out], or `-1` if the channel is empty |
Subsystem
ktor-ioMotivation
KTOR-9171 Redesign ByteReadChannel.readUTF8Line API
Fixes:
Solution
indexOfreadBuffercaching to reduce property access overheadBenchmark: ktorio/ktor-benchmarks#87
I want to discuss if we should use LF + CRLF line ending mode by default. I think this mode should cover 99% of cases and it comes with performance boost. When we don't look for CR we can use only one
indexOfso implementation becomes faster.Currently
readUTF8Line()andreadUTF8LineTo()useLineEndingMode.Anyby default and the variant ofreadUTF8LineTotakinglineEndingas a parameter is marked as@InternalAPI.