rust-brotli icon indicating copy to clipboard operation
rust-brotli copied to clipboard

Flushing sometimes leaves the compressor in an inconsistent state

Open mtth-bfft opened this issue 2 years ago • 0 comments

Thanks for your work porting brotli to Rust :)

I've had an issue when using your crate, specifically when flushing streams: after a flush, all write operations would fail. This would only happen sometimes, depending on the number of bytes compressed so far, so it was pretty hard to track down.

I think I found the culprit: the flush_or_close() method used in the implementation of Write, when called to flush(), improperly stops calling the compressor without checking if there is more data to flush (since the first implementation in c1959aeca4e6deda8021a2a5bcc5ebac753dd599)

The documentation on how to use BROTLI_OPERATION_FLUSH (https://brotli.org/encode.html#aed2 , https://brotli.org/encode.html#a512) mentions the compressor should be called until it has no available input data and BrotliEncoderHasMoreOutput() returns false:

	Under some circumstances (e.g. lack of output stream capacity) this operation would require several calls to BrotliEncoderCompressStream
	[...]
	Warning
	When flushing and finishing, op should not change until operation is complete; input stream should not be swapped, reduced or extended as well.

The current implementation calls BrotliEncoderCompressStream() once and always returns Ok(), which may leave the compressor in BROTLI_OPERATION_FLUSH state and makes future write operations return an Err() (and allow the caller to modify the input stream, leaving room for unexpected compression results).

I'm suggesting a patch in a tiny pull request, which (at least in all the usecases I've been able to reproduce sporadically), solves the issue.

mtth-bfft avatar May 01 '22 12:05 mtth-bfft