io::read is broken for datagrams/segments

[rrichardson]

io::read uses a while loop which assumes that the data is stream-oriented and doesn't need to be separated on segment boundaries.

It reads data into a buffer as long as there is data and buffer space available. This means there is no delineation between messages which can be catastrophic. read() and recv() will only ever return, at most, a single segment, and a buffer should always be large enough to read 1 segment.

This can be circumvented by building a Buf that puts each read into a separate buffer. Or that will return false from has_remaining if it already has a message, but then you're getting into errors with the Edge Triggered model (#45)

My recommendation would be to make write only ever read 1 message. It could return something which indicates that there might be additional remaining data.

I wrote an example here: https://github.com/12sidedtech/mio/blob/master/src/io.rs#L82

[carllerche]

I'm not sure that I am entirely following, but it seems like the solution would be to implement different versions of read() for TcpSocket vs. UdpSocket where the UdpSocket implementation is different and returns after each call to os::read(). Is that correct? I personally don't have any experience working with UDP sockets, so I can't really say.

[rrichardson]

I actually need this functionality for TCP. It actually makes more sense for most TCP protocols, HTTP for example. Most streaming protocols are chunked up into messages that fit into segments.

When read() is called on a stream socket. It will read 1 segment and only 1 segment. I tend to ensure that messages fit inside a single segment, so if I read twice, I expect that I've received 2 messages. If something else reads for me, and it reads twice into the same buffer, now I'm left with two messages catted together that I can't parse (or am not expecting to parse)

[rrichardson]

So I think the answer here is for read to only call read once. If you're using a stream buffer which expects to fill its buffer every time, it should just call read in its own loop, it should not be part of io, or it would be a separate function in io

[carllerche]

Alright, I'm OK with just removing the loop from the MIO read() API in favor of a single call to os::read()

[carllerche]

Resolved by 0122914