Skip to content

caddyfile: assign unique ID per import invocation to fix named matcher boundary detection#7775

Open
Jualhosting wants to merge 4 commits into
caddyserver:masterfrom
Jualhosting:fix-named-matcher-snippet
Open

caddyfile: assign unique ID per import invocation to fix named matcher boundary detection#7775
Jualhosting wants to merge 4 commits into
caddyserver:masterfrom
Jualhosting:fix-named-matcher-snippet

Conversation

@Jualhosting
Copy link
Copy Markdown
Contributor

@Jualhosting Jualhosting commented May 28, 2026

fix(caddyfile): assign unique ID per import invocation to fix named matcher boundary detection

Problem

Named matchers inside reusable snippets fail when the same snippet is imported more than once within the same site block. The root cause is that consecutive imports of the same snippet from the same import line produce identical .imports\ chain strings, causing \isNextOnNewLine\ to fall back to a raw snippet-relative line-number comparison. Because snippet line numbers always reset to 1, the comparison falsely concludes the next token is on the same line, causing the second (and subsequent) imports' directives to be silently swallowed into the first segment.

Fix

  • Added \importCount int\ to the \parser\ struct in \caddyconfig/caddyfile/parse.go.
  • Incremented \importCount\ on every \doImport\ invocation to assign a globally-unique ID.
  • Embedded that ID into the import-chain string format so each invocation produces a distinct provenance string even when the same snippet is imported from the same file and line.

Tests

Added a regression test in \TestParseAll\ (\caddyconfig/caddyfile/parse_test.go) that imports the same named-matcher snippet twice in the same site block and asserts both directives are parsed into separate segments.

Assistance Disclosure

In accordance with Caddy's contribution guidelines, I am disclosing that AI tools were used to assist in the development of this Pull Request.

Specifically, AI was utilized for:

  • Bug Analysis: Assisting in the core discovery of the named matcher boundary detection issue.
  • Code Implementation: Drafting the fix logic to assign a unique ID per import invocation.
  • Test Writing: Generating the accompanying regression test cases.

The contributor has thoroughly reviewed, fully understood, and manually validated all changes. Full responsibility for the correctness of this code is taken by the contributor.

@mohammed90
Copy link
Copy Markdown
Member

@Jualhosting, please disclose the use of AI, if any.

@Jualhosting
Copy link
Copy Markdown
Contributor Author

Thank you for the reminder, @mohammed90. I have updated the PR description to include the Assistance Disclosure. AI tools were used to assist with bug analysis, code implementation, and test writing. I have reviewed and validated all changes and take full responsibility for the correctness of this PR.

…ed 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
@mohammed90 mohammed90 changed the title fix(caddyfile): assign unique ID per import invocation to fix named m… caddyfile: assign unique ID per import invocation to fix named matcher boundary detection May 28, 2026
@Jualhosting
Copy link
Copy Markdown
Contributor Author

Hi @mohammed90,

Just a quick update: I've added the AI Assistance Disclosure to the PR description as requested. I've also merged the latest from master, so the branch is fully up-to-date and conflict-free.

All CI test suites (Linux, macOS, Windows, and Cross-Build) are now completely green and passing.

The PR should be good to go for your final review and merge whenever you have a chance. Let me know if you need anything else from my end!

Thanks again for your help and review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants