From 425f61142f2194de21caff311ea58fa990f40c48 Mon Sep 17 00:00:00 2001 From: Craig Peterson Date: Fri, 13 Oct 2017 11:04:44 -0400 Subject: [PATCH 1/6] initial implementation of caddyfile macros --- caddyfile/parse.go | 175 +++++++++++++++++++++++++++------------- caddyfile/parse_test.go | 41 ++++++++++ 2 files changed, 162 insertions(+), 54 deletions(-) diff --git a/caddyfile/parse.go b/caddyfile/parse.go index d80d575a..1bf84ecc 100644 --- a/caddyfile/parse.go +++ b/caddyfile/parse.go @@ -54,6 +54,7 @@ type parser struct { block ServerBlock // current server block being parsed validDirectives []string // a directive must be valid or it's an error eof bool // if we encounter a valid EOF in a hard place + definedMacros map[string][]Token } func (p *parser) parseAll() ([]ServerBlock, error) { @@ -95,6 +96,25 @@ func (p *parser) begin() error { return nil } + if ok, name := p.isMacro(); ok { + if p.definedMacros == nil { + p.definedMacros = map[string][]Token{} + } + if p.definedMacros[name] != nil { + p.Errf("redeclaration of previously declared macro %s", name) + } + + // consume all tokens til matched close brace + tokens, err := p.macroTokens() + if err != nil { + return err + } + p.definedMacros[name] = tokens + // empty block keys so we don't save this block as a real server. + p.block.Keys = nil + return nil + } + return p.blockContents() } @@ -221,70 +241,75 @@ func (p *parser) doImport() error { if p.NextArg() { return p.Err("Import takes only one argument (glob pattern or file)") } - - // make path relative to Caddyfile rather than current working directory (issue #867) - // and then use glob to get list of matching filenames - absFile, err := filepath.Abs(p.Dispenser.filename) - if err != nil { - return p.Errf("Failed to get absolute path of file: %s: %v", p.Dispenser.filename, err) - } - - var matches []string - var globPattern string - if !filepath.IsAbs(importPattern) { - globPattern = filepath.Join(filepath.Dir(absFile), importPattern) - } else { - globPattern = importPattern - } - matches, err = filepath.Glob(globPattern) - - if err != nil { - return p.Errf("Failed to use import pattern %s: %v", importPattern, err) - } - if len(matches) == 0 { - if strings.Contains(globPattern, "*") { - log.Printf("[WARNING] No files matching import pattern: %s", importPattern) - } else { - return p.Errf("File to import not found: %s", importPattern) - } - } - // splice out the import directive and its argument (2 tokens total) tokensBefore := p.tokens[:p.cursor-1] tokensAfter := p.tokens[p.cursor+1:] - - // collect all the imported tokens var importedTokens []Token - for _, importFile := range matches { - newTokens, err := p.doSingleImport(importFile) + + // first check macros. That is a simple, non-recursive replacement + if p.definedMacros[importPattern] != nil { + importedTokens = p.definedMacros[importPattern] + } else { + // make path relative to Caddyfile rather than current working directory (issue #867) + // and then use glob to get list of matching filenames + absFile, err := filepath.Abs(p.Dispenser.filename) if err != nil { - return err + return p.Errf("Failed to get absolute path of file: %s: %v", p.Dispenser.filename, err) } - var importLine int - for i, token := range newTokens { - if token.Text == "import" { - importLine = token.Line - continue - } - if token.Line == importLine { - var abs string - if filepath.IsAbs(token.Text) { - abs = token.Text - } else if !filepath.IsAbs(importFile) { - abs = filepath.Join(filepath.Dir(absFile), token.Text) - } else { - abs = filepath.Join(filepath.Dir(importFile), token.Text) - } - newTokens[i] = Token{ - Text: abs, - Line: token.Line, - File: token.File, - } + var matches []string + var globPattern string + if !filepath.IsAbs(importPattern) { + globPattern = filepath.Join(filepath.Dir(absFile), importPattern) + } else { + globPattern = importPattern + } + matches, err = filepath.Glob(globPattern) + + if err != nil { + return p.Errf("Failed to use import pattern %s: %v", importPattern, err) + } + if len(matches) == 0 { + if strings.Contains(globPattern, "*") { + log.Printf("[WARNING] No files matching import pattern: %s", importPattern) + } else { + return p.Errf("File to import not found: %s", importPattern) } } - importedTokens = append(importedTokens, newTokens...) + // collect all the imported tokens + + for _, importFile := range matches { + newTokens, err := p.doSingleImport(importFile) + if err != nil { + return err + } + + var importLine int + for i, token := range newTokens { + if token.Text == "import" { + importLine = token.Line + continue + } + if token.Line == importLine { + var abs string + if filepath.IsAbs(token.Text) { + abs = token.Text + } else if !filepath.IsAbs(importFile) { + abs = filepath.Join(filepath.Dir(absFile), token.Text) + } else { + abs = filepath.Join(filepath.Dir(importFile), token.Text) + } + newTokens[i] = Token{ + Text: abs, + Line: token.Line, + File: token.File, + } + } + } + + importedTokens = append(importedTokens, newTokens...) + } } // splice the imported tokens in the place of the import statement @@ -433,3 +458,45 @@ type ServerBlock struct { Keys []string Tokens map[string][]Token } + +func (p *parser) isMacro() (bool, string) { + keys := p.block.Keys + // "macro foo {}" style + if len(keys) == 2 && keys[0] == "macro" { + return true, keys[1] + } + // (foo) style. What to do if more than one server key and some have ()? + if len(keys) == 1 && strings.HasPrefix(keys[0], "(") && strings.HasSuffix(keys[0], ")") { + return true, strings.TrimSuffix(keys[0][1:], ")") + } + return false, "" +} + +// read and store everything in a block for later replay. +func (p *parser) macroTokens() ([]Token, error) { + // TODO: disallow imports in macros for simplicity at import time + // macro must have curlies. + err := p.openCurlyBrace() + if err != nil { + return nil, err + } + count := 1 + tokens := []Token{} + for p.Next() { + if p.Val() == "}" { + count-- + if count == 0 { + break + } + } + if p.Val() == "{" { + count++ + } + tokens = append(tokens, p.tokens[p.cursor]) + } + // make sure we're matched up + if count != 0 { + return nil, p.SyntaxErr("}") + } + return tokens, nil +} diff --git a/caddyfile/parse_test.go b/caddyfile/parse_test.go index ff140703..9daebcf5 100644 --- a/caddyfile/parse_test.go +++ b/caddyfile/parse_test.go @@ -15,6 +15,7 @@ package caddyfile import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -514,3 +515,43 @@ func testParser(input string) parser { p := parser{Dispenser: NewDispenser("Caddyfile", buf)} return p } + +func TestMacro(t *testing.T) { + for _, tst := range []string{"(common)", "macro common"} { + t.Run(tst, func(t *testing.T) { + p := testParser(fmt.Sprintf(` + %s { + gzip foo + errors stderr + } + http://example.com { + import common + } + `, tst)) + blocks, err := p.parseAll() + if err != nil { + t.Fatal(err) + } + for _, b := range blocks { + t.Log(b.Keys) + t.Log(b.Tokens) + } + if len(blocks) != 1 { + t.Fatalf("Expect exactly one server block. Got %d.", len(blocks)) + } + if actual, expected := blocks[0].Keys[0], "http://example.com"; expected != actual { + t.Errorf("Expected server name to be '%s' but was '%s'", expected, actual) + } + if len(blocks[0].Tokens) != 2 { + t.Fatalf("Server block should have tokens from import") + } + if actual, expected := blocks[0].Tokens["gzip"][0].Text, "gzip"; expected != actual { + t.Errorf("Expected argument to be '%s' but was '%s'", expected, actual) + } + if actual, expected := blocks[0].Tokens["errors"][1].Text, "stderr"; expected != actual { + t.Errorf("Expected argument to be '%s' but was '%s'", expected, actual) + } + }) + } + +} From 3a969bc07588a8c92be4c3cfd096ba1f24e98270 Mon Sep 17 00:00:00 2001 From: Craig Peterson Date: Fri, 13 Oct 2017 11:08:17 -0400 Subject: [PATCH 2/6] add nil check --- caddyfile/parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caddyfile/parse.go b/caddyfile/parse.go index 1bf84ecc..84eb2181 100644 --- a/caddyfile/parse.go +++ b/caddyfile/parse.go @@ -247,7 +247,7 @@ func (p *parser) doImport() error { var importedTokens []Token // first check macros. That is a simple, non-recursive replacement - if p.definedMacros[importPattern] != nil { + if p.definedMacros != nil && p.definedMacros[importPattern] != nil { importedTokens = p.definedMacros[importPattern] } else { // make path relative to Caddyfile rather than current working directory (issue #867) From dd4b3efa47b15e437d0dfa0440761d3c43ea1d22 Mon Sep 17 00:00:00 2001 From: Craig Peterson Date: Sun, 15 Oct 2017 19:10:56 -0400 Subject: [PATCH 3/6] remove 'macro foo' syntax --- caddyfile/parse.go | 6 +---- caddyfile/parse_test.go | 54 ++++++++++++++++++----------------------- 2 files changed, 25 insertions(+), 35 deletions(-) diff --git a/caddyfile/parse.go b/caddyfile/parse.go index 84eb2181..c0ec2d4c 100644 --- a/caddyfile/parse.go +++ b/caddyfile/parse.go @@ -461,11 +461,7 @@ type ServerBlock struct { func (p *parser) isMacro() (bool, string) { keys := p.block.Keys - // "macro foo {}" style - if len(keys) == 2 && keys[0] == "macro" { - return true, keys[1] - } - // (foo) style. What to do if more than one server key and some have ()? + // A macro block is a single key with parens. Nothing else qualifies. if len(keys) == 1 && strings.HasPrefix(keys[0], "(") && strings.HasSuffix(keys[0], ")") { return true, strings.TrimSuffix(keys[0][1:], ")") } diff --git a/caddyfile/parse_test.go b/caddyfile/parse_test.go index 9daebcf5..34da75d1 100644 --- a/caddyfile/parse_test.go +++ b/caddyfile/parse_test.go @@ -15,7 +15,6 @@ package caddyfile import ( - "fmt" "io/ioutil" "os" "path/filepath" @@ -517,41 +516,36 @@ func testParser(input string) parser { } func TestMacro(t *testing.T) { - for _, tst := range []string{"(common)", "macro common"} { - t.Run(tst, func(t *testing.T) { - p := testParser(fmt.Sprintf(` - %s { + p := testParser(`(common) { gzip foo errors stderr } http://example.com { import common } - `, tst)) - blocks, err := p.parseAll() - if err != nil { - t.Fatal(err) - } - for _, b := range blocks { - t.Log(b.Keys) - t.Log(b.Tokens) - } - if len(blocks) != 1 { - t.Fatalf("Expect exactly one server block. Got %d.", len(blocks)) - } - if actual, expected := blocks[0].Keys[0], "http://example.com"; expected != actual { - t.Errorf("Expected server name to be '%s' but was '%s'", expected, actual) - } - if len(blocks[0].Tokens) != 2 { - t.Fatalf("Server block should have tokens from import") - } - if actual, expected := blocks[0].Tokens["gzip"][0].Text, "gzip"; expected != actual { - t.Errorf("Expected argument to be '%s' but was '%s'", expected, actual) - } - if actual, expected := blocks[0].Tokens["errors"][1].Text, "stderr"; expected != actual { - t.Errorf("Expected argument to be '%s' but was '%s'", expected, actual) - } - }) + `) + blocks, err := p.parseAll() + if err != nil { + t.Fatal(err) + } + for _, b := range blocks { + t.Log(b.Keys) + t.Log(b.Tokens) + } + if len(blocks) != 1 { + t.Fatalf("Expect exactly one server block. Got %d.", len(blocks)) + } + if actual, expected := blocks[0].Keys[0], "http://example.com"; expected != actual { + t.Errorf("Expected server name to be '%s' but was '%s'", expected, actual) + } + if len(blocks[0].Tokens) != 2 { + t.Fatalf("Server block should have tokens from import") + } + if actual, expected := blocks[0].Tokens["gzip"][0].Text, "gzip"; expected != actual { + t.Errorf("Expected argument to be '%s' but was '%s'", expected, actual) + } + if actual, expected := blocks[0].Tokens["errors"][1].Text, "stderr"; expected != actual { + t.Errorf("Expected argument to be '%s' but was '%s'", expected, actual) } } From 68a495f144603e0268ca2016fa61f63654254617 Mon Sep 17 00:00:00 2001 From: Craig Peterson Date: Thu, 19 Oct 2017 10:27:10 -0400 Subject: [PATCH 4/6] actually return error on redeclaration --- caddyfile/parse.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/caddyfile/parse.go b/caddyfile/parse.go index c0ec2d4c..3b06e8b8 100644 --- a/caddyfile/parse.go +++ b/caddyfile/parse.go @@ -100,10 +100,9 @@ func (p *parser) begin() error { if p.definedMacros == nil { p.definedMacros = map[string][]Token{} } - if p.definedMacros[name] != nil { - p.Errf("redeclaration of previously declared macro %s", name) + if _, found := p.definedMacros[name]; found { + return p.Errf("redeclaration of previously declared macro %s", name) } - // consume all tokens til matched close brace tokens, err := p.macroTokens() if err != nil { From 02ac1f61c4d6e645c26a2744ef0937e57d7e01c0 Mon Sep 17 00:00:00 2001 From: Craig Peterson Date: Thu, 19 Oct 2017 19:54:15 -0400 Subject: [PATCH 5/6] retrigger build --- caddyfile/parse_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/caddyfile/parse_test.go b/caddyfile/parse_test.go index 34da75d1..62255ae9 100644 --- a/caddyfile/parse_test.go +++ b/caddyfile/parse_test.go @@ -519,6 +519,7 @@ func TestMacro(t *testing.T) { p := testParser(`(common) { gzip foo errors stderr + } http://example.com { import common From ad2956fd1d06ff43449d2e1d3e5ed05c41ebe042 Mon Sep 17 00:00:00 2001 From: Craig Peterson Date: Tue, 31 Oct 2017 23:56:24 -0400 Subject: [PATCH 6/6] snippets now --- caddyfile/parse.go | 32 ++++++++++++++++---------------- caddyfile/parse_test.go | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/caddyfile/parse.go b/caddyfile/parse.go index 3b06e8b8..142a87f9 100644 --- a/caddyfile/parse.go +++ b/caddyfile/parse.go @@ -54,7 +54,7 @@ type parser struct { block ServerBlock // current server block being parsed validDirectives []string // a directive must be valid or it's an error eof bool // if we encounter a valid EOF in a hard place - definedMacros map[string][]Token + definedSnippets map[string][]Token } func (p *parser) parseAll() ([]ServerBlock, error) { @@ -96,19 +96,19 @@ func (p *parser) begin() error { return nil } - if ok, name := p.isMacro(); ok { - if p.definedMacros == nil { - p.definedMacros = map[string][]Token{} + if ok, name := p.isSnippet(); ok { + if p.definedSnippets == nil { + p.definedSnippets = map[string][]Token{} } - if _, found := p.definedMacros[name]; found { - return p.Errf("redeclaration of previously declared macro %s", name) + if _, found := p.definedSnippets[name]; found { + return p.Errf("redeclaration of previously declared snippet %s", name) } // consume all tokens til matched close brace - tokens, err := p.macroTokens() + tokens, err := p.snippetTokens() if err != nil { return err } - p.definedMacros[name] = tokens + p.definedSnippets[name] = tokens // empty block keys so we don't save this block as a real server. p.block.Keys = nil return nil @@ -245,9 +245,9 @@ func (p *parser) doImport() error { tokensAfter := p.tokens[p.cursor+1:] var importedTokens []Token - // first check macros. That is a simple, non-recursive replacement - if p.definedMacros != nil && p.definedMacros[importPattern] != nil { - importedTokens = p.definedMacros[importPattern] + // first check snippets. That is a simple, non-recursive replacement + if p.definedSnippets != nil && p.definedSnippets[importPattern] != nil { + importedTokens = p.definedSnippets[importPattern] } else { // make path relative to Caddyfile rather than current working directory (issue #867) // and then use glob to get list of matching filenames @@ -458,9 +458,9 @@ type ServerBlock struct { Tokens map[string][]Token } -func (p *parser) isMacro() (bool, string) { +func (p *parser) isSnippet() (bool, string) { keys := p.block.Keys - // A macro block is a single key with parens. Nothing else qualifies. + // A snippet block is a single key with parens. Nothing else qualifies. if len(keys) == 1 && strings.HasPrefix(keys[0], "(") && strings.HasSuffix(keys[0], ")") { return true, strings.TrimSuffix(keys[0][1:], ")") } @@ -468,9 +468,9 @@ func (p *parser) isMacro() (bool, string) { } // read and store everything in a block for later replay. -func (p *parser) macroTokens() ([]Token, error) { - // TODO: disallow imports in macros for simplicity at import time - // macro must have curlies. +func (p *parser) snippetTokens() ([]Token, error) { + // TODO: disallow imports in snippets for simplicity at import time + // snippet must have curlies. err := p.openCurlyBrace() if err != nil { return nil, err diff --git a/caddyfile/parse_test.go b/caddyfile/parse_test.go index 62255ae9..f119f7c0 100644 --- a/caddyfile/parse_test.go +++ b/caddyfile/parse_test.go @@ -515,7 +515,7 @@ func testParser(input string) parser { return p } -func TestMacro(t *testing.T) { +func TestSnippets(t *testing.T) { p := testParser(`(common) { gzip foo errors stderr