From 74b758034e75ec230409de82676be40e7a8b3bb8 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 24 Jul 2015 10:15:48 -0600 Subject: [PATCH] redir: Allows replacements; defaults to exact match redirects This is a breaking change for those who expect catch-all redirects to preserve path; use {uri} variable explicitly now --- middleware/redirect/redirect.go | 50 +++------- middleware/redirect/redirect_test.go | 132 +++++++++++++-------------- 2 files changed, 80 insertions(+), 102 deletions(-) diff --git a/middleware/redirect/redirect.go b/middleware/redirect/redirect.go index 56275d8b..6f9ab35e 100644 --- a/middleware/redirect/redirect.go +++ b/middleware/redirect/redirect.go @@ -6,9 +6,6 @@ import ( "fmt" "html" "net/http" - "net/url" - "path" - "strings" "github.com/mholt/caddy/middleware" ) @@ -22,36 +19,13 @@ 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 == "/" { - // Catchall redirect preserves path and query string - toURL, err := url.Parse(rule.To) - if err != nil { - return http.StatusInternalServerError, err - } - newPath := path.Join(toURL.Host, toURL.Path, r.URL.Path) - if strings.HasSuffix(r.URL.Path, "/") { - newPath = newPath + "/" - } - newPath = toURL.Scheme + "://" + newPath - parameters := toURL.Query() - for k, v := range r.URL.Query() { - parameters.Set(k, v[0]) - } - if len(parameters) > 0 { - newPath = newPath + "?" + parameters.Encode() - } + if rule.From == "/" || r.URL.Path == rule.From { + to := middleware.NewReplacer(r, nil, "").Replace(rule.To) if rule.Meta { - fmt.Fprintf(w, metaRedir, html.EscapeString(newPath)) + safeTo := html.EscapeString(to) + fmt.Fprintf(w, metaRedir, safeTo, safeTo) } else { - http.Redirect(w, r, newPath, rule.Code) - } - return 0, nil - } - if r.URL.Path == rule.From { - if rule.Meta { - fmt.Fprintf(w, metaRedir, html.EscapeString(rule.To)) - } else { - http.Redirect(w, r, rule.To, rule.Code) + http.Redirect(w, r, to, rule.Code) } return 0, nil } @@ -66,9 +40,13 @@ type Rule struct { Meta bool } -var metaRedir = ` - - - -redirecting... +// Script tag comes first since that will better imitate a redirect in the browser's +// history, but the meta tag is a fallback for most non-JS clients. +const metaRedir = ` + + + + + + Redirecting... ` diff --git a/middleware/redirect/redirect_test.go b/middleware/redirect/redirect_test.go index 07c83116..fd98b33a 100644 --- a/middleware/redirect/redirect_test.go +++ b/middleware/redirect/redirect_test.go @@ -10,72 +10,6 @@ import ( "github.com/mholt/caddy/middleware" ) -func TestMetaRedirect(t *testing.T) { - re := Redirect{ - Rules: []Rule{ - {From: "/", Meta: true, To: "https://example.com/"}, - {From: "/whatever", Meta: true, To: "https://example.com/whatever"}, - }, - } - - for i, test := range re.Rules { - req, err := http.NewRequest("GET", test.From, nil) - if err != nil { - t.Fatalf("Test %d: Could not create HTTP request: %v", i, err) - } - - rec := httptest.NewRecorder() - re.ServeHTTP(rec, req) - - body, err := ioutil.ReadAll(rec.Body) - if err != nil { - t.Fatalf("Test %d: Could not read HTTP response body: %v", i, err) - } - expectedSnippet := `` - if !bytes.Contains(body, []byte(expectedSnippet)) { - t.Errorf("Test %d: Expected Response Body to contain %q but was %q", - i, expectedSnippet, body) - } - } -} - -func TestParametersRedirect(t *testing.T) { - re := Redirect{ - Rules: []Rule{ - {From: "/", Meta: false, To: "http://example.com/"}, - }, - } - - req, err := http.NewRequest("GET", "/a?b=c", nil) - if err != nil { - t.Fatalf("Test: Could not create HTTP request: %v", err) - } - - rec := httptest.NewRecorder() - re.ServeHTTP(rec, req) - - if "http://example.com/a?b=c" != rec.Header().Get("Location") { - t.Fatalf("Test: expected location header %q but was %q", "http://example.com/a?b=c", rec.Header().Get("Location")) - } - - re = Redirect{ - Rules: []Rule{ - {From: "/", Meta: false, To: "http://example.com/a?b=c"}, - }, - } - - req, err = http.NewRequest("GET", "/d?e=f", nil) - if err != nil { - t.Fatalf("Test: Could not create HTTP request: %v", err) - } - - re.ServeHTTP(rec, req) - - if "http://example.com/a/d?b=c&e=f" != rec.Header().Get("Location") { - t.Fatalf("Test: expected location header %q but was %q", "http://example.com/a/d?b=c&e=f", rec.Header().Get("Location")) - } -} - func TestRedirect(t *testing.T) { for i, test := range []struct { from string @@ -121,3 +55,69 @@ func TestRedirect(t *testing.T) { } } } + +func TestParametersRedirect(t *testing.T) { + re := Redirect{ + Rules: []Rule{ + {From: "/", Meta: false, To: "http://example.com{uri}"}, + }, + } + + req, err := http.NewRequest("GET", "/a?b=c", nil) + if err != nil { + t.Fatalf("Test: Could not create HTTP request: %v", err) + } + + rec := httptest.NewRecorder() + re.ServeHTTP(rec, req) + + if rec.Header().Get("Location") != "http://example.com/a?b=c" { + t.Fatalf("Test: expected location header %q but was %q", "http://example.com/a?b=c", rec.Header().Get("Location")) + } + + re = Redirect{ + Rules: []Rule{ + {From: "/", Meta: false, To: "http://example.com/a{path}?b=c&{query}"}, + }, + } + + req, err = http.NewRequest("GET", "/d?e=f", nil) + if err != nil { + t.Fatalf("Test: Could not create HTTP request: %v", err) + } + + re.ServeHTTP(rec, req) + + if "http://example.com/a/d?b=c&e=f" != rec.Header().Get("Location") { + t.Fatalf("Test: expected location header %q but was %q", "http://example.com/a/d?b=c&e=f", rec.Header().Get("Location")) + } +} + +func TestMetaRedirect(t *testing.T) { + re := Redirect{ + Rules: []Rule{ + {From: "/whatever", Meta: true, To: "/something"}, + {From: "/", Meta: true, To: "https://example.com/"}, + }, + } + + for i, test := range re.Rules { + req, err := http.NewRequest("GET", test.From, nil) + if err != nil { + t.Fatalf("Test %d: Could not create HTTP request: %v", i, err) + } + + rec := httptest.NewRecorder() + re.ServeHTTP(rec, req) + + body, err := ioutil.ReadAll(rec.Body) + if err != nil { + t.Fatalf("Test %d: Could not read HTTP response body: %v", i, err) + } + expectedSnippet := `` + if !bytes.Contains(body, []byte(expectedSnippet)) { + t.Errorf("Test %d: Expected Response Body to contain %q but was %q", + i, expectedSnippet, body) + } + } +}