Repository: martinohmann/hcl-rs (rust)

Issue:
deadlock when formatting certain expressions with nested func calls
The following code panics in debug mode and causes a deadlock in release mode:

```
use hcl::{Expression, FuncCall};

let expr = Expression::from(
    FuncCall::builder("foo")
        .arg(Expression::from_iter([("bar", FuncCall::new("baz"))]))
        .build(),
);

let result = hcl::format::to_string(&expr).unwrap();

assert_eq!(result, "foo({ bar = baz() })")
```

This is most likely caused by an invalid formatter state caused by `compact_mode`.

Pull request:
fix(format): prevent subtract with overflow in compact mode
Fixes #87

Patch:
diff --git a/src/format/impls.rs b/src/format/impls.rs
index 41b9c52..c82e736 100644
--- a/src/format/impls.rs
+++ b/src/format/impls.rs
@@ -294,17 +294,17 @@ impl Format for FuncCall {
         self.name.format(fmt)?;
         fmt.write_all(b"(")?;
 
-        fmt.compact_mode(true);
+        fmt.with_compact_mode(|fmt| {
+            for (i, arg) in self.args.iter().enumerate() {
+                if i > 0 {
+                    fmt.write_all(b", ")?;
+                }
 
-        for (i, arg) in self.args.iter().enumerate() {
-            if i > 0 {
-                fmt.write_all(b", ")?;
+                arg.format(fmt)?;
             }
 
-            arg.format(fmt)?;
-        }
-
-        fmt.compact_mode(false);
+            Ok(())
+        })?;
 
         if self.expand_final {
             fmt.write_all(b"...)")?;
diff --git a/src/format/mod.rs b/src/format/mod.rs
index 402d1c0..0251565 100644
--- a/src/format/mod.rs
+++ b/src/format/mod.rs
@@ -36,6 +36,8 @@
 
 mod escape;
 mod impls;
+#[cfg(test)]
+mod tests;
 
 use self::escape::{CharEscape, ESCAPE};
 use crate::Result;
@@ -145,7 +147,7 @@ pub struct Formatter<'a, W> {
     first_element: bool,
     current_indent: usize,
     has_value: bool,
-    compact_mode: bool,
+    compact_mode_level: u64,
 }
 
 /// A builder to create a `Formatter`.
@@ -180,7 +182,7 @@ impl<'a, W> FormatterBuilder<'a, W> {
             first_element: false,
             current_indent: 0,
             has_value: false,
-            compact_mode: false,
+            compact_mode_level: 0,
         }
     }
 }
@@ -305,7 +307,7 @@ where
 
     /// Signals the start of an array to the formatter.
     fn begin_array(&mut self) -> io::Result<()> {
-        if !self.compact_mode {
+        if !self.in_compact_mode() {
             self.current_indent += 1;
         }
         self.has_value = false;
@@ -317,10 +319,10 @@ where
     fn begin_array_value(&mut self) -> io::Result<()> {
         if self.first_element {
             self.first_element = false;
-            if !self.compact_mode {
+            if !self.in_compact_mode() {
                 self.write_all(b"\n")?;
             }
-        } else if self.compact_mode {
+        } else if self.in_compact_mode() {
             self.write_all(b", ")?;
         } else {
             self.write_all(b",\n")?;
@@ -337,7 +339,7 @@ where
 
     /// Signals the end of an array to the formatter.
     fn end_array(&mut self) -> io::Result<()> {
-        if !self.compact_mode {
+        if !self.in_compact_mode() {
             self.current_indent -= 1;
 
             if self.has_value {
@@ -351,7 +353,7 @@ where
 
     /// Signals the start of an object to the formatter.
     fn begin_object(&mut self) -> io::Result<()> {
-        if !self.compact_mode {
+        if !self.in_compact_mode() {
             self.current_indent += 1;
         }
         self.has_value = false;
@@ -360,7 +362,7 @@ where
 
     /// Signals the start of an object key to the formatter.
     fn begin_object_key(&mut self) -> io::Result<()> {
-        if !self.compact_mode {
+        if !self.in_compact_mode() {
             self.write_all(b"\n")?;
             self.write_indent(self.current_indent)?;
         }
@@ -380,7 +382,7 @@ where
 
     /// Signals the end of an object to the formatter.
     fn end_object(&mut self) -> io::Result<()> {
-        if !self.compact_mode {
+        if !self.in_compact_mode() {
             self.current_indent -= 1;
 
             if self.has_value {
@@ -479,10 +481,22 @@ where
         Ok(())
     }
 
-    /// Enables compact mode for the formatter. This is mostly used while serializing array and
-    /// object function arguments.
-    fn compact_mode(&mut self, yes: bool) {
-        self.compact_mode = yes;
+    /// Enables compact mode, runs the closure and disables compact mode again unless it's enabled
+    /// via another call to `with_compact_mode`.
+    ///
+    /// This is mostly used for serializing array and object function arguments.
+    fn with_compact_mode<F>(&mut self, f: F) -> Result<()>
+    where
+        F: FnOnce(&mut Self) -> Result<()>,
+    {
+        self.compact_mode_level += 1;
+        let result = f(self);
+        self.compact_mode_level -= 1;
+        result
+    }
+
+    fn in_compact_mode(&self) -> bool {
+        self.compact_mode_level > 0
     }
 }
 


Test patch:
diff --git a/src/format/tests.rs b/src/format/tests.rs
new file mode 100644
index 0000000..9b2eee3
--- /dev/null
+++ b/src/format/tests.rs
@@ -0,0 +1,15 @@
+use super::*;
+use crate::{Expression, FuncCall};
+
+#[test]
+fn issue_87() {
+    let expr = Expression::from(
+        FuncCall::builder("foo")
+            .arg(Expression::from_iter([("bar", FuncCall::new("baz"))]))
+            .build(),
+    );
+
+    let result = to_string(&expr).unwrap();
+
+    assert_eq!(result, "foo({\"bar\" = baz()})")
+}