Skip to content

tls_codec: use write_all instead of write#2348

Open
keks wants to merge 1 commit into
RustCrypto:masterfrom
cryspen:keks/tls_codec-fix-short-writes
Open

tls_codec: use write_all instead of write#2348
keks wants to merge 1 commit into
RustCrypto:masterfrom
cryspen:keks/tls_codec-fix-short-writes

Conversation

@keks

@keks keks commented Jun 15, 2026

Copy link
Copy Markdown

I ran into an issue when writing into a base64 encoder. Debugging this turned up that the base64 encoder does not always write the entire provided buffer at once. In order to write into this cleanly, I just use write_all instead of write, and track the number of written bytes manually (i.e. not by inspecting the result of write - we now know that the whole slice was written).

I also added a test writer struct that only writes a single byte with each invocation of write_all, and a test that uses that for ensuring this is now handled correctly.

@franziskuskiefer franziskuskiefer self-requested a review June 15, 2026 14:15

@franziskuskiefer franziskuskiefer left a comment

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.

Thanks! lgtm with a small nit.

The InvalidWriteLength is unused now I think. Let's add a comment to that effect (to avoid breakage we can keep it).

Comment thread tls_codec/src/primitives.rs Outdated
@keks keks force-pushed the keks/tls_codec-fix-short-writes branch from 7591211 to ec45cca Compare June 18, 2026 10:38

@franziskuskiefer franziskuskiefer left a comment

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.

Another nit in the test, then we can merge this.

Comment thread tls_codec/tests/encode.rs

let first = [0u8, 1, 2];

first.tls_serialize(&mut short_writer).unwrap();

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.

We should actually test something.
This is what's happening.

Suggested change
first.tls_serialize(&mut short_writer).unwrap();
assert_eq!(short_writer.0.len(), 35);
assert_eq!(&short_writer.0[short_writer.0.len() - 3..], &[0, 1, 2]);

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.

2 participants