From 15bf9c196c5972051f40ebadf50811bd06e328dd Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 14 Feb 2020 11:00:16 -0700 Subject: [PATCH] caddyfile: Refactor; NewFromNextSegment(); fix repeated matchers Now multiple instances of the same matcher can be used within a named matcher without overwriting previous ones. --- caddyconfig/caddyfile/dispenser.go | 15 +++++-- caddyconfig/httpcaddyfile/builtins.go | 4 +- caddyconfig/httpcaddyfile/httptype.go | 10 ++++- caddyconfig/httpcaddyfile/httptype_test.go | 50 ++++++++++++++++++++- caddyconfig/httpcaddyfile/options.go | 2 +- modules/caddyhttp/encode/caddyfile.go | 2 +- modules/caddyhttp/matchers.go | 2 +- modules/caddyhttp/reverseproxy/caddyfile.go | 4 +- 8 files changed, 76 insertions(+), 13 deletions(-) diff --git a/caddyconfig/caddyfile/dispenser.go b/caddyconfig/caddyfile/dispenser.go index 4ed93252..932ab613 100755 --- a/caddyconfig/caddyfile/dispenser.go +++ b/caddyconfig/caddyfile/dispenser.go @@ -249,13 +249,20 @@ func (d *Dispenser) RemainingArgs() []string { return args } -// NewFromNextTokens returns a new dispenser with a copy of +// NewFromNextSegment returns a new dispenser with a copy of // the tokens from the current token until the end of the // "directive" whether that be to the end of the line or // the end of a block that starts at the end of the line; // in other words, until the end of the segment. -func (d *Dispenser) NewFromNextTokens() *Dispenser { - tkns := []Token{d.Token()} +func (d *Dispenser) NewFromNextSegment() *Dispenser { + return NewDispenser(d.NextSegment()) +} + +// NextSegment returns a copy of the tokens from the current +// token until the end of the line or block that starts at +// the end of the line. +func (d *Dispenser) NextSegment() Segment { + tkns := Segment{d.Token()} for d.NextArg() { tkns = append(tkns, d.Token()) } @@ -282,7 +289,7 @@ func (d *Dispenser) NewFromNextTokens() *Dispenser { // next iteration of the enclosing loop will // call Next() and consume it } - return NewDispenser(tkns) + return tkns } // Token returns the current token. diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index bac12da0..b758c39d 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -368,7 +368,7 @@ func parseRoute(h Helper) (caddyhttp.MiddlewareHandler, error) { } subHelper := h - subHelper.Dispenser = h.NewFromNextTokens() + subHelper.Dispenser = h.NewFromNextSegment() results, err := dirFunc(subHelper) if err != nil { @@ -401,7 +401,7 @@ func parseHandle(h Helper) (caddyhttp.MiddlewareHandler, error) { } subHelper := h - subHelper.Dispenser = h.NewFromNextTokens() + subHelper.Dispenser = h.NewFromNextSegment() results, err := dirFunc(subHelper) if err != nil { diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index 5745b66e..e54456ea 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -751,8 +751,16 @@ func parseMatcherDefinitions(d *caddyfile.Dispenser, matchers map[string]caddy.M } matchers[definitionName] = make(caddy.ModuleMap) + // in case there are multiple instances of the same matcher, concatenate + // their tokens (we expect that UnmarshalCaddyfile should be able to + // handle more than one segment); otherwise, we'd overwrite other + // instances of the matcher in this set + tokensByMatcherName := make(map[string][]caddyfile.Token) for nesting := d.Nesting(); d.NextBlock(nesting); { matcherName := d.Val() + tokensByMatcherName[matcherName] = append(tokensByMatcherName[matcherName], d.NextSegment()...) + } + for matcherName, tokens := range tokensByMatcherName { mod, err := caddy.GetModule("http.matchers." + matcherName) if err != nil { return fmt.Errorf("getting matcher module '%s': %v", matcherName, err) @@ -761,7 +769,7 @@ func parseMatcherDefinitions(d *caddyfile.Dispenser, matchers map[string]caddy.M if !ok { return fmt.Errorf("matcher module '%s' is not a Caddyfile unmarshaler", matcherName) } - err = unm.UnmarshalCaddyfile(d.NewFromNextTokens()) + err = unm.UnmarshalCaddyfile(caddyfile.NewDispenser(tokens)) if err != nil { return err } diff --git a/caddyconfig/httpcaddyfile/httptype_test.go b/caddyconfig/httpcaddyfile/httptype_test.go index ae4f042b..3b696773 100644 --- a/caddyconfig/httpcaddyfile/httptype_test.go +++ b/caddyconfig/httpcaddyfile/httptype_test.go @@ -1,6 +1,54 @@ package httpcaddyfile -import "testing" +import ( + "testing" + + "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" +) + +func TestServerType(t *testing.T) { + for i, tc := range []struct { + input string + expectWarn bool + expectError bool + }{ + { + input: `http://localhost + @debug { + query showdebug=1 + } + `, + expectWarn: false, + expectError: false, + }, + { + input: `http://localhost + @debug { + query bad format + } + `, + expectWarn: false, + expectError: true, + }, + } { + + adapter := caddyfile.Adapter{ + ServerType: ServerType{}, + } + + _, warnings, err := adapter.Adapt([]byte(tc.input), nil) + + if len(warnings) > 0 != tc.expectWarn { + t.Errorf("Test %d warning expectation failed Expected: %v, got %v", i, tc.expectWarn, warnings) + continue + } + + if err != nil != tc.expectError { + t.Errorf("Test %d error expectation failed Expected: %v, got %s", i, tc.expectError, err) + continue + } + } +} func TestSpecificity(t *testing.T) { for i, tc := range []struct { diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go index 9ebefea8..fdecfa4f 100644 --- a/caddyconfig/httpcaddyfile/options.go +++ b/caddyconfig/httpcaddyfile/options.go @@ -151,7 +151,7 @@ func parseOptStorage(d *caddyfile.Dispenser) (caddy.StorageConverter, error) { if !ok { return nil, fmt.Errorf("storage module '%s' is not a Caddyfile unmarshaler", mod.ID) } - err = unm.UnmarshalCaddyfile(d.NewFromNextTokens()) + err = unm.UnmarshalCaddyfile(d.NewFromNextSegment()) if err != nil { return nil, err } diff --git a/modules/caddyhttp/encode/caddyfile.go b/modules/caddyhttp/encode/caddyfile.go index dd12de28..629f0e20 100644 --- a/modules/caddyhttp/encode/caddyfile.go +++ b/modules/caddyhttp/encode/caddyfile.go @@ -73,7 +73,7 @@ func (enc *Encode) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if !ok { return fmt.Errorf("encoder module '%s' is not a Caddyfile unmarshaler", mod) } - err = unm.UnmarshalCaddyfile(d.NewFromNextTokens()) + err = unm.UnmarshalCaddyfile(d.NewFromNextSegment()) if err != nil { return err } diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index 873b63da..552aa913 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -520,7 +520,7 @@ func (m *MatchNegate) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if !ok { return d.Errf("matcher module '%s' is not a Caddyfile unmarshaler", matcherName) } - err = unm.UnmarshalCaddyfile(d.NewFromNextTokens()) + err = unm.UnmarshalCaddyfile(d.NewFromNextSegment()) if err != nil { return err } diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index f29050f5..d08e7f1b 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -114,7 +114,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if !ok { return d.Errf("load balancing policy module '%s' is not a Caddyfile unmarshaler", mod) } - err = unm.UnmarshalCaddyfile(d.NewFromNextTokens()) + err = unm.UnmarshalCaddyfile(d.NewFromNextSegment()) if err != nil { return err } @@ -401,7 +401,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if !ok { return d.Errf("transport module '%s' is not a Caddyfile unmarshaler", mod) } - err = unm.UnmarshalCaddyfile(d.NewFromNextTokens()) + err = unm.UnmarshalCaddyfile(d.NewFromNextSegment()) if err != nil { return err }