From 0d776fc5ac77da41119e700d9ed4a95e5e8badbd Mon Sep 17 00:00:00 2001 From: jalikajalika5 <105954036+jalikajalika5@users.noreply.github.com> Date: Thu, 28 May 2026 11:20:25 +0700 Subject: [PATCH 1/3] fix(caddyfile): assign unique ID per import invocation to fix named matcher boundary detection --- caddyconfig/caddyfile/parse.go | 19 ++++++++++++++++--- caddyconfig/caddyfile/parse_test.go | 14 ++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/caddyconfig/caddyfile/parse.go b/caddyconfig/caddyfile/parse.go index 38d3b9bf388..e44b384b7c1 100644 --- a/caddyconfig/caddyfile/parse.go +++ b/caddyconfig/caddyfile/parse.go @@ -117,6 +117,7 @@ type parser struct { definedSnippets map[string][]Token nesting int importGraph importGraph + importCount int // incremented for each doImport call to uniquely identify each import invocation } func (p *parser) parseAll() ([]ServerBlock, error) { @@ -486,6 +487,15 @@ func (p *parser) doImport(nesting int) error { // copy the tokens so we don't overwrite p.definedSnippets tokensCopy := make([]Token, 0, len(importedTokens)) + // increment the import counter so each invocation of doImport gets a unique + // ID. This is appended to the token import-chain string, which ensures that + // isNextOnNewLine correctly identifies the boundary between two consecutive + // imports of the same snippet (otherwise their chain strings are identical + // and the fallback line-number comparison fails because snippet line numbers + // always start at 1, causing a false "same-line" result). + p.importCount++ + importID := p.importCount + var ( maybeSnippet bool maybeSnippetId bool @@ -496,11 +506,14 @@ func (p *parser) doImport(nesting int) error { // golang for range slice return a copy of value // similarly, append also copy value for i, token := range importedTokens { - // update the token's imports to refer to import directive filename, line number and snippet name if there is one + // update the token's imports to refer to import directive filename, line + // number, a unique per-invocation ID, and snippet name if there is one. + // The importID makes each import call produce a distinct chain entry so + // that isNextOnNewLine never conflates tokens from different imports. if token.snippetName != "" { - token.imports = append(token.imports, fmt.Sprintf("%s:%d (import %s)", p.File(), p.Line(), token.snippetName)) + token.imports = append(token.imports, fmt.Sprintf("%s:%d#%d (import %s)", p.File(), p.Line(), importID, token.snippetName)) } else { - token.imports = append(token.imports, fmt.Sprintf("%s:%d (import)", p.File(), p.Line())) + token.imports = append(token.imports, fmt.Sprintf("%s:%d#%d (import)", p.File(), p.Line(), importID)) } // naive way of determine snippets, as snippets definition can only follow name + block diff --git a/caddyconfig/caddyfile/parse_test.go b/caddyconfig/caddyfile/parse_test.go index 9403f7ac39d..3eec35c2439 100644 --- a/caddyconfig/caddyfile/parse_test.go +++ b/caddyconfig/caddyfile/parse_test.go @@ -601,6 +601,20 @@ func TestParseAll(t *testing.T) { :80 import A `, true, [][]string{}}, + + // regression: same snippet imported multiple times in one site block must + // produce separate segments, not a parse error. Previously isNextOnNewLine + // returned false at the boundary between two imports of the same snippet + // because their import-chain strings were identical and the fallback + // line-number comparison failed (snippet lines always reset to 1). + {`(mysnippet) { + respond "hello" + } + :80 { + import mysnippet + import mysnippet + } + `, false, [][]string{{":80"}}}, } { p := testParser(test.input) blocks, err := p.parseAll() From 72f40622e2887aacfd459a0b84a7b0fc1a48aae1 Mon Sep 17 00:00:00 2001 From: jalikajalika5 <105954036+jalikajalika5@users.noreply.github.com> Date: Thu, 28 May 2026 12:15:38 +0700 Subject: [PATCH 2/3] test(caddyfile): fix snippet import test input format and add dedicated regression test - Replace tab-indented backtick string in TestParseAll with explicit \n-escaped string to eliminate whitespace parsing ambiguity - Add TestDuplicateSnippetImport as a standalone function that explicitly asserts two separate segments are produced (one per import) and each has directive 'respond' This gives clearer failure messages if the fix regresses --- caddyconfig/caddyfile/parse_test.go | 41 +++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/caddyconfig/caddyfile/parse_test.go b/caddyconfig/caddyfile/parse_test.go index 3eec35c2439..078154398a0 100644 --- a/caddyconfig/caddyfile/parse_test.go +++ b/caddyconfig/caddyfile/parse_test.go @@ -607,14 +607,7 @@ func TestParseAll(t *testing.T) { // returned false at the boundary between two imports of the same snippet // because their import-chain strings were identical and the fallback // line-number comparison failed (snippet lines always reset to 1). - {`(mysnippet) { - respond "hello" - } - :80 { - import mysnippet - import mysnippet - } - `, false, [][]string{{":80"}}}, + {"(mysnippet) {\nrespond \"hello\"\n}\n:80 {\nimport mysnippet\nimport mysnippet\n}\n", false, [][]string{{":80"}}}, } { p := testParser(test.input) blocks, err := p.parseAll() @@ -1048,3 +1041,35 @@ func TestImportedSnippetDefinitionRetainsBlockPlaceholder(t *testing.T) { func testParser(input string) parser { return parser{Dispenser: NewTestDispenser(input)} } + +// TestDuplicateSnippetImport verifies that importing the same snippet twice in +// a single site block produces two separate directive segments (one per import) +// rather than merging them or returning a parse error. This is the regression +// test for the isNextOnNewLine boundary-detection bug fixed by the importCount +// mechanism in parse.go. +func TestDuplicateSnippetImport(t *testing.T) { + input := "(mysnippet) {\nrespond \"hello\"\n}\n:80 {\nimport mysnippet\nimport mysnippet\n}\n" + p := testParser(input) + blocks, err := p.parseAll() + if err != nil { + t.Fatalf("unexpected parse error: %v", err) + } + if len(blocks) != 1 { + t.Fatalf("expected 1 server block, got %d", len(blocks)) + } + if got := blocks[0].GetKeysText(); len(got) != 1 || got[0] != ":80" { + t.Fatalf("expected server block key ':80', got %v", got) + } + // Each `import mysnippet` should expand to one directive segment (respond "hello"). + // Before the fix both imports collapsed into a single segment because + // isNextOnNewLine incorrectly returned false at the import boundary. + if len(blocks[0].Segments) != 2 { + t.Fatalf("expected 2 segments (one per import), got %d: %v", + len(blocks[0].Segments), blocks[0].Segments) + } + for i, seg := range blocks[0].Segments { + if seg.Directive() != "respond" { + t.Errorf("segment %d: expected directive 'respond', got %q", i, seg.Directive()) + } + } +} From 2bfd86a1f557670d13afab434c2a1dfad2504305 Mon Sep 17 00:00:00 2001 From: jalikajalika5 <105954036+jalikajalika5@users.noreply.github.com> Date: Thu, 28 May 2026 13:25:12 +0700 Subject: [PATCH 3/3] fix(caddyfile): clean import chain in dispenser error messages --- caddyconfig/caddyfile/dispenser.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/caddyconfig/caddyfile/dispenser.go b/caddyconfig/caddyfile/dispenser.go index 66b9b10878e..38cce6ff52a 100644 --- a/caddyconfig/caddyfile/dispenser.go +++ b/caddyconfig/caddyfile/dispenser.go @@ -415,7 +415,7 @@ func (d *Dispenser) ArgErr() error { // SyntaxErr creates a generic syntax error which explains what was // found and what was expected. func (d *Dispenser) SyntaxErr(expected string) error { - msg := fmt.Sprintf("syntax error: unexpected token '%s', expecting '%s', at %s:%d import chain: ['%s']", d.Val(), expected, d.File(), d.Line(), strings.Join(d.Token().imports, "','")) + msg := fmt.Sprintf("syntax error: unexpected token '%s', expecting '%s', at %s:%d import chain: ['%s']", d.Val(), expected, d.File(), d.Line(), strings.Join(cleanImports(d.Token().imports), "','")) return errors.New(msg) } @@ -438,7 +438,7 @@ func (d *Dispenser) Errf(format string, args ...any) error { // WrapErr takes an existing error and adds the Caddyfile file and line number. func (d *Dispenser) WrapErr(err error) error { if len(d.Token().imports) > 0 { - return fmt.Errorf("%w, at %s:%d import chain ['%s']", err, d.File(), d.Line(), strings.Join(d.Token().imports, "','")) + return fmt.Errorf("%w, at %s:%d import chain ['%s']", err, d.File(), d.Line(), strings.Join(cleanImports(d.Token().imports), "','")) } return fmt.Errorf("%w, at %s:%d", err, d.File(), d.Line()) } @@ -531,3 +531,19 @@ func (d *Dispenser) isNextOnNewLine() bool { } const MatcherNameCtxKey = "matcher_name" + +func cleanImports(imports []string) []string { + cleaned := make([]string, len(imports)) + for i, im := range imports { + if idx := strings.Index(im, "#"); idx != -1 { + if spaceIdx := strings.Index(im[idx:], " "); spaceIdx != -1 { + cleaned[i] = im[:idx] + im[idx+spaceIdx:] + } else { + cleaned[i] = im[:idx] + } + } else { + cleaned[i] = im + } + } + return cleaned +}