Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tokio): attempt to decode from internal state even if nothing was read #294

Merged

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Oct 8, 2024

Unlike zlib, miniz_oxide consumes data into internal state
even if there is not enough space in the output buffer.
Next time poll_fill_buf() is called we should try
to decode from internal state even if no new compressed data
was read.

This change is a port of fix #123 (commit 22ed0ac) from futures to tokio.

@link2xt
Copy link
Contributor Author

link2xt commented Oct 8, 2024

miniz_oxide behaves differently from zlib, so even if it consumed all input already, it may be able to output more from the internal state: Frommi/miniz_oxide#158

This is not expected if you just read flate2 implementation, so may be considered a documentation bug: rust-lang/flate2-rs#436
Or maybe minize_oxide will be fixed to behave like zlib.

Same as #123 there is no test, but Delta Chat will serve as a downstream test once this is merged. deltachat/deltachat-core-rust#5990 has an extensive test suite and it passes expect for cargo-deny complaining about git dependency.

@link2xt link2xt force-pushed the link2xt/miniz_oxide-consumes-all-input branch 3 times, most recently from df982ea to cdc1ed9 Compare October 8, 2024 23:23
@link2xt link2xt marked this pull request as ready for review October 9, 2024 00:11
@link2xt link2xt changed the title fix: attempt to decode from internal state even if nothing was read fix(tokio): attempt to decode from internal state even if nothing was read Oct 9, 2024
… read

Unlike zlib, miniz_oxide consumes data into internal state
even if there is not enough space in the output buffer.
Next time poll_fill_buf() is called we should try
to decode from internal state even if no new compressed data
was read.

This change is a port of fix
<Nullus157#123>
(commit Nullus157@22ed0ac)
from `futures` to `tokio`.
@link2xt
Copy link
Contributor Author

link2xt commented Oct 9, 2024

cc @NobodyXu This is ready for review and I am not going to change this further.

@NobodyXu
Copy link
Collaborator

Thanks, that looks good to me

@NobodyXu NobodyXu added this pull request to the merge queue Oct 10, 2024
Merged via the queue into Nullus157:main with commit 23ca593 Oct 10, 2024
16 checks passed
@NobodyXu NobodyXu mentioned this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decoder might block if there is data remaining to be flushed from the codec and the stream blocks
2 participants