From 8a2d0890a20577675f1f309d24846045f361a2c1 Mon Sep 17 00:00:00 2001 From: Maxime Date: Sun, 12 Jul 2015 16:01:32 +0200 Subject: [PATCH 1/3] Changes regarding issue 180 The get parameters are now forwarded when redirected. Added some tests to validate this behavior. --- middleware/redirect/redirect.go | 19 ++++++++++++-- middleware/redirect/redirect_test.go | 37 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/middleware/redirect/redirect.go b/middleware/redirect/redirect.go index f9a1812b3..e65b1078d 100644 --- a/middleware/redirect/redirect.go +++ b/middleware/redirect/redirect.go @@ -6,7 +6,8 @@ import ( "fmt" "html" "net/http" - "strings" + "net/url" + "regexp" "github.com/mholt/caddy/middleware" ) @@ -22,7 +23,21 @@ func (rd Redirect) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error for _, rule := range rd.Rules { if rule.From == "/" { // Catchall redirect preserves path (TODO: Standardize/formalize this behavior) - newPath := strings.TrimSuffix(rule.To, "/") + r.URL.Path + toURL, err := url.ParseRequestURI(rule.To) + if err != nil { + return 500, err + } + newPath := toURL.Host + toURL.Path + r.URL.Path + rmSlashs := regexp.MustCompile("//+") + newPath = rmSlashs.ReplaceAllString(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.Meta { fmt.Fprintf(w, metaRedir, html.EscapeString(newPath)) } else { diff --git a/middleware/redirect/redirect_test.go b/middleware/redirect/redirect_test.go index 662010f17..07c83116a 100644 --- a/middleware/redirect/redirect_test.go +++ b/middleware/redirect/redirect_test.go @@ -39,6 +39,43 @@ func TestMetaRedirect(t *testing.T) { } } +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 From eea68c34adb9217f1c6a536c152671d73ab5f682 Mon Sep 17 00:00:00 2001 From: Maxime Date: Sun, 12 Jul 2015 16:43:35 +0200 Subject: [PATCH 2/3] Changes regarding comment. Used http status code instead of a hardcoded value. Used url.Parse instead of url.ParseRequestURI, so that you can parse both absolute and relative URL. --- middleware/redirect/redirect.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/middleware/redirect/redirect.go b/middleware/redirect/redirect.go index e65b1078d..00ca94833 100644 --- a/middleware/redirect/redirect.go +++ b/middleware/redirect/redirect.go @@ -23,9 +23,9 @@ func (rd Redirect) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error for _, rule := range rd.Rules { if rule.From == "/" { // Catchall redirect preserves path (TODO: Standardize/formalize this behavior) - toURL, err := url.ParseRequestURI(rule.To) + toURL, err := url.Parse(rule.To) if err != nil { - return 500, err + return http.StatusInternalServerError, err } newPath := toURL.Host + toURL.Path + r.URL.Path rmSlashs := regexp.MustCompile("//+") From d9ebc5398ac6b3583eb848590ac94046f01ea4cb Mon Sep 17 00:00:00 2001 From: Maxime Date: Sun, 12 Jul 2015 21:22:15 +0200 Subject: [PATCH 3/3] Changes regarding review Use path.Join and then check if the request had a slash at the end to place it again. --- middleware/redirect/redirect.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/middleware/redirect/redirect.go b/middleware/redirect/redirect.go index 00ca94833..2c5de4103 100644 --- a/middleware/redirect/redirect.go +++ b/middleware/redirect/redirect.go @@ -7,7 +7,8 @@ import ( "html" "net/http" "net/url" - "regexp" + "path" + "strings" "github.com/mholt/caddy/middleware" ) @@ -27,9 +28,10 @@ func (rd Redirect) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error if err != nil { return http.StatusInternalServerError, err } - newPath := toURL.Host + toURL.Path + r.URL.Path - rmSlashs := regexp.MustCompile("//+") - newPath = rmSlashs.ReplaceAllString(newPath, "/") + 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() {