From b7bb23720f64c9f4256622d5a6927c6f7f6cb25c Mon Sep 17 00:00:00 2001 From: Tim Raymond Date: Tue, 9 May 2017 12:05:10 -0700 Subject: [PATCH 1/2] Fix infinite spinner with /chronograf basepath When using a basepath of /chronograf, the app would present a never-ending spinner when visiting the root route. This was because the prefixingRedirector middleware which is responsible for appending the basepath to redirects from downstream http.Handlers thought that the prefix was already appended since it saw `/chronograf/v1`. In reality, it should have produced a location like `/chronograf/chronograf/v1`. The solution was to look beyond the first instance of a prefix and check for the presence of another prefix to detect if a prefix was already applied by a downstream handler. --- server/prefixing_redirector.go | 3 +- server/prefixing_redirector_test.go | 87 +++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 server/prefixing_redirector_test.go diff --git a/server/prefixing_redirector.go b/server/prefixing_redirector.go index eec4e9fd7..86f957efa 100644 --- a/server/prefixing_redirector.go +++ b/server/prefixing_redirector.go @@ -16,7 +16,8 @@ func (i *interceptingResponseWriter) WriteHeader(status int) { if status >= 300 && status < 400 { location := i.ResponseWriter.Header().Get("Location") if u, err := url.Parse(location); err == nil && !u.IsAbs() { - if !strings.HasPrefix(location, i.Prefix) { + hasPrefix := strings.HasPrefix(u.Path, i.Prefix) + if !hasPrefix || (hasPrefix && !strings.HasPrefix(u.Path[len(i.Prefix):], i.Prefix)) { i.ResponseWriter.Header().Set("Location", path.Join(i.Prefix, location)+"/") } } diff --git a/server/prefixing_redirector_test.go b/server/prefixing_redirector_test.go new file mode 100644 index 000000000..728c90604 --- /dev/null +++ b/server/prefixing_redirector_test.go @@ -0,0 +1,87 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +var prefixingRedirectTests = []struct { + CaseName string + RedirectTarget string + Prefix string + Expected string +}{ + { + "ChronografBasepath", + "/chronograf/v1/", + "/chronograf", + "/chronograf/chronograf/v1/", + }, + { + "DifferentBasepath", + "/chronograf/v1/", + "/delorean", + "/delorean/chronograf/v1/", + }, + { + "TrailingSlashPrefix", + "/chronograf/v1/", + "/delorean/", + "/delorean/chronograf/v1/", + }, + { + "NoPrefix", + "/chronograf/v1/", + "", + "/chronograf/v1/", + }, + { + "SlashPrefix", + "/chronograf/v1/", + "/", + "/chronograf/v1/", + }, + { + "AlreadyPrefixed", + "/chronograf/chronograf/v1/", + "/chronograf", + "/chronograf/chronograf/v1/", + }, +} + +func Test_PrefixingRedirector(t *testing.T) { + t.Parallel() + for _, p := range prefixingRedirectTests { + t.Run(p.CaseName, func(subt *testing.T) { + hf := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Location", p.RedirectTarget) + rw.WriteHeader(http.StatusTemporaryRedirect) + }) + pr := PrefixedRedirect(p.Prefix, hf) + + ts := httptest.NewServer(pr) + defer ts.Close() + + hc := http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + + mockBody := strings.NewReader("") + req, _ := http.NewRequest("GET", ts.URL, mockBody) + + resp, err := hc.Do(req) + if err != nil { + subt.Fatal("Unexpected http err:", err) + } + + expected := p.Expected + if loc := resp.Header.Get("Location"); loc != expected { + subt.Fatal("Unexpected redirected location. Expected:", expected, "Actual:", loc) + } + }) + } +} From 8cae369cb629bbe59ce29ae6769754975d67691c Mon Sep 17 00:00:00 2001 From: Tim Raymond Date: Tue, 9 May 2017 12:24:43 -0700 Subject: [PATCH 2/2] Update CHANGELOG for infinite spinner basepath fix --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aec821865..4bcc1af78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +## v1.3.1 [unreleased] + +### Bug Fixes + 1. [#1450](https://github.com/influxdata/chronograf/pull/1450): Fix infinite spinner when using "/chronograf" as a basepath + +### Features + +### UI Improvements + ## v1.3.0 [2017-05-09] ### Bug Fixes