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 <