Commit 3f9f675c authored by Matthew Holt's avatar Matthew Holt

redir: Include scheme in redirect rules

And added tests for status code and scheme
parent 698399e6
......@@ -37,13 +37,13 @@ func redirParse(c *Controller) ([]redirect.Rule, error) {
// checkAndSaveRule checks the rule for validity (except the redir code)
// and saves it if it's valid, or returns an error.
checkAndSaveRule := func(rule redirect.Rule) error {
if rule.From == rule.To {
if rule.FromPath == rule.To {
return c.Err("'from' and 'to' values of redirect rule cannot be the same")
}
for _, otherRule := range redirects {
if otherRule.From == rule.From {
return c.Errf("rule with duplicate 'from' value: %s -> %s", otherRule.From, otherRule.To)
if otherRule.FromPath == rule.FromPath {
return c.Errf("rule with duplicate 'from' value: %s -> %s", otherRule.FromPath, otherRule.To)
}
}
......@@ -60,6 +60,12 @@ func redirParse(c *Controller) ([]redirect.Rule, error) {
var rule redirect.Rule
if c.Config.TLS.Enabled {
rule.FromScheme = "https"
} else {
rule.FromScheme = "http"
}
// Set initial redirect code
// BUG: If the code is specified for a whole block and that code is invalid,
// the line number will appear on the first line inside the block, even if that
......@@ -84,15 +90,15 @@ func redirParse(c *Controller) ([]redirect.Rule, error) {
// To specified (catch-all redirect)
// Not sure why user is doing this in a table, as it causes all other redirects to be ignored.
// As such, this feature remains undocumented.
rule.From = "/"
rule.FromPath = "/"
rule.To = insideArgs[0]
case 2:
// From and To specified
rule.From = insideArgs[0]
rule.FromPath = insideArgs[0]
rule.To = insideArgs[1]
case 3:
// From, To, and Code specified
rule.From = insideArgs[0]
rule.FromPath = insideArgs[0]
rule.To = insideArgs[1]
err := setRedirCode(insideArgs[2], &rule)
if err != nil {
......@@ -110,16 +116,23 @@ func redirParse(c *Controller) ([]redirect.Rule, error) {
if !hadOptionalBlock {
var rule redirect.Rule
if c.Config.TLS.Enabled {
rule.FromScheme = "https"
} else {
rule.FromScheme = "http"
}
rule.Code = http.StatusMovedPermanently // default
switch len(args) {
case 1:
// To specified (catch-all redirect)
rule.From = "/"
rule.FromPath = "/"
rule.To = args[0]
case 2:
// To and Code specified (catch-all redirect)
rule.From = "/"
rule.FromPath = "/"
rule.To = args[0]
err := setRedirCode(args[1], &rule)
if err != nil {
......@@ -127,7 +140,7 @@ func redirParse(c *Controller) ([]redirect.Rule, error) {
}
case 3:
// From, To, and Code specified
rule.From = args[0]
rule.FromPath = args[0]
rule.To = args[1]
err := setRedirCode(args[2], &rule)
if err != nil {
......@@ -149,12 +162,12 @@ func redirParse(c *Controller) ([]redirect.Rule, error) {
// httpRedirs is a list of supported HTTP redirect codes.
var httpRedirs = map[string]int{
"300": 300, // Multiple Choices
"301": 301, // Moved Permanently
"302": 302, // Found (NOT CORRECT for "Temporary Redirect", see 307)
"303": 303, // See Other
"304": 304, // Not Modified
"305": 305, // Use Proxy
"307": 307, // Temporary Redirect
"300": http.StatusMultipleChoices,
"301": http.StatusMovedPermanently,
"302": http.StatusFound, // (NOT CORRECT for "Temporary Redirect", see 307)
"303": http.StatusSeeOther,
"304": http.StatusNotModified,
"305": http.StatusUseProxy,
"307": http.StatusTemporaryRedirect,
"308": 308, // Permanent Redirect
}
......@@ -19,7 +19,7 @@ type Redirect struct {
// ServeHTTP implements the middleware.Handler interface.
func (rd Redirect) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
for _, rule := range rd.Rules {
if rule.From == "/" || r.URL.Path == rule.From {
if (rule.FromPath == "/" || r.URL.Path == rule.FromPath) && schemeMatches(rule, r) {
to := middleware.NewReplacer(r, nil, "").Replace(rule.To)
if rule.Meta {
safeTo := html.EscapeString(to)
......@@ -33,9 +33,14 @@ func (rd Redirect) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error
return rd.Next.ServeHTTP(w, r)
}
func schemeMatches(rule Rule, req *http.Request) bool {
return (rule.FromScheme == "https" && req.TLS != nil) ||
(rule.FromScheme != "https" && req.TLS == nil)
}
// Rule describes an HTTP redirect rule.
type Rule struct {
From, To string
FromScheme, FromPath, To string
Code int
Meta bool
}
......
......@@ -2,9 +2,11 @@ package redirect
import (
"bytes"
"crypto/tls"
"io/ioutil"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/mholt/caddy/middleware"
......@@ -14,15 +16,22 @@ func TestRedirect(t *testing.T) {
for i, test := range []struct {
from string
expectedLocation string
expectedCode int
}{
{"/from", "/to"},
{"/a", "/b"},
{"/aa", ""},
{"/", ""},
{"/a?foo=bar", "/b"},
{"/asdf?foo=bar", ""},
{"/foo#bar", ""},
{"/a#foo", "/b"},
{"http://localhost/from", "/to", http.StatusMovedPermanently},
{"http://localhost/a", "/b", http.StatusTemporaryRedirect},
{"http://localhost/aa", "", http.StatusOK},
{"http://localhost/", "", http.StatusOK},
{"http://localhost/a?foo=bar", "/b", http.StatusTemporaryRedirect},
{"http://localhost/asdf?foo=bar", "", http.StatusOK},
{"http://localhost/foo#bar", "", http.StatusOK},
{"http://localhost/a#foo", "/b", http.StatusTemporaryRedirect},
{"http://localhost/scheme", "https://localhost/scheme", http.StatusMovedPermanently},
{"https://localhost/scheme", "", http.StatusOK},
{"https://localhost/scheme2", "http://localhost/scheme2", http.StatusMovedPermanently},
{"http://localhost/scheme2", "", http.StatusOK},
{"http://localhost/scheme3", "https://localhost/scheme3", http.StatusMovedPermanently},
{"https://localhost/scheme3", "", http.StatusOK},
} {
var nextCalled bool
......@@ -32,8 +41,11 @@ func TestRedirect(t *testing.T) {
return 0, nil
}),
Rules: []Rule{
{From: "/from", To: "/to"},
{From: "/a", To: "/b"},
{FromPath: "/from", To: "/to", Code: http.StatusMovedPermanently},
{FromPath: "/a", To: "/b", Code: http.StatusTemporaryRedirect},
{FromScheme: "http", FromPath: "/scheme", To: "https://localhost/scheme", Code: http.StatusMovedPermanently},
{FromScheme: "https", FromPath: "/scheme2", To: "http://localhost/scheme2", Code: http.StatusMovedPermanently},
{FromScheme: "", FromPath: "/scheme3", To: "https://localhost/scheme3", Code: http.StatusMovedPermanently},
},
}
......@@ -41,6 +53,9 @@ func TestRedirect(t *testing.T) {
if err != nil {
t.Fatalf("Test %d: Could not create HTTP request: %v", i, err)
}
if strings.HasPrefix(test.from, "https://") {
req.TLS = new(tls.ConnectionState) // faux HTTPS
}
rec := httptest.NewRecorder()
re.ServeHTTP(rec, req)
......@@ -50,6 +65,11 @@ func TestRedirect(t *testing.T) {
i, test.expectedLocation, rec.Header().Get("Location"))
}
if rec.Code != test.expectedCode {
t.Errorf("Test %d: Expected status code to be %d but was %d",
i, test.expectedCode, rec.Code)
}
if nextCalled && test.expectedLocation != "" {
t.Errorf("Test %d: Next handler was unexpectedly called", i)
}
......@@ -59,7 +79,7 @@ func TestRedirect(t *testing.T) {
func TestParametersRedirect(t *testing.T) {
re := Redirect{
Rules: []Rule{
{From: "/", Meta: false, To: "http://example.com{uri}"},
{FromPath: "/", Meta: false, To: "http://example.com{uri}"},
},
}
......@@ -77,7 +97,7 @@ func TestParametersRedirect(t *testing.T) {
re = Redirect{
Rules: []Rule{
{From: "/", Meta: false, To: "http://example.com/a{path}?b=c&{query}"},
{FromPath: "/", Meta: false, To: "http://example.com/a{path}?b=c&{query}"},
},
}
......@@ -96,13 +116,13 @@ func TestParametersRedirect(t *testing.T) {
func TestMetaRedirect(t *testing.T) {
re := Redirect{
Rules: []Rule{
{From: "/whatever", Meta: true, To: "/something"},
{From: "/", Meta: true, To: "https://example.com/"},
{FromPath: "/whatever", Meta: true, To: "/something"},
{FromPath: "/", Meta: true, To: "https://example.com/"},
},
}
for i, test := range re.Rules {
req, err := http.NewRequest("GET", test.From, nil)
req, err := http.NewRequest("GET", test.FromPath, nil)
if err != nil {
t.Fatalf("Test %d: Could not create HTTP request: %v", i, err)
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment