fix: Escape control characters in serialized KDL strings
Hegel property tests with generators::text() found that strings containing control characters produce invalid KDL — the kdl crate v6.5 only escapes \n, \r, \t, \b, \f, \\, and ". All other C0/C1 controls, DEL, and Unicode line separators were written verbatim, breaking roundtrips. Added string_entry() that sets a properly escaped value_repr on KdlEntry when the string contains characters the kdl crate misses. Also strengthened property tests: replaced narrow ASCII regex with full Unicode text(), removed artificial max_size() bounds, and used unique keys for map generation. Assisted-by: Claude Opus 4.6 via pi
diff --git a/.pi/pinch-lock.json b/.pi/pinch-lock.json
new file mode 100644
index 0000000..6f4991b
--- /dev/null
+++ b/.pi/pinch-lock.json
@@ -0,0 +1,6 @@
+{
+ "hegel-skill": {
+ "commit": "07c6cc8cad9ca5ee1b98d0016239e60562afb713",
+ "ref": "main"
+ }
+}
diff --git a/.pi/pinch.json b/.pi/pinch.json
new file mode 100644
index 0000000..8a78c99
--- /dev/null
+++ b/.pi/pinch.json
@@ -0,0 +1,7 @@
+{
+ "hegel-skill": {
+ "repo": "https://github.com/hegeldev/hegel-skill",
+ "ref": "main",
+ "plugins": [{"name": "hegel-skill", "path": "."}]
+ }
+}
diff --git a/src/ser.rs b/src/ser.rs
index bab87e3..ecc85f9 100644
--- a/src/ser.rs
+++ b/src/ser.rs
@@ -12,11 +12,80 @@
//! - **Enum (newtype)** → Child node: `VariantName value`.
//! - **Enum (struct)** → Child node with children: `VariantName { field val }`.
-use kdl::{KdlDocument, KdlEntry, KdlNode, KdlValue};
+use kdl::{KdlDocument, KdlEntry, KdlEntryFormat, KdlNode, KdlValue};
use serde::ser::{self, Serialize};
use crate::error::{Error, Result};
+/// Returns true if `c` is a character that the `kdl` crate (v6.5) writes
+/// verbatim but the KDL v2 spec forbids in unescaped form inside quoted
+/// strings.
+///
+/// The kdl crate escapes: `\\`, `"`, `\n` (0x0A), `\r` (0x0D), `\t`
+/// (0x09), `\b` (0x08), `\f` (0x0C). All other C0 controls, DEL, C1
+/// controls, and Unicode newline characters are written raw.
+fn is_unescaped_by_kdl(c: char) -> bool {
+ let cp = c as u32;
+ matches!(
+ cp,
+ // C0 controls not handled by the kdl crate.
+ 0x00..=0x07 | 0x0B | 0x0E..=0x1F
+ // DEL.
+ | 0x7F
+ // C1 controls (includes NEL at 0x85).
+ | 0x80..=0x9F
+ // Unicode line/paragraph separators treated as newlines by KDL.
+ | 0x2028..=0x2029
+ )
+}
+
+/// Build a `KdlEntry` for a string value, escaping control characters that
+/// the `kdl` crate's `Display` doesn't handle.
+///
+/// The KDL v2 spec forbids unescaped control characters (U+0000–U+0008,
+/// U+000E–U+001F, U+007F, and others) inside quoted strings. The `kdl`
+/// crate (v6.5) only escapes `\n`, `\r`, `\t`, `\b`, `\f`, `\\`, and `"`,
+/// writing all other characters verbatim. This produces invalid KDL for
+/// strings containing other control characters.
+///
+/// We work around this by setting `value_repr` on the entry to a properly
+/// escaped quoted string.
+fn string_entry(s: &str) -> KdlEntry {
+ let needs_escape = s.chars().any(is_unescaped_by_kdl);
+
+ let mut entry = KdlEntry::new(KdlValue::String(s.to_string()));
+
+ if needs_escape {
+ let mut repr = String::with_capacity(s.len() + 2);
+ repr.push('"');
+ for c in s.chars() {
+ match c {
+ '\\' => repr.push_str("\\\\"),
+ '"' => repr.push_str("\\\""),
+ '\n' => repr.push_str("\\n"),
+ '\r' => repr.push_str("\\r"),
+ '\t' => repr.push_str("\\t"),
+ '\u{08}' => repr.push_str("\\b"),
+ '\u{0C}' => repr.push_str("\\f"),
+ c if is_unescaped_by_kdl(c) => {
+ repr.push_str(&format!("\\u{{{:x}}}", c as u32));
+ }
+ c => repr.push(c),
+ }
+ }
+ repr.push('"');
+ entry.set_format(KdlEntryFormat {
+ value_repr: repr,
+ leading: " ".into(),
+ // Prevent autoformat() from clearing the escaped repr.
+ autoformat_keep: true,
+ ..Default::default()
+ });
+ }
+
+ entry
+}
+
/// Serialize a value to a KDL string.
pub fn to_string<T: Serialize>(value: &T) -> Result<String> {
let doc = to_doc(value)?;
@@ -70,7 +139,7 @@ impl Value {
Value::Bool(b) => Some(KdlValue::Bool(*b)),
Value::Integer(i) => Some(KdlValue::Integer(*i)),
Value::Float(f) => Some(KdlValue::Float(*f)),
- Value::String(s) => Some(KdlValue::String(s.clone())),
+ Value::String(_) => unreachable!("strings use string_entry, not to_kdl_value"),
_ => unreachable!("to_kdl_value is only called on primitive values"),
}
}
@@ -123,7 +192,7 @@ fn value_to_nodes(name: &str, value: Value) -> Result<Vec<KdlNode>> {
}
Value::String(s) => {
let mut node = KdlNode::new(name);
- node.push(KdlEntry::new(KdlValue::String(s)));
+ node.push(string_entry(&s));
Ok(vec![node])
}
@@ -145,8 +214,13 @@ fn value_to_nodes(name: &str, value: Value) -> Result<Vec<KdlNode>> {
// Single node, multiple arguments
let mut node = KdlNode::new(name);
for elem in &elements {
- if let Some(kv) = elem.to_kdl_value() {
- node.push(KdlEntry::new(kv));
+ match elem {
+ Value::String(s) => node.push(string_entry(s)),
+ _ => {
+ if let Some(kv) = elem.to_kdl_value() {
+ node.push(KdlEntry::new(kv));
+ }
+ }
}
}
Ok(vec![node])
diff --git a/tests/property.rs b/tests/property.rs
index f94acc1..17cc080 100644
--- a/tests/property.rs
+++ b/tests/property.rs
@@ -1,19 +1,17 @@
use hegel::TestCase;
use hegel::generators::{
- Generator, booleans, floats, from_regex, integers, optional, sampled_from, vecs,
+ Generator, booleans, floats, from_regex, integers, optional, sampled_from, text, vecs,
};
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
// ── Helpers ──────────────────────────────────────────────────────────────
-/// Generator for strings that are safe to roundtrip through KDL.
-/// Excludes characters that would break KDL parsing.
-fn kdl_safe_string() -> impl Generator<String> {
- from_regex("[a-zA-Z0-9 _.,:;!?@#%^&*()+=/<>\\[\\]{}|~'-]{0,64}").fullmatch(true)
-}
-
-/// Generator for f64 values that can roundtrip (no NaN/infinity).
+/// Generator for f64 values that can roundtrip through KDL.
+///
+/// NaN is excluded because `NaN != NaN`, so roundtrip equality assertions
+/// always fail. Infinity is excluded because KDL has no infinity literal —
+/// the serializer would need to encode it as a string, which changes the type.
fn finite_f64() -> impl Generator<f64> {
floats::<f64>().allow_nan(false).allow_infinity(false)
}
@@ -36,7 +34,7 @@ struct FlatStruct {
#[hegel::test]
fn flat_struct_roundtrip(tc: TestCase) {
let val = FlatStruct {
- name: tc.draw(kdl_safe_string()),
+ name: tc.draw(text()),
count: tc.draw(integers()),
enabled: tc.draw(booleans()),
ratio: tc.draw(finite_f64()),
@@ -63,9 +61,9 @@ struct Outer {
#[hegel::test]
fn nested_struct_roundtrip(tc: TestCase) {
let val = Outer {
- label: tc.draw(kdl_safe_string()),
+ label: tc.draw(text()),
inner: Inner {
- host: tc.draw(kdl_safe_string()),
+ host: tc.draw(text()),
port: tc.draw(integers()),
},
};
@@ -85,8 +83,8 @@ struct WithVecStrings {
#[hegel::test]
fn vec_strings_roundtrip(tc: TestCase) {
let val = WithVecStrings {
- label: tc.draw(kdl_safe_string()),
- tags: tc.draw(vecs(kdl_safe_string()).max_size(10)),
+ label: tc.draw(text()),
+ tags: tc.draw(vecs(text())),
};
let serialized = serde_kdl2::to_string(&val).unwrap();
let deserialized: WithVecStrings = serde_kdl2::from_str(&serialized).unwrap();
@@ -102,8 +100,8 @@ struct WithVecInts {
#[hegel::test]
fn vec_ints_roundtrip(tc: TestCase) {
let val = WithVecInts {
- label: tc.draw(kdl_safe_string()),
- numbers: tc.draw(vecs(integers::<i64>()).max_size(10)),
+ label: tc.draw(text()),
+ numbers: tc.draw(vecs(integers::<i64>())),
};
let serialized = serde_kdl2::to_string(&val).unwrap();
let deserialized: WithVecInts = serde_kdl2::from_str(&serialized).unwrap();
@@ -126,16 +124,16 @@ struct WithItems {
#[hegel::test]
fn vec_structs_roundtrip(tc: TestCase) {
- let count = tc.draw(integers::<usize>().min_value(2).max_value(4));
+ let count = tc.draw(integers::<usize>().max_value(10));
let mut items = Vec::new();
for _ in 0..count {
items.push(Item {
- name: tc.draw(kdl_safe_string()),
+ name: tc.draw(text()),
value: tc.draw(integers()),
});
}
let val = WithItems {
- title: tc.draw(kdl_safe_string()),
+ title: tc.draw(text()),
item: items,
};
let serialized = serde_kdl2::to_string(&val).unwrap();
@@ -159,8 +157,8 @@ struct WithOptions {
#[hegel::test]
fn option_fields_roundtrip(tc: TestCase) {
let val = WithOptions {
- required: tc.draw(kdl_safe_string()),
- maybe_str: tc.draw(optional(kdl_safe_string())),
+ required: tc.draw(text()),
+ maybe_str: tc.draw(optional(text())),
maybe_num: tc.draw(optional(integers::<i64>())),
maybe_bool: tc.draw(optional(booleans())),
};
@@ -187,7 +185,7 @@ struct WithEnum {
#[hegel::test]
fn unit_enum_roundtrip(tc: TestCase) {
let val = WithEnum {
- label: tc.draw(kdl_safe_string()),
+ label: tc.draw(text()),
color: tc.draw(sampled_from(vec![Color::Red, Color::Green, Color::Blue])),
};
let serialized = serde_kdl2::to_string(&val).unwrap();
@@ -224,7 +222,7 @@ fn complex_enum_roundtrip(tc: TestCase) {
_ => Shape::Point,
};
let val = WithShape {
- name: tc.draw(kdl_safe_string()),
+ name: tc.draw(text()),
shape,
};
let serialized = serde_kdl2::to_string(&val).unwrap();
@@ -242,13 +240,13 @@ struct WithMap {
#[hegel::test]
fn btreemap_roundtrip(tc: TestCase) {
- let count = tc.draw(integers::<usize>().min_value(0).max_value(4));
+ let keys = tc.draw(vecs(kdl_identifier()).unique(true));
let mut metadata = BTreeMap::new();
- for _ in 0..count {
- metadata.insert(tc.draw(kdl_identifier()), tc.draw(kdl_safe_string()));
+ for key in keys {
+ metadata.insert(key, tc.draw(text()));
}
let val = WithMap {
- title: tc.draw(kdl_safe_string()),
+ title: tc.draw(text()),
metadata,
};
let serialized = serde_kdl2::to_string(&val).unwrap();
@@ -319,7 +317,7 @@ struct WithTuple {
#[hegel::test]
fn tuple_roundtrip(tc: TestCase) {
let val = WithTuple {
- label: tc.draw(kdl_safe_string()),
+ label: tc.draw(text()),
pair: (tc.draw(integers()), tc.draw(integers())),
};
let serialized = serde_kdl2::to_string(&val).unwrap();
@@ -349,9 +347,9 @@ struct Level1 {
#[hegel::test]
fn deeply_nested_roundtrip(tc: TestCase) {
let val = Level1 {
- name: tc.draw(kdl_safe_string()),
+ name: tc.draw(text()),
level2: Level2 {
- tag: tc.draw(kdl_safe_string()),
+ tag: tc.draw(text()),
level3: Level3 {
value: tc.draw(integers()),
},
@@ -379,11 +377,11 @@ struct KitchenSink {
#[hegel::test]
fn kitchen_sink_roundtrip(tc: TestCase) {
let val = KitchenSink {
- s: tc.draw(kdl_safe_string()),
+ s: tc.draw(text()),
i: tc.draw(integers()),
b: tc.draw(booleans()),
f: tc.draw(finite_f64()),
- tags: tc.draw(vecs(kdl_safe_string()).max_size(5)),
+ tags: tc.draw(vecs(text())),
opt: tc.draw(optional(integers::<i32>())),
};
let serialized = serde_kdl2::to_string(&val).unwrap();
@@ -396,7 +394,7 @@ fn kitchen_sink_roundtrip(tc: TestCase) {
#[hegel::test]
fn pretty_print_roundtrip(tc: TestCase) {
let val = FlatStruct {
- name: tc.draw(kdl_safe_string()),
+ name: tc.draw(text()),
count: tc.draw(integers()),
enabled: tc.draw(booleans()),
ratio: tc.draw(finite_f64()),
@@ -424,12 +422,12 @@ struct WithNewtype {
fn newtype_enum_roundtrip(tc: TestCase) {
let variant = tc.draw(booleans());
let wrapped = if variant {
- Wrapper::Text(tc.draw(kdl_safe_string()))
+ Wrapper::Text(tc.draw(text()))
} else {
Wrapper::Number(tc.draw(integers()))
};
let val = WithNewtype {
- label: tc.draw(kdl_safe_string()),
+ label: tc.draw(text()),
wrapped,
};
let serialized = serde_kdl2::to_string(&val).unwrap();