fix for new rewrite test case and add table test to replacer (#890)

* rewrite: fix new case
- added new test case and solution
- fix test case on rewrite_test

* replacer: change to table test

caddyhttp/httpserver/replacer_test.go

@@ -58,54 +58,45 @@ func TestReplace(t *testing.T) {
 	if err != nil {
 		t.Fatal("Failed to determine hostname\n")
 	}
-	if expected, actual := "This hostname is "+hostname, repl.Replace("This hostname is {hostname}"); expected != actual {
-		t.Errorf("{hostname} replacement: expected '%s', got '%s'", expected, actual)
-	}
-
-	if expected, actual := "This host is localhost.", repl.Replace("This host is {host}."); expected != actual {
-		t.Errorf("{host} replacement: expected '%s', got '%s'", expected, actual)
-	}
-	if expected, actual := "This request method is POST.", repl.Replace("This request method is {method}."); expected != actual {
-		t.Errorf("{method} replacement: expected '%s', got '%s'", expected, actual)
-	}
-	if expected, actual := "The response status is 200.", repl.Replace("The response status is {status}."); expected != actual {
-		t.Errorf("{status} replacement: expected '%s', got '%s'", expected, actual)
-	}
-	if expected, actual := "The Custom header is foobarbaz.", repl.Replace("The Custom header is {>Custom}."); expected != actual {
-		t.Errorf("{>Custom} replacement: expected '%s', got '%s'", expected, actual)
-	}
-	if expected, actual := "The request is POST / HTTP/1.1\\r\\nHost: localhost\\r\\nCustom: foobarbaz\\r\\nShorterval: 1\\r\\n\\r\\n.", repl.Replace("The request is {request}."); expected != actual {
-		t.Errorf("{request} replacement: expected '%s', got '%s'", expected, actual)
-	}
-
-	// Test header case-insensitivity
-	if expected, actual := "The cUsToM header is foobarbaz...", repl.Replace("The cUsToM header is {>cUsToM}..."); expected != actual {
-		t.Errorf("{>cUsToM} replacement: expected '%s', got '%s'", expected, actual)
-	}
-
-	// Test non-existent header/value
-	if expected, actual := "The Non-Existent header is -.", repl.Replace("The Non-Existent header is {>Non-Existent}."); expected != actual {
-		t.Errorf("{>Non-Existent} replacement: expected '%s', got '%s'", expected, actual)
-	}
-
-	// Test bad placeholder
-	if expected, actual := "Bad {host placeholder...", repl.Replace("Bad {host placeholder..."); expected != actual {
-		t.Errorf("bad placeholder: expected '%s', got '%s'", expected, actual)
-	}
 
-	// Test bad header placeholder
-	if expected, actual := "Bad {>Custom placeholder", repl.Replace("Bad {>Custom placeholder"); expected != actual {
-		t.Errorf("bad header placeholder: expected '%s', got '%s'", expected, actual)
+	testCases := []struct {
+		template string
+		expect   string
+	}{
+		{"This hostname is {hostname}", "This hostname is " + hostname},
+		{"This host is {host}.", "This host is localhost."},
+		{"This request method is {method}.", "This request method is POST."},
+		{"The response status is {status}.", "The response status is 200."},
+		{"The Custom header is {>Custom}.", "The Custom header is foobarbaz."},
+		{"The request is {request}.", "The request is POST / HTTP/1.1\\r\\nHost: localhost\\r\\nCustom: foobarbaz\\r\\nShorterval: 1\\r\\n\\r\\n."},
+		{"The cUsToM header is {>cUsToM}...", "The cUsToM header is foobarbaz..."},
+		{"The Non-Existent header is {>Non-Existent}.", "The Non-Existent header is -."},
+		{"Bad {host placeholder...", "Bad {host placeholder..."},
+		{"Bad {>Custom placeholder", "Bad {>Custom placeholder"},
+		{"Bad {>Custom placeholder {>ShorterVal}", "Bad -"},
+	}
+
+	for _, c := range testCases {
+		if expected, actual := c.expect, repl.Replace(c.template); expected != actual {
+			t.Errorf("for template '%s', expected '%s', got '%s'", c.template, expected, actual)
+		}
 	}
 
-	// Test bad header placeholder with valid one later
-	if expected, actual := "Bad -", repl.Replace("Bad {>Custom placeholder {>ShorterVal}"); expected != actual {
-		t.Errorf("bad header placeholders: expected '%s', got '%s'", expected, actual)
+	complexCases := []struct {
+		template     string
+		replacements map[string]string
+		expect       string
+	}{
+		{"/a{1}/{2}", map[string]string{"{1}": "12", "{2}": ""}, "/a12/"},
 	}
 
-	// Test shorter header value with multiple placeholders
-	if expected, actual := "Short value 1 then foobarbaz.", repl.Replace("Short value {>ShorterVal} then {>Custom}."); expected != actual {
-		t.Errorf("short value: expected '%s', got '%s'", expected, actual)
+	for _, c := range complexCases {
+		repl := &replacer{
+			replacements: c.replacements,
+		}
+		if expected, actual := c.expect, repl.Replace(c.template); expected != actual {
+			t.Errorf("for template '%s', expected '%s', got '%s'", c.template, expected, actual)
+		}
 	}
 }
 

caddyhttp/rewrite/rewrite_test.go

@@ -85,7 +85,7 @@ func TestRewrite(t *testing.T) {
 		{"/abcde/abcde.html", "/a"},
 		{"/abcde/abcde.html#1234", "/a#1234"},
 		{"/ab/ab.jpg", "/ajpg"},
-		{"/reggrp/ad/12", "/a12"},
+		{"/reggrp/ad/12", "/a12/"},
 		{"/reggrp/ad/124a", "/a124/a"},
 		{"/reggrp/ad/124abc", "/a124/abc"},
 		{"/reg2grp/ad/124abc", "/ad/124abc"},

caddyhttp/rewrite/to.go

@@ -28,6 +28,10 @@ func To(fs http.FileSystem, r *http.Request, to string, replacer httpserver.Repl
 			query = tparts[1]
 		}
 
+		if strings.HasSuffix(tparts[0], "/") && !strings.HasSuffix(t, "/") {
+			t += "/"
+		}
+
 		// add trailing slash for directories, if present
 		if strings.HasSuffix(v, "/") && !strings.HasSuffix(t, "/") {
 			t += "/"

caddyhttp/rewrite/to_test.go

@@ -22,7 +22,8 @@ func TestTo(t *testing.T) {
 		{"/?a=b", "/testfile /index.php?{query}", "/testfile?a=b"},
 		{"/?a=b", "/testdir /index.php?{query}", "/index.php?a=b"},
 		{"/?a=b", "/testdir/ /index.php?{query}", "/testdir/?a=b"},
-		{"/test?url=http://caddyserver.com", " /p/{path}?{query}", "/p/test?url=http://caddyserver.com"},
+		{"/test?url=http://", " /p/{path}?{query}", "/p/test?url=http://"},
+		{"/test/?url=http://", " /{uri}", "/test/?url=http://"},
 	}
 
 	uri := func(r *url.URL) string {