capnproto / capnproto

Cap'n Proto serialization/RPC system - core tools and C++ library
https://capnproto.org
Other
11.39k stars 908 forks source link

BufferedInputStreamWrapper::tryRead fails on EOF #1980

Open alexrobomind opened 3 months ago

alexrobomind commented 3 months ago

Currently, the implementation of BufferedInputStreamWrapper::tryRead will throw on premature EOF due to forwarding to InputStream::read if the buffer is insufficient to fill the request:

https://github.com/capnproto/capnproto/blob/737dba8da94fcde061ebd8b9b3e7efff727b9877/c%2B%2B/src/kj/io.c%2B%2B#L156 https://github.com/capnproto/capnproto/blob/737dba8da94fcde061ebd8b9b3e7efff727b9877/c%2B%2B/src/kj/io.c%2B%2B#L164

Wouldn't it make more sense to forward the read requests to InputStream::tryRead instead?

kentonv commented 2 months ago

Looks like this bug was introduced in 227a4911e9e572b47a9a5b5d2bbeeb9ce5b954c6 when tryRead() was introduced and this implementation of read() was renamed to tryRead() without looking carefully enough at the details.

And that was 11 years ago, and no one ever noticed, I guess...