Skip to content

Commit 602846e

Browse files
committed
dumpfile: Document escaping behavior relative to C composefs
The dumpfile format is flexible, e.g. it allows escaping some things that technically don't need to be. While doing some fuzzing it was discovered that there were some differences. Let's note those here. Assisted-by: OpenCode (Claude claude-opus-4-6) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 315f922 commit 602846e

2 files changed

Lines changed: 45 additions & 6 deletions

File tree

crates/composefs/src/dumpfile.rs

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,29 +31,47 @@ fn write_empty(writer: &mut impl fmt::Write) -> fmt::Result {
3131
writer.write_str("-")
3232
}
3333

34+
/// Escape a byte slice for a space-delimited dumpfile field.
35+
///
36+
/// This corresponds to `print_escaped_optional` in the C composefs
37+
/// `composefs-info.c`, combining `ESCAPE_STANDARD | ESCAPE_LONE_DASH`.
38+
/// Empty values map to `-` (the "none" sentinel), and a bare `-` is
39+
/// hex-escaped so it is not confused with the sentinel.
40+
///
41+
/// Not appropriate for xattr values — use [`write_escaped_raw`] instead.
3442
fn write_escaped(writer: &mut impl fmt::Write, bytes: &[u8]) -> fmt::Result {
3543
if bytes.is_empty() {
3644
return write_empty(writer);
3745
}
3846

39-
// A bare `-` must be escaped because the parser uses `-` as
40-
// the sentinel for "empty/none" (cf. `optional_str` in the parser).
47+
// Matches C ESCAPE_LONE_DASH: a bare `-` must be escaped because
48+
// the parser uses `-` as the sentinel for "empty/none".
4149
if bytes == b"-" {
4250
return writer.write_str("\\x2d");
4351
}
4452

4553
write_escaped_raw(writer, bytes)
4654
}
4755

48-
/// Write escaped bytes without special handling for empty or bare `-`.
56+
/// Escape a byte slice without the `-` sentinel logic.
4957
///
50-
/// Used for xattr values where the `-` sentinel is not applicable:
51-
/// xattr values are always present when the key=value pair exists,
52-
/// and a value of `-` or empty is perfectly valid.
58+
/// This corresponds to `print_escaped` with `ESCAPE_EQUAL` (but without
59+
/// `ESCAPE_LONE_DASH`) in the C composefs `composefs-info.c`. Used for
60+
/// xattr values where `-` and empty are valid literal values, not
61+
/// sentinels.
62+
///
63+
/// Note: we unconditionally escape `=` in all fields, whereas the C code
64+
/// only uses `ESCAPE_EQUAL` for xattr keys and values. This is harmless
65+
/// since `\x3d` round-trips correctly, but means our output for paths
66+
/// containing `=` is slightly more verbose than the C output.
5367
fn write_escaped_raw(writer: &mut impl fmt::Write, bytes: &[u8]) -> fmt::Result {
5468
for c in bytes {
5569
let c = *c;
5670

71+
// The set of hex-escaped characters matches C `!isgraph(c)` in the
72+
// POSIX locale (i.e. outside 0x21..=0x7E), plus `=` and `\`.
73+
// The C code uses named escapes for `\\`, `\n`, `\r`, `\t` while
74+
// we hex-escape everything uniformly; both forms parse correctly.
5775
if c < b'!' || c == b'=' || c == b'\\' || c > b'~' {
5876
write!(writer, "\\x{c:02x}")?;
5977
} else {
@@ -234,6 +252,12 @@ pub fn write_leaf(
234252
///
235253
/// Creates a special entry that links the given path to an existing target path
236254
/// that was already written to the dumpfile.
255+
///
256+
/// The C composefs writes the real metadata (mode, uid, etc.) from the
257+
/// target node; we write placeholder zeros since both parsers ignore all
258+
/// fields except the target path for hardlink entries. The nlink/uid/gid/rdev
259+
/// fields must be valid integers (not `-`) because the parser reads them
260+
/// unconditionally before checking the `@` hardlink prefix.
237261
pub fn write_hardlink(writer: &mut impl fmt::Write, path: &Path, target: &OsStr) -> fmt::Result {
238262
write_escaped(writer, path.as_os_str().as_bytes())?;
239263
write!(writer, " 0 @120000 0 0 0 0 0.0 ")?;

crates/composefs/src/dumpfile_parse.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,15 @@ enum EscapeMode {
270270
}
271271

272272
/// Escape a byte array according to the composefs dump file text format.
273+
///
274+
/// Note: this function unconditionally maps empty → `-` and escapes a
275+
/// bare `-`. That matches C `ESCAPE_LONE_DASH` and is correct for
276+
/// space-delimited fields (path, payload, content), but the C code does
277+
/// NOT set `ESCAPE_LONE_DASH` for xattr values — there, `-` and empty
278+
/// are valid literals. The `Entry` Display impl currently uses this for
279+
/// xattr values via `EscapeMode::Standard`, which diverges from C.
280+
/// The `write_dumpfile` writer in `dumpfile.rs` avoids this by using
281+
/// a separate `write_escaped_raw` for xattr values.
273282
fn escape<W: std::fmt::Write>(out: &mut W, s: &[u8], mode: EscapeMode) -> std::fmt::Result {
274283
// Empty content must be represented by `-`
275284
if s.is_empty() {
@@ -568,6 +577,12 @@ impl Display for Entry<'_> {
568577
f.write_char(' ')?;
569578
escape(f, xattr.key.as_bytes(), EscapeMode::XattrKey)?;
570579
f.write_char('=')?;
580+
// NOTE: the C code uses ESCAPE_EQUAL (not ESCAPE_LONE_DASH)
581+
// for xattr values, meaning it does not escape bare `-` or
582+
// map empty to `-`. Using `Standard` mode here is slightly
583+
// inconsistent with C but harmless since `\x2d` parses back
584+
// to `-`. The `write_dumpfile` writer uses `write_escaped_raw`
585+
// which matches C more closely.
571586
escape(f, &xattr.value, EscapeMode::Standard)?;
572587
}
573588
std::fmt::Result::Ok(())

0 commit comments

Comments
 (0)