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.

Sign up for GitHub

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

Jump to bottom

implicitly grow BytesMut; add BufMutExt::chain_mut #316

Merged
merged 8 commits into from Nov 20, 2019
Merged

implicitly grow BytesMut; add BufMutExt::chain_mut #316

merged 8 commits into from Nov 20, 2019

Conversation

carllerche
Copy link
Member

@carllerche carllerche commented Nov 20, 2019 •
edited

This brings BytesMut in line with Vec<u8> behavior.

Note this PR also fixes an existing bug in BytesMut::bytes_mut that exposes invalid slices.

In order to fix a test, BufMutExt::chain_mut is provided. Withou this,
it is not possible to chain two &mut [u8].

Closes #170

@carllerche
implicitly grow BytesMut; add BufMutExt::chain_mut
e93b867 
This brings `BytesMut` in line with `Vec<u8>` behavior.

In order to fix a test, `BufMutExt::chain_mut` is provided. Withou this,
it is not possible to chain two `&mut [u8]`.

Closes #170
@carllerche carllerche mentioned this pull request Nov 20, 2019
Merged
hawkw
hawkw approved these changes Nov 20, 2019
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

It looks like the bug in BytesMut::bytes_mut that you fixed would previously return a slice that exceeded the length of the buffer. This seems like it could be a potential security issue — is there anything we need to do about that besides fixing the code?

src/buf/ext/mod.rs Outdated Show resolved Hide resolved
src/bytes_mut.rs Show resolved Hide resolved
src/bytes_mut.rs Outdated Show resolved Hide resolved
@carllerche @hawkw
Update src/buf/ext/mod.rs
083f1d0 
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
@carllerche
Copy link
Member Author

@hawkw the bug was only on master for a bit (never released).

@carllerche carllerche merged commit 8135c1f into master Nov 20, 2019
@mzabaluev mzabaluev mentioned this pull request Nov 25, 2019
Closed
@seanmonstar seanmonstar deleted the bytes-mut-implicit-grow branch November 27, 2019 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkw hawkw hawkw approved these changes

@seanmonstar seanmonstar Awaiting requested review from seanmonstar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BytesMut should implicitly grow the internal storage in its BufMut implementation.
2 participants
@carllerche @hawkw