From 2459c292a4d6fb0552eb9be3cecd955093ed853b Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Tue, 10 Sep 2019 19:21:52 -0600
Subject: caddyfile: Improve Dispenser.NextBlock() to support nesting

---
 caddyconfig/caddyfile/dispenser.go                 | 75 ++++++++++++++++------
 caddyconfig/caddyfile/dispenser_test.go            |  2 +-
 caddyconfig/httpcaddyfile/builtins.go              |  2 +-
 caddyconfig/httpcaddyfile/handlers.go              |  2 +-
 caddyconfig/httpcaddyfile/options.go               |  4 +-
 modules/caddyhttp/encode/caddyfile.go              |  2 +-
 modules/caddyhttp/fileserver/caddyfile.go          |  2 +-
 modules/caddyhttp/fileserver/matcher.go            |  2 +-
 modules/caddyhttp/headers/caddyfile.go             |  2 +-
 modules/caddyhttp/matchers.go                      |  3 +
 modules/caddyhttp/reverseproxy/caddyfile.go        |  5 +-
 .../caddyhttp/reverseproxy/fastcgi/caddyfile.go    | 52 +++++++--------
 modules/caddyhttp/staticresp.go                    |  2 +-
 modules/caddyhttp/templates/caddyfile.go           |  2 +-
 14 files changed, 97 insertions(+), 60 deletions(-)

diff --git a/caddyconfig/caddyfile/dispenser.go b/caddyconfig/caddyfile/dispenser.go
index 938c985..de67ee1 100755
--- a/caddyconfig/caddyfile/dispenser.go
+++ b/caddyconfig/caddyfile/dispenser.go
@@ -123,18 +123,39 @@ func (d *Dispenser) NextLine() bool {
 
 // NextBlock can be used as the condition of a for loop
 // to load the next token as long as it opens a block or
-// is already in a block. It returns true if a token was
-// loaded, or false when the block's closing curly brace
-// was loaded and thus the block ended. Nested blocks are
-// not supported.
-func (d *Dispenser) NextBlock() bool {
-	if d.nesting > 0 {
-		d.Next()
+// is already in a block nested more than initialNestingLevel.
+// In other words, a loop over NextBlock() will iterate
+// all tokens in the block assuming the next token is an
+// open curly brace, until the matching closing brace.
+// The open and closing brace tokens for the outer-most
+// block will be consumed internally and omitted from
+// the iteration.
+//
+// Proper use of this method looks like this:
+//
+//     for nesting := d.Nesting(); d.NextBlock(nesting); {
+//     }
+//
+// However, in simple cases where it is known that the
+// Dispenser is new and has not already traversed state
+// by a loop over NextBlock(), this will do:
+//
+//     for d.NextBlock(0) {
+//     }
+//
+// As with other token parsing logic, a loop over
+// NextBlock() should be contained within a loop over
+// Next(), as it is usually prudent to skip the initial
+// token.
+func (d *Dispenser) NextBlock(initialNestingLevel int) bool {
+	if d.nesting > initialNestingLevel {
+		if !d.Next() {
+			return false // should be EOF error
+		}
 		if d.Val() == "}" {
 			d.nesting--
-			return false
 		}
-		return true
+		return d.nesting > initialNestingLevel
 	}
 	if !d.nextOnSameLine() { // block must open on same line
 		return false
@@ -143,19 +164,18 @@ func (d *Dispenser) NextBlock() bool {
 		d.cursor-- // roll back if not opening brace
 		return false
 	}
-	d.Next()
+	d.Next() // consume open curly brace
 	if d.Val() == "}" {
-		// open and then closed right away
-		return false
+		return false // open and then closed right away
 	}
 	d.nesting++
 	return true
 }
 
-// Nested returns true if the token is currently nested
-// inside a block (i.e. an open curly brace was consumed).
-func (d *Dispenser) Nested() bool {
-	return d.nesting > 0
+// Nesting returns the current nesting level. Necessary
+// if using NextBlock()
+func (d *Dispenser) Nesting() int {
+	return d.nesting
 }
 
 // Val gets the text of the current token. If there is no token
@@ -230,19 +250,32 @@ func (d *Dispenser) RemainingArgs() []string {
 // NewFromNextTokens 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.
+// 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()}
 	for d.NextArg() {
 		tkns = append(tkns, d.Token())
 	}
-	for d.NextBlock() {
-		for d.Nested() {
+	var openedBlock bool
+	for nesting := d.Nesting(); d.NextBlock(nesting); {
+		if !openedBlock {
+			// because NextBlock() consumes the initial open
+			// curly brace, we rewind here to append it, since
+			// our case is special in that we want to include
+			// all the tokens including surrounding curly braces
+			// for a new dispenser to have
+			d.Prev()
 			tkns = append(tkns, d.Token())
-			d.NextBlock()
+			d.Next()
+			openedBlock = true
 		}
+		tkns = append(tkns, d.Token())
+	}
+	if openedBlock {
+		// include closing brace accordingly
+		tkns = append(tkns, d.Token())
 	}
-	tkns = append(tkns, d.Token())
 	return NewDispenser(tkns)
 }
 
diff --git a/caddyconfig/caddyfile/dispenser_test.go b/caddyconfig/caddyfile/dispenser_test.go
index 07fbbee..494db36 100755
--- a/caddyconfig/caddyfile/dispenser_test.go
+++ b/caddyconfig/caddyfile/dispenser_test.go
@@ -148,7 +148,7 @@ func TestDispenser_NextBlock(t *testing.T) {
 	d := newTestDispenser(input)
 
 	assertNextBlock := func(shouldLoad bool, expectedCursor, expectedNesting int) {
-		if loaded := d.NextBlock(); loaded != shouldLoad {
+		if loaded := d.NextBlock(0); loaded != shouldLoad {
 			t.Errorf("NextBlock(): Should return %v but got %v", shouldLoad, loaded)
 		}
 		if d.cursor != expectedCursor {
diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go
index db2eee5..0ac436e 100644
--- a/caddyconfig/httpcaddyfile/builtins.go
+++ b/caddyconfig/httpcaddyfile/builtins.go
@@ -107,7 +107,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 		}
 
 		var hasBlock bool
-		for h.NextBlock() {
+		for h.NextBlock(0) {
 			hasBlock = true
 
 			switch h.Val() {
diff --git a/caddyconfig/httpcaddyfile/handlers.go b/caddyconfig/httpcaddyfile/handlers.go
index 9a29e97..e133028 100644
--- a/caddyconfig/httpcaddyfile/handlers.go
+++ b/caddyconfig/httpcaddyfile/handlers.go
@@ -28,7 +28,7 @@ func (st *ServerType) parseMatcherDefinitions(d *caddyfile.Dispenser) (map[strin
 	matchers := make(map[string]map[string]json.RawMessage)
 	for d.Next() {
 		definitionName := d.Val()
-		for d.NextBlock() {
+		for nesting := d.Nesting(); d.NextBlock(nesting); {
 			matcherName := d.Val()
 			mod, err := caddy.GetModule("http.matchers." + matcherName)
 			if err != nil {
diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go
index f99ce51..d9fb4b0 100644
--- a/caddyconfig/httpcaddyfile/options.go
+++ b/caddyconfig/httpcaddyfile/options.go
@@ -60,10 +60,10 @@ func parseHandlerOrder(d *caddyfile.Dispenser) ([]string, error) {
 	if len(order) == 1 && order[0] == "appearance" {
 		return []string{"appearance"}, nil
 	}
-	if len(order) > 0 && d.NextBlock() {
+	if len(order) > 0 && d.NextBlock(0) {
 		return nil, d.Err("cannot open block if there are arguments")
 	}
-	for d.NextBlock() {
+	for d.NextBlock(0) {
 		order = append(order, d.Val())
 		if d.NextArg() {
 			return nil, d.ArgErr()
diff --git a/modules/caddyhttp/encode/caddyfile.go b/modules/caddyhttp/encode/caddyfile.go
index 5762bd3..d23eab9 100644
--- a/modules/caddyhttp/encode/caddyfile.go
+++ b/modules/caddyhttp/encode/caddyfile.go
@@ -67,7 +67,7 @@ func (enc *Encode) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 			enc.EncodingsRaw[arg] = caddyconfig.JSON(encoding, nil)
 		}
 
-		for d.NextBlock() {
+		for d.NextBlock(0) {
 			name := d.Val()
 			mod, err := caddy.GetModule("http.encoders." + name)
 			if err != nil {
diff --git a/modules/caddyhttp/fileserver/caddyfile.go b/modules/caddyhttp/fileserver/caddyfile.go
index b7cb311..b50c166 100644
--- a/modules/caddyhttp/fileserver/caddyfile.go
+++ b/modules/caddyhttp/fileserver/caddyfile.go
@@ -43,7 +43,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 			return nil, h.ArgErr()
 		}
 
-		for h.NextBlock() {
+		for h.NextBlock(0) {
 			switch h.Val() {
 			case "hide":
 				fsrv.Hide = h.RemainingArgs()
diff --git a/modules/caddyhttp/fileserver/matcher.go b/modules/caddyhttp/fileserver/matcher.go
index fde086e..4a7f657 100644
--- a/modules/caddyhttp/fileserver/matcher.go
+++ b/modules/caddyhttp/fileserver/matcher.go
@@ -69,7 +69,7 @@ func (MatchFile) CaddyModule() caddy.ModuleInfo {
 //
 func (m *MatchFile) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	for d.Next() {
-		for d.NextBlock() {
+		for d.NextBlock(0) {
 			switch d.Val() {
 			case "root":
 				if !d.NextArg() {
diff --git a/modules/caddyhttp/headers/caddyfile.go b/modules/caddyhttp/headers/caddyfile.go
index 5eaf064..12ec8a0 100644
--- a/modules/caddyhttp/headers/caddyfile.go
+++ b/modules/caddyhttp/headers/caddyfile.go
@@ -49,7 +49,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 		}
 
 		// if not, they should be in a block
-		for h.NextBlock() {
+		for h.NextBlock(0) {
 			if hasArgs {
 				return nil, h.Err("cannot specify headers in both arguments and block")
 			}
diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go
index 4d0eea5..23047d0 100644
--- a/modules/caddyhttp/matchers.go
+++ b/modules/caddyhttp/matchers.go
@@ -258,6 +258,9 @@ func (MatchHeader) CaddyModule() caddy.ModuleInfo {
 
 // UnmarshalCaddyfile implements caddyfile.Unmarshaler.
 func (m *MatchHeader) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
+	if *m == nil {
+		*m = make(map[string][]string)
+	}
 	for d.Next() {
 		var field, val string
 		if !d.Args(&field, &val) {
diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go
index ffa3ca0..56b3a5a 100644
--- a/modules/caddyhttp/reverseproxy/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/caddyfile.go
@@ -81,7 +81,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 			})
 		}
 
-		for d.NextBlock() {
+		for d.NextBlock(0) {
 			switch d.Val() {
 			case "to":
 				args := d.RemainingArgs()
@@ -343,7 +343,6 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 				if !ok {
 					return d.Errf("transport module '%s' is not a Caddyfile unmarshaler", mod.Name)
 				}
-				d.Next() // consume the module name token
 				err = unm.UnmarshalCaddyfile(d.NewFromNextTokens())
 				if err != nil {
 					return err
@@ -377,7 +376,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 //     }
 //
 func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
-	for d.NextBlock() {
+	for d.NextBlock(0) {
 		switch d.Val() {
 		case "read_buffer":
 			if !d.NextArg() {
diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
index 1476d60..fd82c5a 100644
--- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
@@ -39,32 +39,34 @@ func init() {
 //     }
 //
 func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
-	for d.NextBlock() {
-		switch d.Val() {
-		case "root":
-			if !d.NextArg() {
-				return d.ArgErr()
+	for d.Next() {
+		for d.NextBlock(0) {
+			switch d.Val() {
+			case "root":
+				if !d.NextArg() {
+					return d.ArgErr()
+				}
+				t.Root = d.Val()
+
+			case "split":
+				if !d.NextArg() {
+					return d.ArgErr()
+				}
+				t.SplitPath = d.Val()
+
+			case "env":
+				args := d.RemainingArgs()
+				if len(args) != 2 {
+					return d.ArgErr()
+				}
+				if t.EnvVars == nil {
+					t.EnvVars = make(map[string]string)
+				}
+				t.EnvVars[args[0]] = args[1]
+
+			default:
+				return d.Errf("unrecognized subdirective %s", d.Val())
 			}
-			t.Root = d.Val()
-
-		case "split":
-			if !d.NextArg() {
-				return d.ArgErr()
-			}
-			t.SplitPath = d.Val()
-
-		case "env":
-			args := d.RemainingArgs()
-			if len(args) != 2 {
-				return d.ArgErr()
-			}
-			if t.EnvVars == nil {
-				t.EnvVars = make(map[string]string)
-			}
-			t.EnvVars[args[0]] = args[1]
-
-		default:
-			return d.Errf("unrecognized subdirective %s", d.Val())
 		}
 	}
 	return nil
diff --git a/modules/caddyhttp/staticresp.go b/modules/caddyhttp/staticresp.go
index 942459b..21ff9d5 100644
--- a/modules/caddyhttp/staticresp.go
+++ b/modules/caddyhttp/staticresp.go
@@ -57,7 +57,7 @@ func (s *StaticResponse) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 		if d.Args(&statusCodeStr) {
 			s.StatusCode = WeakString(statusCodeStr)
 		}
-		for d.NextBlock() {
+		for d.NextBlock(0) {
 			switch d.Val() {
 			case "body":
 				if s.Body != "" {
diff --git a/modules/caddyhttp/templates/caddyfile.go b/modules/caddyhttp/templates/caddyfile.go
index 1336a60..5dc124f 100644
--- a/modules/caddyhttp/templates/caddyfile.go
+++ b/modules/caddyhttp/templates/caddyfile.go
@@ -34,7 +34,7 @@ func init() {
 func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
 	t := new(Templates)
 	for h.Next() {
-		for h.NextBlock() {
+		for h.NextBlock(0) {
 			switch h.Val() {
 			case "mime":
 				t.MIMETypes = h.RemainingArgs()
-- 
cgit v1.2.3