Merge pull request #1115 from influxdata/feature/tr-disable-prefixing-no-flusher

Bypass URLPrefixer if http.Flusher is unavailable

CHANGELOG.md

@@ -9,6 +9,7 @@
   1. [#1106](https://github.com/influxdata/chronograf/issues/1106): Fix obscured legends in dashboards
   1. [#1051](https://github.com/influxdata/chronograf/issue/1051): Exit presentation mode when using the browser back button
   1. [#1123](https://github.com/influxdata/chronograf/issue/1123): Widen single column results in data explorer
+  1. [#1115](https://github.com/influxdata/chronograf/pull/1115): Fix Basepath issue where content would fail to render under certain circumstances
 
 ### Features
   1. [#1112](https://github.com/influxdata/chronograf/pull/1112): Add ability to delete a dashboard

server/builders_test.go

@@ -0,0 +1,30 @@
+package server_test
+
+import (
+	"testing"
+
+	"github.com/influxdata/chronograf/server"
+)
+
+func TestLayoutBuilder(t *testing.T) {
+	var l server.LayoutBuilder = &server.MultiLayoutBuilder{}
+	layout, err := l.Build(nil)
+	if err != nil {
+		t.Fatalf("MultiLayoutBuilder can't build a MultiLayoutStore: %v", err)
+	}
+
+	if layout == nil {
+		t.Fatal("LayoutBuilder should have built a layout")
+	}
+}
+
+func TestSourcesStoresBuilder(t *testing.T) {
+	var b server.SourcesBuilder = &server.MultiSourceBuilder{}
+	sources, err := b.Build(nil)
+	if err != nil {
+		t.Fatalf("MultiSourceBuilder can't build a MultiSourcesStore: %v", err)
+	}
+	if sources == nil {
+		t.Fatal("SourcesBuilder should have built a MultiSourceStore")
+	}
+}

server/server_test.go

@@ -1,26 +1,74 @@
-package server
+package server_test
 
-import "testing"
+import (
+	"fmt"
+	"io"
 
-func TestLayoutBuilder(t *testing.T) {
+	"github.com/influxdata/chronograf"
-	var l LayoutBuilder = &MultiLayoutBuilder{}
+)
-	layout, err := l.Build(nil)
-	if err != nil {
-		t.Fatalf("MultiLayoutBuilder can't build a MultiLayoutStore: %v", err)
-	}
 
-	if layout == nil {
+type LogMessage struct {
-		t.Fatal("LayoutBuilder should have built a layout")
+	Level string
-	}
+	Body  string
 }
 
-func TestSourcesStoresBuilder(t *testing.T) {
+// TestLogger is a chronograf.Logger which allows assertions to be made on the
-	var b SourcesBuilder = &MultiSourceBuilder{}
+// contents of its messages.
-	sources, err := b.Build(nil)
+type TestLogger struct {
-	if err != nil {
+	Messages []LogMessage
-		t.Fatalf("MultiSourceBuilder can't build a MultiSourcesStore: %v", err)
+}
+
+func (tl *TestLogger) Debug(args ...interface{}) {
+	tl.Messages = append(tl.Messages, LogMessage{"debug", tl.stringify(args...)})
+}
+
+func (tl *TestLogger) Info(args ...interface{}) {
+	tl.Messages = append(tl.Messages, LogMessage{"info", tl.stringify(args...)})
+}
+
+func (tl *TestLogger) Error(args ...interface{}) {
+	tl.Messages = append(tl.Messages, LogMessage{"error", tl.stringify(args...)})
+}
+
+func (tl *TestLogger) WithField(key string, value interface{}) chronograf.Logger {
+	return tl
+}
+
+func (tl *TestLogger) Writer() *io.PipeWriter {
+	_, write := io.Pipe()
+	return write
+}
+
+// HasMessage will return true if the TestLogger has been called with an exact
+// match of a particular log message at a particular log level
+func (tl *TestLogger) HasMessage(level string, body string) bool {
+	for _, msg := range tl.Messages {
+		if msg.Level == level && msg.Body == body {
+			return true
+		}
 	}
-	if sources == nil {
+	return false
-		t.Fatal("SourcesBuilder should have built a MultiSourceStore")
+}
+
+func (tl *TestLogger) stringify(args ...interface{}) string {
+	out := []byte{}
+	for _, arg := range args[:len(args)-1] {
+		out = append(out, tl.stringifyArg(arg)...)
+		out = append(out, []byte(" ")...)
+	}
+	out = append(out, tl.stringifyArg(args[len(args)-1])...)
+	return string(out)
+}
+
+func (tl *TestLogger) stringifyArg(arg interface{}) []byte {
+	switch a := arg.(type) {
+	case fmt.Stringer:
+		return []byte(a.String())
+	case error:
+		return []byte(a.Error())
+	case string:
+		return []byte(a)
+	default:
+		return []byte("UNKNOWN")
 	}
 }

server/url_prefixer.go

@@ -9,6 +9,10 @@ import (
 	"github.com/influxdata/chronograf"
 )
 
+const (
+	ErrNotFlusher = "Expected http.ResponseWriter to be an http.Flusher, but wasn't"
+)
+
 // URLPrefixer is a wrapper for an http.Handler that will prefix all occurrences of a relative URL with the configured Prefix
 type URLPrefixer struct {
 	Prefix string            // the prefix to be appended after any detected Attrs
@@ -70,21 +74,21 @@ const ChunkSize int = 512
 // stream through the ResponseWriter, and appending the Prefix after any of the
 // Attrs detected in the stream.
 func (up *URLPrefixer) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
+	// extract the flusher for flushing chunks
+	flusher, ok := rw.(http.Flusher)
+
+	if !ok {
+		up.Logger.Info(ErrNotFlusher)
+		up.Next.ServeHTTP(rw, r)
+		return
+	}
+
 	// chunked transfer because we're modifying the response on the fly, so we
 	// won't know the final content-length
 	rw.Header().Set("Connection", "Keep-Alive")
 	rw.Header().Set("Transfer-Encoding", "chunked")
 
 	writtenCount := 0 // number of bytes written to rw
-
-	// extract the flusher for flushing chunks
-	flusher, ok := rw.(http.Flusher)
-	if !ok {
-		msg := "Expected http.ResponseWriter to be an http.Flusher, but wasn't"
-		Error(rw, http.StatusInternalServerError, msg, up.Logger)
-		return
-	}
-
 	nextRead, nextWrite := io.Pipe()
 	go func() {
 		defer nextWrite.Close()

server/url_prefixer_test.go

@@ -106,3 +106,72 @@ func Test_Server_Prefixer_RewritesURLs(t *testing.T) {
 		}
 	}
 }
+
+// clogger is an http.ResponseWriter that is not an http.Flusher. It is used
+// for testing the behavior of handlers that may rely on specific behavior of
+// http.Flusher
+type clogger struct {
+	next http.ResponseWriter
+}
+
+func (c *clogger) Header() http.Header {
+	return c.next.Header()
+}
+
+func (c *clogger) Write(bytes []byte) (int, error) {
+	return c.next.Write(bytes)
+}
+
+func (c *clogger) WriteHeader(code int) {
+	c.next.WriteHeader(code)
+}
+
+func Test_Server_Prefixer_NoPrefixingWithoutFlusther(t *testing.T) {
+	backend := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(rw, "<a href=\"/valley\">Hill Valley Preservation Society</a>")
+	})
+
+	wrapFunc := func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+			clog := &clogger{rw}
+			next.ServeHTTP(clog, r)
+		})
+	}
+
+	tl := &TestLogger{}
+	pfx := &server.URLPrefixer{
+		Prefix: "/hill",
+		Next:   backend,
+		Logger: tl,
+		Attrs: [][]byte{
+			[]byte("href=\""),
+		},
+	}
+
+	ts := httptest.NewServer(wrapFunc(pfx))
+	defer ts.Close()
+
+	res, err := http.Get(ts.URL)
+	if err != nil {
+		t.Fatal("Unexpected error fetching from prefixer: err:", err)
+	}
+
+	actual, err := ioutil.ReadAll(res.Body)
+	if err != nil {
+		t.Fatal("Unable to read prefixed body: err:", err)
+	}
+
+	unexpected := "<a href=\"/hill/valley\">Hill Valley Preservation Society</a>"
+	expected := "<a href=\"/valley\">Hill Valley Preservation Society</a>"
+	if string(actual) == unexpected {
+		t.Error("No Flusher", ":\n Prefixing occurred without an http.Flusher")
+	}
+
+	if string(actual) != expected {
+		t.Error("No Flusher", ":\n\tPrefixing failed to output without an http.Flusher\n\t\tWant:\n", expected, "\n\t\tGot:\n", string(actual))
+	}
+
+	if !tl.HasMessage("info", server.ErrNotFlusher) {
+		t.Error("No Flusher", ":\n Expected Error Message: \"", server.ErrNotFlusher, "\" but saw none. Msgs:", tl.Messages)
+	}
+}