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) <noreply@anthropic.com>
caddyfile-formatter-unification
Francis Lavoie 2026-09-16 09:39:27 -04:00
parent 93ecef86a4
commit 4ca58380dc
No known key found for this signature in database
2 changed files with 40 additions and 6 deletions

View File

@ -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

View File

@ -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 <<E\nhi\nE\n",