plugin/rewrite: tweak parse error messaging, add tests (#1737)

* tweak parse error messaging, add tests

* looser err msg checking

plugin/rewrite/README.md

@@ -155,7 +155,7 @@ ftp-us-west-1.coredns.rocks. 0    IN A    10.20.20.20
 ftp-us-west-1.coredns.rocks. 0    IN A    10.30.30.30
 ```
 
-The syntax for the response of DNS request and response is as follows:
+The syntax for the rewrite of DNS request and response is as follows:
 
 ```
 rewrite [continue|stop] {
@@ -164,6 +164,19 @@ rewrite [continue|stop] {
 }
 ```
 
+Note that the above syntax is strict.  For response rewrites only `name`
+rules are allowed to match the question section, and only by match type
+`regex`. The answer rewrite must be after the name, as ordered in the
+syntax example. There must only be two lines (a `name` follwed by an
+`answer`) in the brackets, additional rules are not supported.
+
+An alternate syntax for the rewrite of DNS request and response is as
+follows:
+
+```
+rewrite [continue|stop] name regex STRING STRING answer name STRING STRING
+```
+
 ## EDNS0 Options
 
 Using FIELD edns0, you can set, append, or replace specific EDNS0 options on the request.

plugin/rewrite/name.go

@@ -167,7 +167,7 @@ func newNameRule(nextAction string, args ...string) (Rule, error) {
 		return nil, fmt.Errorf("the rewrite of response is supported only for name regex rule")
 	}
 	if len(args) > 3 && len(args) != 7 {
-		return nil, fmt.Errorf("exceeded the number of arguments for a name rule")
+		return nil, fmt.Errorf("response rewrites must consist only of a name rule with 3 arguments and an answer rule with 3 arguments")
 	}
 	return &nameRule{nextAction, plugin.Name(args[0]).Normalize(), plugin.Name(args[1]).Normalize()}, nil
 }

plugin/rewrite/rewrite.go

@@ -91,7 +91,7 @@ func newRule(args ...string) (Rule, error) {
 	mode := Stop
 	switch arg0 {
 	case Continue:
-		mode = arg0
+		mode = Continue
 		ruleType = strings.ToLower(args[1])
 		expectNumArgs = len(args) - 1
 		startArg = 2
@@ -106,9 +106,14 @@ func newRule(args ...string) (Rule, error) {
 		startArg = 1
 	}
 
+	if ruleType == "answer" {
+		return nil, fmt.Errorf("response rewrites must begin with a name rule")
+	}
+
 	if ruleType != "edns0" && ruleType != "name" && expectNumArgs != 3 {
 		return nil, fmt.Errorf("%s rules must have exactly two arguments", ruleType)
 	}
+
 	switch ruleType {
 	case "name":
 		return newNameRule(mode, args[startArg:]...)

plugin/rewrite/setup_test.go

 package rewrite
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/mholt/caddy"
@@ -22,4 +23,45 @@ func TestParse(t *testing.T) {
 	if err != nil {
 		t.Errorf("Expected success but found %s for `rewrite name a.com b.com`", err)
 	}
+
+	c = caddy.NewTestController("dns",
+		`rewrite stop {
+    name regex foo bar
+    answer name bar foo
+}`)
+	_, err = rewriteParse(c)
+	if err != nil {
+		t.Errorf("Expected success but found %s for valid response rewrite", err)
+	}
+
+	c = caddy.NewTestController("dns", `rewrite stop name regex foo bar answer name bar foo`)
+	_, err = rewriteParse(c)
+	if err != nil {
+		t.Errorf("Expected success but found %s for valid response rewrite", err)
+	}
+
+	c = caddy.NewTestController("dns",
+		`rewrite stop {
+    name regex foo bar
+    answer name bar foo
+    name baz qux
+}`)
+	_, err = rewriteParse(c)
+	if err == nil {
+		t.Errorf("Expected error but got success for invalid response rewrite")
+	} else if !strings.Contains(err.Error(), "must consist only of") {
+		t.Errorf("Got wrong error for invalid response rewrite: %v", err.Error())
+	}
+
+	c = caddy.NewTestController("dns",
+		`rewrite stop {
+    answer name bar foo
+    name regex foo bar
+}`)
+	_, err = rewriteParse(c)
+	if err == nil {
+		t.Errorf("Expected error but got success for invalid response rewrite")
+	} else if !strings.Contains(err.Error(), "must begin with a name rule") {
+		t.Errorf("Got wrong error for invalid response rewrite: %v", err.Error())
+	}
 }