russh-sftp icon indicating copy to clipboard operation
russh-sftp copied to clipboard

Client: Issuing requests asynchronously causes reading mangled packets

Open bspot opened this issue 8 months ago • 0 comments

When I spawn multiple requests asynchronously, the client starts reading mangled packets from the server.

This presents itself as timeouts, Client error. (Bad message: no remaining for u8), Client error. (Bad message: unknown type). This is probably also the cause behind https://github.com/AspectUnk/russh-sftp/issues/21.

I believe that the root cause is the implementation of run in https://github.com/AspectUnk/russh-sftp/blob/bac8dd7ab43be2bf67054763f0ddd1ced4a41f2b/src/client/mod.rs#L62-L82

Inside the loop, the select! waits for either a packet to be read from the server in the first branch, or a new packet beeing sent by the client in the second branch. However, when any of those happen, the other one gets cancelled.

So when I issue a new request, the second branch gets triggered to send the request packet to the server. If, at that time, the first branch has read parts of a packet (e.g. if it has only read the length in https://github.com/AspectUnk/russh-sftp/blob/bac8dd7ab43be2bf67054763f0ddd1ced4a41f2b/src/utils.rs#L15, or only parts of the body in https://github.com/AspectUnk/russh-sftp/blob/bac8dd7ab43be2bf67054763f0ddd1ced4a41f2b/src/utils.rs#L18), that partial packet is dropped. In the next iteration of the loop, a fresh invocation of read_packet starts reading in the middle of the previous packet.

I think that this can be fixed by io::spliting the stream and running the read and write parts separately in run.

bspot avatar May 27 '24 13:05 bspot