search

sfackler / rust-socks

Apache License 2.0
83 stars 25 forks source link

Bugfix: Use of BufReader causes occasional data loss #17

Closed s-rah closed 3 years ago

s-rah commented 3 years ago

Thanks for the library!

I noticed that I was occasionally failing to receive the correct data via read_exact() despite packet traces demonstrating that the data was on the wire (for clarity, this didn't surface as an error, but in incorrect data)

Looking through the packet traces, the failures happened when the SOCKS5 Address Info arrived the same TCP packet as the initial data from the remote server.

I believe in this instance that the initial data from the remote was being hijacked by the BufReader being used in read_response() function. Removing the BufReader resolved the sporadic failures.

More generally, as implemented, the BufReader will read an arbitrary amount of data from the socket, and then be dropped out of scope - taking whatever data it has consumed (whether related to the SOCKS proxy or the remote server) with it. This PR removes BufReader from the flow entirely.

sfackler commented 3 years ago

Ah yeah, that logic definitely seems wrong. Thanks!