Fix proxy upstream parser issue and add test cases (#263)

This fix tries to fix 261 where proxy upstream parser is not
able to parse upstream correctly.

Several test cases have also been added to cover the changes
and prevent regression in the future.

This fix fixes 261.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

3 files changed, 80 insertions, 3 deletions

diff --git a/middleware/proxy/setup.go b/middleware/proxy/setup.go
index a6c133f21..3757fbbe3 100644
--- a/middleware/proxy/setup.go
+++ b/middleware/proxy/setup.go
@@ -15,7 +15,7 @@ func init() {
 }
 
 func setup(c *caddy.Controller) error {
-	upstreams, err := NewStaticUpstreams(c.Dispenser)
+	upstreams, err := NewStaticUpstreams(&c.Dispenser)
 	if err != nil {
 		return middleware.Error("proxy", err)
 	}
diff --git a/middleware/proxy/upstream.go b/middleware/proxy/upstream.go
index 12ec00d76..08795e290 100644
--- a/middleware/proxy/upstream.go
+++ b/middleware/proxy/upstream.go
@@ -44,7 +44,7 @@ type Options struct {
 
 // NewStaticUpstreams parses the configuration input and sets up
 // static upstreams for the proxy middleware.
-func NewStaticUpstreams(c caddyfile.Dispenser) ([]Upstream, error) {
+func NewStaticUpstreams(c *caddyfile.Dispenser) ([]Upstream, error) {
 	var upstreams []Upstream
 	for c.Next() {
 		upstream := &staticUpstream{
@@ -126,7 +126,7 @@ func (u *staticUpstream) Options() Options {
 	return u.options
 }
 
-func parseBlock(c caddyfile.Dispenser, u *staticUpstream) error {
+func parseBlock(c *caddyfile.Dispenser, u *staticUpstream) error {
 	switch c.Val() {
 	case "policy":
 		if !c.NextArg() {
diff --git a/middleware/proxy/upstream_test.go b/middleware/proxy/upstream_test.go
index a589cf85e..c48b4446b 100644
--- a/middleware/proxy/upstream_test.go
+++ b/middleware/proxy/upstream_test.go
@@ -3,6 +3,8 @@ package proxy
 import (
 	"testing"
 	"time"
+
+	"github.com/mholt/caddy"
 )
 
 func TestHealthCheck(t *testing.T) {
@@ -75,3 +77,78 @@ func TestAllowedPaths(t *testing.T) {
 		}
 	}
 }
+
+func TestProxyParse(t *testing.T) {
+	tests := []struct {
+		inputUpstreams string
+		shouldErr      bool
+	}{
+		{
+			`proxy . 8.8.8.8:53`,
+			false,
+		},
+		{
+			`
+proxy . 8.8.8.8:53 {
+    policy round_robin
+}`,
+			false,
+		},
+		{
+			`
+proxy . 8.8.8.8:53 {
+    fail_timeout 5s
+}`,
+			false,
+		},
+		{
+			`
+proxy . 8.8.8.8:53 {
+    max_fails 10
+}`,
+			false,
+		},
+		{
+			`
+proxy . 8.8.8.8:53 {
+    health_check /health:8080
+}`,
+			false,
+		},
+		{
+			`
+proxy . 8.8.8.8:53 {
+    without without
+}`,
+			false,
+		},
+		{
+			`
+proxy . 8.8.8.8:53 {
+    except miek.nl example.org
+}`,
+			false,
+		},
+		{
+			`
+proxy . 8.8.8.8:53 {
+    spray
+}`,
+			false,
+		},
+		{
+			`
+proxy . 8.8.8.8:53 {
+    error_option
+}`,
+			true,
+		},
+	}
+	for i, test := range tests {
+		c := caddy.NewTestController("dns", test.inputUpstreams)
+		_, err := NewStaticUpstreams(&c.Dispenser)
+		if (err != nil) != test.shouldErr {
+			t.Errorf("Test %d expected no error, got %v", i+1, err)
+		}
+	}
+}