Repository: martinohmann/hcl-rs
Pull request #346 (2024-05-20 10:22:12)

fix: don't leak internal handles during serialization
Fixes https://github.com/martinohmann/hcl-rs/issues/344


Patch:
diff --git a/crates/hcl-rs/src/expr/ser/mod.rs b/crates/hcl-rs/src/expr/ser/mod.rs
index 2cc5a6c..32694f3 100644
--- a/crates/hcl-rs/src/expr/ser/mod.rs
+++ b/crates/hcl-rs/src/expr/ser/mod.rs
@@ -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 {
diff --git a/crates/hcl-rs/src/structure/ser/mod.rs b/crates/hcl-rs/src/structure/ser/mod.rs
index c4adb8e..e796bc7 100644
--- a/crates/hcl-rs/src/structure/ser/mod.rs
+++ b/crates/hcl-rs/src/structure/ser/mod.rs
@@ -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;
 
@@ -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)))
         }
@@ -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(),
         }
     }


Test patch:
diff --git a/crates/hcl-rs/tests/regressions.rs b/crates/hcl-rs/tests/regressions.rs
index 1e15fd0..95ad8ac 100644
--- a/crates/hcl-rs/tests/regressions.rs
+++ b/crates/hcl-rs/tests/regressions.rs
@@ -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());
+}