From 123c40028f8afd3c816c6f39747d3896de9bf92f Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Mon, 11 Jan 2016 17:46:00 +0000 Subject: [PATCH] Less hacky fix for /%2F bug --- app/api_report_test.go | 5 +- app/api_topologies.go | 4 +- app/api_topologies_test.go | 4 +- app/api_topology.go | 23 +++++---- app/api_topology_test.go | 14 ++++++ app/router.go | 86 ++++++++++++++++++++++++---------- prog/app.go | 10 ++-- render/expected/expected.go | 26 +++++----- test/fixture/report_fixture.go | 4 +- 9 files changed, 112 insertions(+), 64 deletions(-) diff --git a/app/api_report_test.go b/app/api_report_test.go index d355571611..c42773c546 100644 --- a/app/api_report_test.go +++ b/app/api_report_test.go @@ -12,9 +12,8 @@ import ( ) func topologyServer() *httptest.Server { - router := mux.NewRouter() - app.RegisterTopologyRoutes(StaticReport{}, router) - return httptest.NewServer(router) + handler := app.TopologyHandler(StaticReport{}, mux.NewRouter(), nil) + return httptest.NewServer(handler) } func TestAPIReport(t *testing.T) { diff --git a/app/api_topologies.go b/app/api_topologies.go index d9c1862923..7787fbe73e 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -259,9 +259,9 @@ func (r *registry) captureRenderer(rep Reporter, f reportRenderHandler) http.Han } } -func (r *registry) captureRendererWithoutFilters(rep Reporter, f reportRenderHandler) http.HandlerFunc { +func (r *registry) captureRendererWithoutFilters(rep Reporter, topologyID string, f reportRenderHandler) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { - topology, ok := r.get(mux.Vars(req)["topology"]) + topology, ok := r.get(topologyID) if !ok { http.NotFound(w, req) return diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index 03aa5b1afb..25f6264606 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -53,9 +53,9 @@ func TestAPITopology(t *testing.T) { func TestAPITopologyAddsKubernetes(t *testing.T) { router := mux.NewRouter() c := app.NewCollector(1 * time.Minute) - app.RegisterTopologyRoutes(c, router) app.RegisterReportPostHandler(c, router) - ts := httptest.NewServer(router) + handler := app.TopologyHandler(c, router, nil) + ts := httptest.NewServer(handler) defer ts.Close() body := getRawJSON(t, ts, "/api/topology") diff --git a/app/api_topology.go b/app/api_topology.go index 93911c084c..1e682294dc 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -4,7 +4,6 @@ import ( "net/http" "time" - "github.com/gorilla/mux" "github.com/gorilla/websocket" "github.com/weaveworks/scope/render" @@ -50,18 +49,18 @@ func handleWs(rep Reporter, renderer render.Renderer, w http.ResponseWriter, r * } // Individual nodes. -func handleNode(rep Reporter, renderer render.Renderer, w http.ResponseWriter, r *http.Request) { - var ( - vars = mux.Vars(r) - nodeID = vars["id"] - rpt = rep.Report() - node, ok = renderer.Render(rep.Report())[nodeID] - ) - if !ok { - http.NotFound(w, r) - return +func handleNode(nodeID string) func(Reporter, render.Renderer, http.ResponseWriter, *http.Request) { + return func(rep Reporter, renderer render.Renderer, w http.ResponseWriter, r *http.Request) { + var ( + rpt = rep.Report() + node, ok = renderer.Render(rep.Report())[nodeID] + ) + if !ok { + http.NotFound(w, r) + return + } + respondWith(w, http.StatusOK, APINode{Node: render.MakeDetailedNode(rpt, node)}) } - respondWith(w, http.StatusOK, APINode{Node: render.MakeDetailedNode(rpt, node)}) } var upgrader = websocket.Upgrader{ diff --git a/app/api_topology_test.go b/app/api_topology_test.go index 2a216ce00c..600d01f07c 100644 --- a/app/api_topology_test.go +++ b/app/api_topology_test.go @@ -87,6 +87,20 @@ func TestAPITopologyApplications(t *testing.T) { equals(t, false, node.Node.Pseudo) // Let's not unit-test the specific content of the detail tables } + + { + body := getRawJSON(t, ts, "/api/topology/applications-by-name/"+ + url.QueryEscape(fixture.Client1Name)) + var node app.APINode + if err := json.Unmarshal(body, &node); err != nil { + t.Fatal(err) + } + equals(t, fixture.Client1Name, node.Node.ID) + equals(t, fixture.Client1Name, node.Node.LabelMajor) + equals(t, "2 processes", node.Node.LabelMinor) + equals(t, false, node.Node.Pseudo) + // Let's not unit-test the specific content of the detail tables + } } func TestAPITopologyHosts(t *testing.T) { diff --git a/app/router.go b/app/router.go index 31456ea404..13dcc91559 100644 --- a/app/router.go +++ b/app/router.go @@ -29,48 +29,86 @@ var ( // mistakenly unescapes the Path before parsing it. This breaks %2F (encoded // forward slashes) in the paths. func URLMatcher(pattern string) mux.MatcherFunc { - matchParts := strings.Split(pattern, "/") - return func(r *http.Request, rm *mux.RouteMatch) bool { - path := strings.SplitN(r.RequestURI, "?", 2)[0] - parts := strings.Split(path, "/") - if len(parts) != len(matchParts) { - return false + vars, match := matchURL(r, pattern) + if match { + rm.Vars = vars } + return match + } +} - rm.Vars = map[string]string{} - for i, part := range parts { - unescaped, err := url.QueryUnescape(part) - if err != nil { - return false - } - match := matchParts[i] - if strings.HasPrefix(match, "{") && strings.HasSuffix(match, "}") { - rm.Vars[strings.Trim(match, "{}")] = unescaped - } else if matchParts[i] != unescaped { - return false - } +func matchURL(r *http.Request, pattern string) (map[string]string, bool) { + matchParts := strings.Split(pattern, "/") + path := strings.SplitN(r.RequestURI, "?", 2)[0] + parts := strings.Split(path, "/") + if len(parts) != len(matchParts) { + return nil, false + } + + vars := map[string]string{} + for i, part := range parts { + unescaped, err := url.QueryUnescape(part) + if err != nil { + return nil, false + } + match := matchParts[i] + if strings.HasPrefix(match, "{") && strings.HasSuffix(match, "}") { + vars[strings.Trim(match, "{}")] = unescaped + } else if matchParts[i] != unescaped { + return nil, false } - return true } + return vars, true } func gzipHandler(h http.HandlerFunc) http.HandlerFunc { return handlers.GZIPHandlerFunc(h, nil) } -// RegisterTopologyRoutes registers the various topology routes with a http mux. -func RegisterTopologyRoutes(c Reporter, router *mux.Router) { - get := router.Methods("GET").Subrouter() +// TopologyHandler registers the various topology routes with a http mux. +// +// The returned http.Handler has to be passed directly to http.ListenAndServe, +// and cannot be nested inside another gorrilla.mux. +// +// Routes which should be matched before the topology routes should be added +// to a router and passed in preRoutes. Routes to be matches after topology +// routes should be added to a router and passed to postRoutes. +func TopologyHandler(c Reporter, preRoutes *mux.Router, postRoutes http.Handler) http.Handler { + get := preRoutes.Methods("GET").Subrouter() get.HandleFunc("/api", gzipHandler(apiHandler)) get.HandleFunc("/api/topology", gzipHandler(topologyRegistry.makeTopologyList(c))) get.HandleFunc("/api/topology/{topology}", gzipHandler(topologyRegistry.captureRenderer(c, handleTopology))) get.HandleFunc("/api/topology/{topology}/ws", topologyRegistry.captureRenderer(c, handleWs)) // NB not gzip! - get.MatcherFunc(URLMatcher("/api/topology/{topology}/{id}")).HandlerFunc( - gzipHandler(topologyRegistry.captureRendererWithoutFilters(c, handleNode))) get.HandleFunc("/api/report", gzipHandler(makeRawReportHandler(c))) + + if postRoutes != nil { + preRoutes.PathPrefix("/").Handler(postRoutes) + } + + // We have to handle the node details path manually due to + // various bugs in gorilla.mux and Go URL parsing. See #804. + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + vars, match := matchURL(r, "/api/topology/{topology}/{id}") + if !match { + preRoutes.ServeHTTP(w, r) + return + } + + topologyID := vars["topology"] + nodeID := vars["id"] + if nodeID == "ws" { + preRoutes.ServeHTTP(w, r) + return + } + + handler := gzipHandler(topologyRegistry.captureRendererWithoutFilters( + c, topologyID, handleNode(nodeID), + )) + handler.ServeHTTP(w, r) + }) } // RegisterReportPostHandler registers the handler for report submission diff --git a/prog/app.go b/prog/app.go index 8d8a150904..2251300e14 100644 --- a/prog/app.go +++ b/prog/app.go @@ -18,14 +18,12 @@ import ( ) // Router creates the mux for all the various app components. -func router(c app.Collector) *mux.Router { +func router(c app.Collector) http.Handler { router := mux.NewRouter() - app.RegisterTopologyRoutes(c, router) app.RegisterReportPostHandler(c, router) app.RegisterControlRoutes(router) app.RegisterPipeRoutes(router) - router.Methods("GET").PathPrefix("/").Handler(http.FileServer(FS(false))) - return router + return app.TopologyHandler(c, router, http.FileServer(FS(false))) } // Main runs the app @@ -48,10 +46,10 @@ func appMain() { app.UniqueID = strconv.FormatInt(rand.Int63(), 16) app.Version = version log.Printf("app starting, version %s, ID %s", app.Version, app.UniqueID) - http.Handle("/", router(app.NewCollector(*window))) + handler := router(app.NewCollector(*window)) go func() { log.Printf("listening on %s", *listen) - log.Print(http.ListenAndServe(*listen, nil)) + log.Print(http.ListenAndServe(*listen, handler)) }() common.SignalHandlerLoop() diff --git a/render/expected/expected.go b/render/expected/expected.go index 65f290d64b..a863d657da 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -102,7 +102,7 @@ var ( }, ServerProcessID: { ID: ServerProcessID, - LabelMajor: "apache", + LabelMajor: fixture.ServerName, LabelMinor: fmt.Sprintf("%s (%s)", fixture.ServerHostID, fixture.ServerPID), Rank: fixture.ServerName, Pseudo: false, @@ -137,11 +137,11 @@ var ( }).Prune() RenderedProcessNames = (render.RenderableNodes{ - "curl": { - ID: "curl", - LabelMajor: "curl", + fixture.Client1Name: { + ID: fixture.Client1Name, + LabelMajor: fixture.Client1Name, LabelMinor: "2 processes", - Rank: "curl", + Rank: fixture.Client1Name, Pseudo: false, Origins: report.MakeIDList( fixture.Client54001NodeID, @@ -150,17 +150,17 @@ var ( fixture.ClientProcess2NodeID, fixture.ClientHostNodeID, ), - Node: report.MakeNode().WithAdjacent("apache"), + Node: report.MakeNode().WithAdjacent(fixture.ServerName), EdgeMetadata: report.EdgeMetadata{ EgressPacketCount: newu64(30), EgressByteCount: newu64(300), }, }, - "apache": { - ID: "apache", - LabelMajor: "apache", + fixture.ServerName: { + ID: fixture.ServerName, + LabelMajor: fixture.ServerName, LabelMinor: "1 process", - Rank: "apache", + Rank: fixture.ServerName, Pseudo: false, Origins: report.MakeIDList( fixture.Server80NodeID, @@ -187,9 +187,9 @@ var ( Node: report.MakeNode().WithAdjacent(render.TheInternetID), EdgeMetadata: report.EdgeMetadata{}, }, - unknownPseudoNode1ID: unknownPseudoNode1("apache"), - unknownPseudoNode2ID: unknownPseudoNode2("apache"), - render.TheInternetID: theInternetNode("apache"), + unknownPseudoNode1ID: unknownPseudoNode1(fixture.ServerName), + unknownPseudoNode2ID: unknownPseudoNode2(fixture.ServerName), + render.TheInternetID: theInternetNode(fixture.ServerName), }).Prune() RenderedContainers = (render.RenderableNodes{ diff --git a/test/fixture/report_fixture.go b/test/fixture/report_fixture.go index 9f1d70ddbf..bf4f2eeed9 100644 --- a/test/fixture/report_fixture.go +++ b/test/fixture/report_fixture.go @@ -47,8 +47,8 @@ var ( ServerPID = "215" NonContainerPID = "1234" - Client1Name = "curl" - Client2Name = "curl" + Client1Name = "/usr/bin/curl" + Client2Name = "/usr/bin/curl" ServerName = "apache" NonContainerName = "bash"