Skip to content

octets: add generic Huffman encoding writer#2502

Open
ghedo wants to merge 2 commits into
masterfrom
octets-writer-huffman
Open

octets: add generic Huffman encoding writer#2502
ghedo wants to merge 2 commits into
masterfrom
octets-writer-huffman

Conversation

@ghedo

@ghedo ghedo commented Jun 4, 2026

Copy link
Copy Markdown
Member

Move HPACK Huffman output behind a small writer trait so callers can reuse the encoder with sinks other than OctetsMut. Keep the existing OctetsMut API by delegating through the trait implementation and add tests for custom sink output and error propagation.

This makes the encoder useful for streaming or segmented output where building one contiguous temporary buffer would be unnecessary (e.g. for HTTP/2 HEADERS+CONTINUATION frames).

@ghedo ghedo requested a review from a team as a code owner June 4, 2026 11:46
Comment thread octets/src/lib.rs Outdated
Move the public API tests out of src/lib.rs into octets/tests so
the crate code no longer carries a large inline test module. Keep the
Huffman-specific coverage in its own feature-gated test file.
Comment thread octets/src/lib.rs
pending -= 64;
// Take only the bits that fit.
bits |= code >> pending;
write(&bits.to_be_bytes())?;

@jrouviere jrouviere Jun 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing that the write function needs to be retryable safely?

For instance it's not ok to write directly to the network with it, since huffman_encode_with might fail after writing some partial data.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, basically the same assumption (the underlying sink has enough capacity) applies, so either an error is returned or there needs to be some buffering within the writer itself.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Might be worth mentioning explicitly in the docs, otherwise I feel like that could be a footgun.
Users might be tempted to write a sink that eagerly sends on the network, but the sink cannot return an error in that case.

Move HPACK Huffman output behind a small writer trait so callers can
reuse the encoder with sinks other than OctetsMut. Keep the existing
OctetsMut API by delegating through the trait implementation and add
tests for custom sink output and error propagation.

This makes the encoder useful for streaming or segmented output where
building one contiguous temporary buffer would be unnecessary (e.g.
for HTTP/2 HEADERS+CONTINUATION frames).
@ghedo ghedo force-pushed the octets-writer-huffman branch from 6b3fac6 to 7de8df7 Compare June 4, 2026 13:40
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.

3 participants