Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions caddyconfig/caddyfile/dispenser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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())
}
Expand Down Expand Up @@ -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
}
19 changes: 16 additions & 3 deletions caddyconfig/caddyfile/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
39 changes: 39 additions & 0 deletions caddyconfig/caddyfile/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,13 @@ 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) {\nrespond \"hello\"\n}\n:80 {\nimport mysnippet\nimport mysnippet\n}\n", false, [][]string{{":80"}}},
} {
p := testParser(test.input)
blocks, err := p.parseAll()
Expand Down Expand Up @@ -1034,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())
}
}
}
Loading