search

Geal / rust-syslog

Send syslog messages from Rust
MIT License
109 stars 55 forks source link

Appending null byte in the write impl has unintended consequences #23

Closed vorner closed 6 years ago

vorner commented 7 years ago

As promised, I went to confirm the changes work and I discovered some unintended behaviour of the null byte fix.

socket.write(&message[..]).and_then(|_| socket.write(&null))

On success, this'll always return Ok(1). This leads to the caller (eg. write_all, or someone else trying really hard to submit the whole message) retrying to send the same message with the first byte removed, the next time with another one removed, etc. This leads to the same message being submitted again and again (in more and more truncated way):

Aug 18 20:21:17 hydra 0:21:17 pakon-aggregator[30302]: Shutting down, app: aggregator/0.1.0
Aug 18 20:21:17 hydra :21:17 pakon-aggregator[30302]: Shutting down, app: aggregator/0.1.0
Aug 18 20:21:17 hydra 21:17 pakon-aggregator[30302]: Shutting down, app: aggregator/0.1.0
Aug 18 20:21:17 hydra 1:17 pakon-aggregator[30302]: Shutting down, app: aggregator/0.1.0
Aug 18 20:21:17 hydra :17 pakon-aggregator[30302]: Shutting down, app: aggregator/0.1.0
Aug 18 20:21:17 hydra 17 pakon-aggregator[30302]: Shutting down, app: aggregator/0.1.0
Aug 18 20:21:17 hydra 7 pakon-aggregator[30302]: Shutting down, app: aggregator/0.1.0

I'm not completely sure on how to solve this properly. A workaround would be to have this:

socket.write(&message[..]).and_then(|len| socket.write(&null).map(|_| len))

But that assumes the null byte did fit when the message fit. I guess the chance of that one not fitting is pretty low and in the worst case, two messages may get concatenated, but a less hacky solution would be nice.

Geal commented 6 years ago

Hello, this should be fixed with 98744a3 and 5867edc