New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KTOR-5252 Fix EOFException in read and readUtf8Line #3285
Conversation
@@ -1631,6 +1631,7 @@ internal open class ByteBufferChannel( | |||
|
|||
override suspend fun read(min: Int, consumer: (ByteBuffer) -> Unit) { | |||
require(min >= 0) { "min should be positive or zero" } | |||
if (min == 0) return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it really correct?
f.e. if we call it with min=0
and we have 5 bytes in channel, I think, that those should be consumed, no?
And in case there will be no bytes in channel I don't know what is expected behaviour - may be return or call consumer with empty buffer, any way, no suspension should happen to await more bytes.
So in my opinion, it should work like non-suspend readAvailable
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will check
} | ||
} catch (_: EOFException) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not an expert of course, but I see this as something easy to break - is it possible to avoid it? or at least add a comment, on why it's ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, it's ignored by the contract of the method
} | ||
} catch (_: EOFException) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
No description provided.