Skip to content

Commit

Permalink
Store the wrapped template of TemplateBuf in MaybeUninit.
Browse files Browse the repository at this point in the history
This avoids having a real reference to the source data,
which should avoid UB from aliasing the `String`/`Vec`.
  • Loading branch information
de-vri-es committed Dec 13, 2024
1 parent 1d8f793 commit e71d8f8
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 39 deletions.
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ mod features;
#[allow(unused_imports)] // Might not re-export anything if all features are disabled.
pub use features::*;

mod non_aliasing;

/// Substitute variables in a string.
///
/// Variables have the form `$NAME`, `${NAME}` or `${NAME:default}`.
Expand Down
37 changes: 37 additions & 0 deletions src/non_aliasing.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use core::mem::MaybeUninit;

/// Simple wrapper around [`MaybeUninit`] that guarantees the type is initialized.
///
/// This may sound odd, but the compiler can not assume that `MaybeUninit` is initialized,
/// but as far is the compiler is concerned, `MaybeUninit<T>` is not keeping any references around, even if `T` would.
/// So `NonAliasing<T>` is not aliassing anything untill you call `inner()` to get your `&T` back.
///
/// We use this to create a (hopefully) sound self referential struct in `TemplateBuf` and `ByteTemplateBuf`.
pub struct NonAliasing<T> {
inner: MaybeUninit<T>,
}

impl<T> NonAliasing<T> {
pub fn new(inner: T) -> Self {
let inner = MaybeUninit::new(inner);
Self { inner }
}

pub fn inner(&self) -> &T {
// SAFETY: We always initialize `inner` in the constructor.
unsafe {
self.inner.assume_init_ref()
}
}
}

impl<T> Drop for NonAliasing<T> {
fn drop(&mut self) {
// SAFETY: We always initialize `inner` in the constructor,
// the API only exposes `assume_init_ref()`,
// and we're in the destructor, so nobody is going to call assume_init_read() again.
unsafe {
drop(self.inner.assume_init_read())
}
}
}
101 changes: 62 additions & 39 deletions src/template/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use crate::error::{ExpandError, ParseError};
use core::pin::Pin;

use crate::VariableMap;
use crate::error::{ExpandError, ParseError};
use crate::non_aliasing::NonAliasing;

mod raw;

Expand Down Expand Up @@ -94,22 +97,27 @@ impl<'a> Template<'a> {
pub struct TemplateBuf {
// SAFETY: To avoid dangling references, Template must be dropped before
// source, therefore the template field must be precede the source field.
template: Template<'static>,
source: String,
//
// SAFETY: We use NonAliasing<T> to avoid aliassing the source data directly.
// We only re-create the reference to the source when we call template.inner().
template: NonAliasing<Template<'static>>,
source: Pin<String>,
}

impl Clone for TemplateBuf {
fn clone(&self) -> Self {
let source = self.source.clone();
let raw = self.template.inner().raw.clone();

let template = Template {
raw: self.template.raw.clone(),
source: &source,
raw,
source: &*source,

Check warning on line 114 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:114:12 | 114 | source: &*source, | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref = note: `#[warn(clippy::explicit_auto_deref)]` on by default

Check warning on line 114 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:114:12 | 114 | source: &*source, | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref = note: `#[warn(clippy::explicit_auto_deref)]` on by default
};
// SAFETY:
// The str slice given to `template` must remain valid.
// SAFETY: The str slice given to `template` must remain valid.
// Since `String` keeps data on the heap, it remains valid when the `source` is moved.
// We MUST ensure we do not modify, drop or overwrite `source`.
let template = unsafe { template.transmute_lifetime() };
let template = NonAliasing::new(template);
Self {
template,
source,
Expand All @@ -120,7 +128,9 @@ impl Clone for TemplateBuf {
impl std::fmt::Debug for TemplateBuf {
#[inline]
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_tuple("TemplateBuf").field(&self.source).finish()
f.debug_tuple("TemplateBuf")
.field(&self.template.inner().source())
.finish()
}
}

Expand All @@ -139,29 +149,30 @@ impl TemplateBuf {
/// You can escape dollar signs, backslashes, colons and braces with a backslash.
#[inline]
pub fn from_string(source: String) -> Result<Self, ParseError> {
let template = Template::from_str(&source)?;
let source = Pin::new(source);
let template = Template::from_str(&*source)?;

Check warning on line 153 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:153:37 | 153 | let template = Template::from_str(&*source)?; | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref

Check warning on line 153 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:153:37 | 153 | let template = Template::from_str(&*source)?; | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref

// SAFETY:
// The str slice given to `template` must remain valid.
// SAFETY: The str slice given to `template` must remain valid.
// Since `String` keeps data on the heap, it remains valid when the `source` is moved.
// We MUST ensure we do not modify, drop or overwrite `source`.
let template = unsafe { template.transmute_lifetime() };
let template = NonAliasing::new(template);
Ok(Self { source, template })
}

/// Consume the template to get the original source string.
#[inline]
pub fn into_source(self) -> String {
// SAFETY: Drop template before source to avoid dangling reference
// SAFETY: Drop `template` before moving `source` to avoid dangling reference.
drop(self.template);
self.source
Pin::into_inner(self.source)
}

/// Borrow the template.
#[inline]
#[allow(clippy::needless_lifetimes)]
pub fn as_template<'a>(&'a self) -> &'a Template<'a> {
&self.template
self.template.inner()
}

/// Expand the template.
Expand All @@ -175,7 +186,7 @@ impl TemplateBuf {
M: VariableMap<'b> + ?Sized,
M::Value: AsRef<str>,
{
self.template.expand(variables)
self.as_template().expand(variables)
}
}

Expand Down Expand Up @@ -203,18 +214,18 @@ impl From<&Template<'_>> for TemplateBuf {
impl From<Template<'_>> for TemplateBuf {
#[inline]
fn from(other: Template<'_>) -> Self {
let source: String = other.source.into();
let source: Pin<String> = Pin::new(other.source.into());

let template = Template {
source: source.as_str(),
source: &*source,

Check warning on line 220 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:220:12 | 220 | source: &*source, | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref

Check warning on line 220 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:220:12 | 220 | source: &*source, | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref
raw: other.raw,
};

// SAFETY:
// The str slice given to `template` must remain valid.
// SAFETY: The slice given to `template` must remain valid.
// Since `String` keeps data on the heap, it remains valid when the `source` is moved.
// We MUST ensure we do not modify, drop or overwrite `source`.
let template = unsafe { template.transmute_lifetime() };
let template = NonAliasing::new(template);

Self { source, template }
}
Expand Down Expand Up @@ -309,21 +320,30 @@ impl<'a> ByteTemplate<'a> {
pub struct ByteTemplateBuf {
// SAFETY: To avoid dangling references, Template must be dropped before
// source, therefore the template field must be precede the source field.
template: ByteTemplate<'static>,
source: Vec<u8>,
//
// SAFETY: We use NonAliasing<T> to avoid aliassing the source data directly.
// We only re-create the reference to the source when we call template.inner().
template: NonAliasing<ByteTemplate<'static>>,

source: Pin<Vec<u8>>,
}

impl Clone for ByteTemplateBuf {
fn clone(&self) -> Self {
let source = self.source.clone();
let raw = self.template.inner().raw.clone();

let template = ByteTemplate {
raw: self.template.raw.clone(),
source: &source,
raw,
source: &*source,

Check warning on line 338 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:338:12 | 338 | source: &*source, | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref

Check warning on line 338 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:338:12 | 338 | source: &*source, | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref
};
// The slice given to `template` must remain valid.
// Since `Vec` keeps data on the heap, it remains valid when the `source` is moved.

// SAFETY: The slice given to `template` must remain valid.
// Since `Pin<Vec<u8>>` keeps data on the heap, it remains valid when the `source` is moved.
// We MUST ensure we do not modify, drop or overwrite `source`.
let template = unsafe { template.transmute_lifetime() };
let template = NonAliasing::new(template);

Self {
template,
source,
Expand All @@ -335,7 +355,7 @@ impl std::fmt::Debug for ByteTemplateBuf {
#[inline]
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_tuple("ByteTemplateBuf")
.field(&DebugByteString(&self.source))
.field(&DebugByteString(self.as_template().source()))
.finish()
}
}
Expand All @@ -353,29 +373,31 @@ impl ByteTemplateBuf {
/// You can escape dollar signs, backslashes, colons and braces with a backslash.
#[inline]
pub fn from_vec(source: Vec<u8>) -> Result<Self, ParseError> {
let template = ByteTemplate::from_slice(&source)?;
let source = Pin::new(source);
let template = ByteTemplate::from_slice(&*source)?;

Check warning on line 377 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:377:43 | 377 | let template = ByteTemplate::from_slice(&*source)?; | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref

Check warning on line 377 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:377:43 | 377 | let template = ByteTemplate::from_slice(&*source)?; | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref

// SAFETY:
// The slice given to `template` must remain valid.
// SAFETY: The slice given to `template` must remain valid.
// Since `Vec` keeps data on the heap, it remains valid when the `source` is moved.
// We MUST ensure we do not modify, drop or overwrite `source`.
let template = unsafe { std::mem::transmute::<ByteTemplate<'_>, ByteTemplate<'static>>(template) };
let template = NonAliasing::new(template);

Ok(Self { source, template })
}

/// Consume the template to get the original source vector.
#[inline]
pub fn into_source(self) -> Vec<u8> {
// SAFETY: Drop template before source to avoid dangling reference
// SAFETY: Drop `template` before moving `source` to avoid dangling reference.
drop(self.template);
self.source
Pin::into_inner(self.source)
}

/// Borrow the template.
#[inline]
#[allow(clippy::needless_lifetimes)]
pub fn as_template<'a>(&'a self) -> &'a ByteTemplate<'static> {
&self.template
pub fn as_template<'a>(&'a self) -> &'a ByteTemplate<'a> {
self.template.inner()
}

/// Expand the template.
Expand All @@ -389,7 +411,7 @@ impl ByteTemplateBuf {
M: VariableMap<'b> + ?Sized,
M::Value: AsRef<[u8]>,
{
self.template.expand(variables)
self.as_template().expand(variables)
}
}

Expand Down Expand Up @@ -418,17 +440,18 @@ impl From<ByteTemplate<'_>> for ByteTemplateBuf {
#[inline]
fn from(other: ByteTemplate<'_>) -> Self {
let source: Vec<u8> = other.source.into();
let source = Pin::new(source);

let template = ByteTemplate {
source: source.as_slice(),
source: &*source,

Check warning on line 446 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:446:12 | 446 | source: &*source, | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref

Check warning on line 446 in src/template/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

deref which would be done by auto-deref

warning: deref which would be done by auto-deref --> src/template/mod.rs:446:12 | 446 | source: &*source, | ^^^^^^^^ help: try: `&source` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref
raw: other.raw,
};

// SAFETY:
// The slice given to `template` must remain valid.
// Since `Vec` keeps data on the heap, it remains valid when the `source` is moved.
// SAFETY: The slice given to `template` must remain valid.
// Since `Pin<Vec<u8>>` keeps data on the heap, it remains valid when the `source` is moved.
// We MUST ensure we do not modify, drop or overwrite `source`.
let template = unsafe { template.transmute_lifetime() };
let template = NonAliasing::new(template);

Self { source, template }
}
Expand All @@ -454,7 +477,7 @@ mod tests {
use std::collections::BTreeMap;

#[test]
fn test_clone() {
fn test_clone_template_buf() {
let mut map: BTreeMap<String, String> = BTreeMap::new();
map.insert("name".into(), "world".into());
let source = "Hello ${name}!";
Expand Down

0 comments on commit e71d8f8

Please sign in to comment.