search

gquintard / varnish-rs

Rust bindings for the Varnish Cache project
BSD 3-Clause "New" or "Revised" License
20 stars 4 forks source link

Undefined behavior (UB) with `IntoVCL<VCL_STRING> for &[u8]` #46

Open nyurik opened 1 week ago

nyurik commented 1 week ago

The impl IntoVCL<VCL_STRING> for &[u8] should not exist in the safe code because it assumes the content of [u8] to be non-null bytes (with the possibility of the last byte being null). Is it even needed?

Instead, I propose the following API:

impl IntoVCL<VCL_STRING> for CString {}
impl IntoVCL<VCL_STRING> for String {}
impl IntoVCL<VCL_STRING> for &CStr {}
impl IntoVCL<VCL_STRING> for &str {}
gquintard commented 1 week ago

we can probably take it out, and think about adding it if it's needed (we need to check the vmods using the crate to be sure).

Coming from C I probably was a bit overzealous in the conversion layer, and also a bit too trusting in the vmod writers ability to be safe, but didn't want to add too much processing if not needed. Your plan works for me now that we have a bit more experience