Merge e63c851392 into 13a4c3f43c

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
+}

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

caddyconfig/caddyfile/parse_test.go

@@ -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()
@@ -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())
+		}
+	}
+}