Skip to content

caddyfile: Allow block to do nothing if nothing passed to import#7206

Merged
francislavoie merged 1 commit intocaddyserver:masterfrom
BeeJay28:feature/CADDY-6821-Change-parser-to-allow-not-replacing-block-directives
Sep 12, 2025
Merged

caddyfile: Allow block to do nothing if nothing passed to import#7206
francislavoie merged 1 commit intocaddyserver:masterfrom
BeeJay28:feature/CADDY-6821-Change-parser-to-allow-not-replacing-block-directives

Conversation

@BeeJay28
Copy link
Copy Markdown
Contributor

Fixes #6821

Before the parser left the {block}-token (or {blocks.key}-token) as is during parsing, if the corresponding import did not specify any block (pasting it into the import-body as a literal).

After this change, the parser ignores {block} tokens, if there is no body to replace it with.

Also add two adapt-tests to pin the new behavior of {block} and {blocks.key} respectively.

@elee1766
Copy link
Copy Markdown
Contributor

@francislavoie i guess the chance that anyone is using {block} or {blocks.key} in their caddyfile and not trying to use the feature is very very small. i dont think anyone complained. maybe we could put a small note in the release, but im not sure its needed

dont really see any problems, the code is good.

Copy link
Copy Markdown
Member

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, I didn't see this PR for some reason when you made it. LGTM!

@francislavoie francislavoie added this to the v2.10.3 milestone Sep 12, 2025
@francislavoie francislavoie force-pushed the feature/CADDY-6821-Change-parser-to-allow-not-replacing-block-directives branch from 5e8c1f3 to cca54a9 Compare September 12, 2025 19:58
@francislavoie francislavoie changed the title Change parser to allow not replacing block-directives. caddyfile: Allow block to do nothing if nothing passed to import Sep 12, 2025
@francislavoie francislavoie enabled auto-merge (squash) September 12, 2025 20:00
@francislavoie francislavoie merged commit 0ba8786 into caddyserver:master Sep 12, 2025
27 of 28 checks passed
@francislavoie francislavoie modified the milestones: v2.10.3, v2.11.0 Oct 16, 2025
@github-actions github-actions bot mentioned this pull request Dec 3, 2025
4 tasks
prettysunflower added a commit to prettysunflower/caddy that referenced this pull request Mar 10, 2026
Resolve issue caddyserver#7557

So, here is the situation:
- Pull request caddyserver#7206 included some changes to the doImport's function of
  Caddyfile's parser. What it does is that if there is no token within a
  block that follows the import, and the import contains `{block}`, then
  the `{block}` token is discarded.
- This implementation had a few flaws:
  - Issue caddyserver#7518 noticed that in cases that `{block}` was not imported,
    a runtime error was raised due to the assumption that tokens were
    always added to `tokensCopy` on every iteration of `importedTokens`.
    This was fixed by pull request caddyserver#7543.
  - Issue caddyserver#7557 notices that {block} can be ignored when imported from a
    certain file. There, it's again an issue with how the import works.
    When `import snippets` is called, this import instruction doesn't
    contains any nested blocks. And when the argument replacer that is
    the `importedTokens` loop is called and finds `{block}`, it uses the
    block from the file's import (which in this case is nothing),
    `{block}` is erased, and unavailable when the import directive is
    called for the imported snippet.

These changes fix the issues raised in caddyserver#7557 and tries to stabilize
the situation by adding a `Disposable` field to `Token`. It works like
this:
- When a `{block}` is encountered in the argument replacer loop, instead
  of ignoring it and not copying it, the token is now copied (instead of
  being erased), but with the `Disposable` field toggled on.
- When subsequent imports happens, the "disposable" `{block}` token is
  available for the processing of the new import, and can be used to
  call snippets from other files.
- When all tokens are processed within a server block, disposable tokens
  are removed from the keys and segments of the blocks.

This way, `{block}` tokens are there for subsequent inputs, and are
removed if unused.

Tests added in pull requests caddyserver#7206 and caddyserver#7543 passes with this new
implementation, confirming that unused `{block}` are accepted if nothing
is passed to `import`, as well as the other usual tests.
A new test was also added based on issue caddyserver#7557 reporting, and also passes.

... hopefully this commit description is clear?

Signed-off-by: prettysunflower <me@prettysunflower.moe>
prettysunflower added a commit to prettysunflower/caddy that referenced this pull request Mar 10, 2026
Resolve issue caddyserver#7557

So, here is the situation:
- Pull request caddyserver#7206 included some changes to the doImport's function of
  Caddyfile's parser. What it does is that if there is no token within a
  block that follows the import, and the import contains `{block}`, then
  the `{block}` token is discarded.
- After this pull request:
  - Issue caddyserver#7518 noticed that in cases that `{block}` was not imported,
    a runtime error was raised due to the assumption that tokens were
    always added to `tokensCopy` on every iteration of `importedTokens`.
    This was fixed by pull request caddyserver#7543.
  - Issue caddyserver#7557 notices that {block} can be ignored when imported from a
    certain file. There, it's again an issue with how the import works.
    When `import snippets` is called, this import instruction doesn't
    contains any nested blocks. And when the argument replacer that is
    the `importedTokens` loop is called and finds `{block}`, it uses the
    block from the file's import (which in this case is nothing),
    `{block}` is erased, and unavailable when the import directive is
    called for the imported snippet.

The changed in this commit addresses the second issue by checking before
replacing `{block}` if we're currently in a snippet definition, and
appending the `{block}` token to `tokensCopy` if we are.

With this changes, when importing those snippets, the `{block}` token
will be available to be replaced by the nested blocks in `tokensToAdd`
if needed, or erased if there are no nested blocks and `tokensToAdd` is empty.

Tests added in pull requests caddyserver#7206 and caddyserver#7543 passes with this new
implementation, confirming that unused `{block}` are accepted if nothing
is passed to `import`, as well as the other usual tests.
A new test was also added based on issue caddyserver#7557 reporting, and also passes.

Signed-off-by: prettysunflower <me@prettysunflower.moe>
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.

Not possible to omit a {block} in a snippet import if it's declared in the snippet

4 participants