Skip to content

Commit

Permalink
fix: don't leak internal handles during serialization
Browse files Browse the repository at this point in the history
Fixes #344
  • Loading branch information
martinohmann committed May 20, 2024
1 parent 870df83 commit 5a82710
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 8 deletions.
4 changes: 2 additions & 2 deletions crates/hcl-rs/src/expr/ser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ use crate::ser::{in_internal_serialization, InternalHandles, SerializeInternalHa
use crate::Error;
use serde::ser::{self, Impossible, SerializeMap};

const EXPR_HANDLE_MARKER: &str = "\x00$hcl::ExprHandle";
pub(crate) const EXPR_HANDLE_MARKER: &str = "\x00$hcl::ExprHandle";

thread_local! {
static EXPR_HANDLES: InternalHandles<Expression> = InternalHandles::new(EXPR_HANDLE_MARKER);
pub(crate) static EXPR_HANDLES: InternalHandles<Expression> = InternalHandles::new(EXPR_HANDLE_MARKER);
}

macro_rules! impl_serialize_for_expr {
Expand Down
46 changes: 40 additions & 6 deletions crates/hcl-rs/src/structure/ser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ mod tests;
use super::{Attribute, Block, BlockLabel, Body, Structure};
use crate::expr::ser::{
ExpressionSerializer, SerializeExpressionMap, SerializeExpressionStruct,
SerializeExpressionStructVariant, SerializeExpressionTupleVariant,
SerializeExpressionStructVariant, SerializeExpressionTupleVariant, EXPR_HANDLES,
EXPR_HANDLE_MARKER,
};
use crate::ser::{
blocks::{BLOCK_MARKER, LABELED_BLOCK_MARKER},
in_internal_serialization, IdentifierSerializer, InternalHandles,
SerializeInternalHandleStruct, StringSerializer,
};
use crate::{Error, Expression, Identifier, Result};
use crate::{Error, Expression, Identifier, ObjectKey, Result};
use serde::ser::{self, Serialize, SerializeMap, SerializeStruct};
use std::fmt;

Expand Down Expand Up @@ -302,14 +303,17 @@ impl ser::SerializeMap for SerializeBodyMap {
}

pub(crate) enum SerializeBodyStruct {
InternalHandle(SerializeInternalHandleStruct),
InternalStructureHandle(SerializeInternalHandleStruct),
InternalExprHandle(SerializeInternalHandleStruct),
Map(SerializeBodyMap),
}

impl SerializeBodyStruct {
fn new(name: &'static str, len: usize) -> Self {
if name == STRUCTURE_HANDLE_MARKER {
SerializeBodyStruct::InternalHandle(SerializeInternalHandleStruct::new())
SerializeBodyStruct::InternalStructureHandle(SerializeInternalHandleStruct::new())
} else if name == EXPR_HANDLE_MARKER {
SerializeBodyStruct::InternalExprHandle(SerializeInternalHandleStruct::new())
} else {
SerializeBodyStruct::Map(SerializeBodyMap::new(Some(len)))
}
Expand All @@ -326,15 +330,45 @@ impl ser::SerializeStruct for SerializeBodyStruct {
{
match self {
SerializeBodyStruct::Map(ser) => ser.serialize_entry(key, value),
SerializeBodyStruct::InternalHandle(ser) => ser.serialize_field(key, value),
SerializeBodyStruct::InternalStructureHandle(ser)
| SerializeBodyStruct::InternalExprHandle(ser) => ser.serialize_field(key, value),
}
}

fn end(self) -> Result<Self::Ok> {
match self {
SerializeBodyStruct::InternalHandle(ser) => ser
SerializeBodyStruct::InternalStructureHandle(ser) => ser
.end()
.map(|handle| STRUCTURE_HANDLES.with(|sh| sh.remove(handle)).into()),
SerializeBodyStruct::InternalExprHandle(ser) => {
let handle = ser.end()?;

match EXPR_HANDLES.with(|sh| sh.remove(handle)) {
Expression::Object(object) => {
let attrs = object
.into_iter()
.map(|(key, value)| {
let key = match key {
ObjectKey::Identifier(ident) => ident,
ObjectKey::Expression(Expression::String(s)) => s.into(),
ObjectKey::Expression(expr) => {
return Err(ser::Error::custom(format!(
"encountered invalid HCL attribute key `{expr}`",
)))
}
};

Ok(Attribute::new(key, value))
})
.collect::<Result<Vec<_>>>()?;

Ok(attrs.into())
}
_ => Err(ser::Error::custom(
"non-object HCL expressions are not permitted in this context",
)),
}
}
SerializeBodyStruct::Map(ser) => ser.end(),
}
}
Expand Down
23 changes: 23 additions & 0 deletions crates/hcl-rs/tests/regressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,26 @@ fn issue_248() {
let parsed = hcl::parse(&formatted).unwrap();
assert_eq!(parsed, body);
}

// https://github.com/martinohmann/hcl-rs/issues/344
#[test]
fn issue_344() {
use hcl::Expression;
use vecmap::VecMap;

let value = [("foo", "bar"), ("baz", "qux")];
assert!(hcl::to_string(&VecMap::from_iter(value)).is_ok());
assert!(hcl::to_string(&Expression::from_iter(value)).is_ok());

let value = [(1usize, "bar"), (2usize, "qux")];
assert!(hcl::to_string(&VecMap::from_iter(value)).is_err());
assert!(hcl::to_string(&Expression::from_iter(value)).is_err());

let value = true;
assert!(hcl::to_string(&value).is_err());
assert!(hcl::to_string(&Expression::Bool(value)).is_err());

let value = [1, 2];
assert!(hcl::to_string(&value).is_err());
assert!(hcl::to_string(&Expression::from_iter(value)).is_err());
}

0 comments on commit 5a82710

Please sign in to comment.