ParkMyCar / compact_str

A memory efficient string type that can store up to 24* bytes on the stack
MIT License
558 stars 41 forks source link

bug: Mutating underlying `&'static str` #313

Closed ParkMyCar closed 8 months ago

ParkMyCar commented 8 months ago

In the yet to be release API CompactString::from_static_str, you can create an instance of CompactString, backed by a &'static str. There are a few APIs on CompactString though that assume we own the underlying data, and thus modify it directly, e.g.

We need to fix these APIs to copy the underlying data before modifying it

NobodyXu commented 8 months ago

retain() should be ok, since it should invoke as_mut_buf which already handle this https://github.com/ParkMyCar/compact_str/blob/df20a0148ecd5df509f82a20b12c45f77a45471e/compact_str/src/repr/mod.rs#L497

NobodyXu commented 8 months ago

drain() also calls as_mut_buf(), so it should be ok.

NobodyXu commented 8 months ago

insert() calls reserve() which handles this, so it should be also ok.

NobodyXu commented 8 months ago

replace_range*() calls as_mut_ptr() or reserve() (in replace_range_grow()), so they should be ok.

ParkMyCar commented 8 months ago

@NobodyXu my bad 🤦‍♂️ you're totally right, thank you for looking into this! I updated the branch that adds fuzz coverage, will run it for a bit, and then release v0.8.0-beta!

NobodyXu commented 8 months ago

No worries! Better to be careful than rush to fix the bug later.