mirror of
https://github.com/caddyserver/caddy.git
synced 2026-05-13 09:06:41 +00:00
fix(caddyfile): {block} in snippet (#7558)
* fix(caddyfile): {block} in snippet
Resolve issue #7557
So, here is the situation:
- Pull request #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 #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 #7543.
- Issue #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 #7206 and #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 #7557 reporting, and also passes.
Signed-off-by: prettysunflower <me@prettysunflower.moe>
* caddyfile: add imported snippet block placeholder coverage
---------
Signed-off-by: prettysunflower <me@prettysunflower.moe>
Co-authored-by: Zen Dodd <mail@steadytao.com>
This commit is contained in:
parent
7586e68e27
commit
7dedd1486c
4 changed files with 98 additions and 1 deletions
|
|
@ -550,7 +550,11 @@ func (p *parser) doImport(nesting int) error {
|
|||
}
|
||||
|
||||
if foundBlockDirective {
|
||||
tokensCopy = append(tokensCopy, tokensToAdd...)
|
||||
if maybeSnippet {
|
||||
tokensCopy = append(tokensCopy, token)
|
||||
} else {
|
||||
tokensCopy = append(tokensCopy, tokensToAdd...)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -960,6 +960,77 @@ import `+importFile2+`
|
|||
}
|
||||
}
|
||||
|
||||
func TestImportedSnippetDefinitionRetainsBlockPlaceholder(t *testing.T) {
|
||||
tempDir := t.TempDir()
|
||||
importFile := filepath.Join(tempDir, "snippets.caddy")
|
||||
|
||||
err := os.WriteFile(importFile, []byte(`
|
||||
(site) {
|
||||
http://{args[0]} {
|
||||
respond "before"
|
||||
{block}
|
||||
respond "after"
|
||||
}
|
||||
}
|
||||
`), 0o644)
|
||||
if err != nil {
|
||||
t.Fatalf("writing imported snippet file: %v", err)
|
||||
}
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
input string
|
||||
expectedDirectives []string
|
||||
}{
|
||||
{
|
||||
name: "with nested block",
|
||||
input: `
|
||||
import ` + importFile + `
|
||||
|
||||
import site example.com {
|
||||
redir https://example.net
|
||||
}
|
||||
`,
|
||||
expectedDirectives: []string{"respond", "redir", "respond"},
|
||||
},
|
||||
{
|
||||
name: "without nested block",
|
||||
input: `
|
||||
import ` + importFile + `
|
||||
|
||||
import site example.com
|
||||
`,
|
||||
expectedDirectives: []string{"respond", "respond"},
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
p := testParser(tc.input)
|
||||
blocks, err := p.parseAll()
|
||||
if err != nil {
|
||||
t.Fatalf("parseAll: %v", err)
|
||||
}
|
||||
|
||||
if len(blocks) != 1 {
|
||||
t.Fatalf("expected exactly one server block, got %d", len(blocks))
|
||||
}
|
||||
|
||||
if actual := blocks[0].GetKeysText(); len(actual) != 1 || actual[0] != "http://example.com" {
|
||||
t.Fatalf("expected server block key http://example.com, got %v", actual)
|
||||
}
|
||||
|
||||
if len(blocks[0].Segments) != len(tc.expectedDirectives) {
|
||||
t.Fatalf("expected %d segments, got %d", len(tc.expectedDirectives), len(blocks[0].Segments))
|
||||
}
|
||||
|
||||
for i, directive := range tc.expectedDirectives {
|
||||
if actual := blocks[0].Segments[i].Directive(); actual != directive {
|
||||
t.Fatalf("segment %d: expected directive %q, got %q", i, directive, actual)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func testParser(input string) parser {
|
||||
return parser{Dispenser: NewTestDispenser(input)}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
{
|
||||
admin off
|
||||
auto_https off
|
||||
}
|
||||
|
||||
import testdata/issue_7557_invalid_subdirective_snippet.conf
|
||||
|
||||
:8080 {
|
||||
import test {
|
||||
this_is_nonsense
|
||||
}
|
||||
}
|
||||
|
||||
----------
|
||||
parsing caddyfile tokens for 'reverse_proxy': unrecognized subdirective this_is_nonsense
|
||||
7
caddytest/integration/testdata/issue_7557_invalid_subdirective_snippet.conf
vendored
Normal file
7
caddytest/integration/testdata/issue_7557_invalid_subdirective_snippet.conf
vendored
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
# Used by import_block_snippet_invalid_subdirective.caddyfiletest
|
||||
|
||||
(test) {
|
||||
reverse_proxy {
|
||||
{block}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue