From 4ca58380dce4ed1e0e27820a17809f21bfaff779 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 16 Sep 2026 09:39:27 -0400 Subject: [PATCH] caddyfile: stop Format from mutating its input buffer The three fallback paths returned append(bytes.TrimSpace(input), '\n'). TrimSpace returns a subslice sharing the caller's backing array, so the append wrote the newline into the caller's buffer. os.ReadFile returns a slice with one byte of spare capacity, which is exactly what the append needs, so this reached real callers: $ printf 'foo "unterminated \n\n' > Caddyfile $ caddy fmt --diff foo "unterminated <- reported unchanged; its trailing - whitespace was in fact stripped cmd/commandfuncs.go compares input against output after calling Format, and by then input had been overwritten to match. The same aliasing corrupted FuzzFormatIdempotent, which reported failures whose recorded inputs did not reproduce. Copy into a fresh buffer instead, and assert both non-mutation and non-aliasing. Co-Authored-By: Claude Opus 5 (1M context) --- caddyconfig/caddyfile/formatter.go | 22 ++++++++++++++++------ caddyconfig/caddyfile/formatter_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/caddyconfig/caddyfile/formatter.go b/caddyconfig/caddyfile/formatter.go index eed0622a0..c7149af6d 100644 --- a/caddyconfig/caddyfile/formatter.go +++ b/caddyconfig/caddyfile/formatter.go @@ -43,8 +43,7 @@ func FormatWithOptions(input []byte, opts FormatOptions) []byte { if err != nil { // On a lex error, fall back to the trimmed input with a trailing newline; // Format never panics (Invariant 3). - trimmed := bytes.TrimSpace(input) - return append(trimmed, '\n') + return trimmedWithNewline(input) } // Some token shapes cannot be rendered without changing what they lex back // to, which would break idempotency. When one is present, preserve the @@ -57,8 +56,7 @@ func FormatWithOptions(input []byte, opts FormatOptions) []byte { // fixed point. Detect that here so Format falls back to the trimmed input; // trimming removes the trailing newline again, so the fallback is stable. if hasUnformattableToken(tokens) || trailingNewlineChangesTokens(input) { - trimmed := bytes.TrimSpace(input) - return append(trimmed, '\n') + return trimmedWithNewline(input) } parseTokens, parseErr := Tokenize(input, "") wrapped := false @@ -102,8 +100,7 @@ func FormatWithOptions(input []byte, opts FormatOptions) []byte { parseChanged := !wrapped && parseErr == nil && (outParseErr != nil || !sameTokenTexts(parseTokens, outParseTokens)) if rerr != nil || parseChanged || !sameTokenTexts(tokens, reToks) || !bytes.Equal(out, formatTokens(reToks)) { - trimmed := bytes.TrimSpace(input) - return append(trimmed, '\n') + return trimmedWithNewline(input) } return out } @@ -290,6 +287,19 @@ func wrapUnbracedSite(tokens []Token) []Token { return wrapped } +// trimmedWithNewline returns input trimmed of surrounding whitespace with a +// single trailing newline, in a freshly allocated buffer. The copy matters: +// bytes.TrimSpace returns a subslice that shares input's backing array, so +// appending to it can write into the caller's buffer (os.ReadFile hands back a +// slice with spare capacity), corrupting the input a caller still holds — for +// example the "caddy fmt --diff" comparison of input against output. +func trimmedWithNewline(input []byte) []byte { + trimmed := bytes.TrimSpace(input) + out := make([]byte, 0, len(trimmed)+1) + out = append(out, trimmed...) + return append(out, '\n') +} + // trailingNewlineChangesTokens reports whether appending a newline to input // changes its format-mode token-text sequence. This is true for inputs whose // final token would swallow Format's mandatory trailing newline — an diff --git a/caddyconfig/caddyfile/formatter_test.go b/caddyconfig/caddyfile/formatter_test.go index 13fc85774..52548f347 100644 --- a/caddyconfig/caddyfile/formatter_test.go +++ b/caddyconfig/caddyfile/formatter_test.go @@ -803,6 +803,30 @@ func hasHeredocOpenerShapedToken(in []byte) bool { return false } +// TestFormatDoesNotMutateInput guards the fallback paths, which trim the input +// and append the mandatory newline. bytes.TrimSpace returns a subslice sharing +// the caller's backing array, so appending to it without copying writes into +// the caller's buffer. os.ReadFile returns a slice with spare capacity, so this +// corrupted the input that "caddy fmt --diff" compares its output against. +func TestFormatDoesNotMutateInput(t *testing.T) { + cases := []string{ + "foo \"unterminated \n\n", // unterminated quote + trailing whitespace + "a b\\ \n\n", // dangling escape + trailing whitespace + " site {\n\tfoo\n} \n\n", // ordinary input with surrounding whitespace + } + for _, in := range cases { + // Spare capacity, as os.ReadFile and pooled buffers provide. + buf := append(make([]byte, 0, len(in)+8), in...) + out := Format(buf) + if string(buf) != in { + t.Errorf("Format mutated its input:\n in %q\n after %q", in, string(buf)) + } + if len(out) > 0 && len(buf) > 0 && &out[0] == &buf[0] { + t.Errorf("Format output aliases its input for %q", in) + } + } +} + func FuzzFormatIdempotent(f *testing.F) { for _, s := range []string{ "", " ", "a{\nb\n}", "site {\n\tfoo # c\n}\n", "x <