From 0f329e4cd70a0f5cf0256c221ada0c6834abb469 Mon Sep 17 00:00:00 2001
From: Simon Engmann <simon.engmann@sovity.de>
Date: Wed, 4 Sep 2024 13:51:43 +0200
Subject: [PATCH] Propagate errors during route building

This fixes cases such as invalid paths being silently discarded after
creation by throwing a visible error in such cases.
Due to the way gorilla/mux's fluent API is designed, it is necessary to
manually call `.GetError()` to check for errors while building routes.
---
 pkg/upstream/proxy.go | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/pkg/upstream/proxy.go b/pkg/upstream/proxy.go
index 74b0d02d..dc4f1d5b 100644
--- a/pkg/upstream/proxy.go
+++ b/pkg/upstream/proxy.go
@@ -57,7 +57,9 @@ func NewProxy(upstreams options.UpstreamConfig, sigData *options.SignatureData,
 		}
 	}
 
-	registerTrailingSlashHandler(m.serveMux)
+	if err := registerTrailingSlashHandler(m.serveMux); err != nil {
+		return nil, fmt.Errorf("could not register trailing slash handler: %w", err)
+	}
 	return m, nil
 }
 
@@ -93,8 +95,7 @@ func (m *multiUpstreamProxy) registerHTTPUpstreamProxy(upstream options.Upstream
 // registerHandler ensures the given handler is regiestered with the serveMux.
 func (m *multiUpstreamProxy) registerHandler(upstream options.Upstream, handler http.Handler, writer pagewriter.Writer) error {
 	if upstream.RewriteTarget == "" {
-		m.registerSimpleHandler(upstream.Path, handler)
-		return nil
+		return m.registerSimpleHandler(upstream.Path, handler)
 	}
 
 	return m.registerRewriteHandler(upstream, handler, writer)
@@ -102,12 +103,12 @@ func (m *multiUpstreamProxy) registerHandler(upstream options.Upstream, handler
 
 // registerSimpleHandler maintains the behaviour of the go standard serveMux
 // by ensuring any path with a trailing `/` matches all paths under that prefix.
-func (m *multiUpstreamProxy) registerSimpleHandler(path string, handler http.Handler) {
+func (m *multiUpstreamProxy) registerSimpleHandler(path string, handler http.Handler) error {
 	if strings.HasSuffix(path, "/") {
-		m.serveMux.PathPrefix(path).Handler(handler)
-	} else {
-		m.serveMux.Path(path).Handler(handler)
+		return m.serveMux.PathPrefix(path).Handler(handler).GetError()
 	}
+
+	return m.serveMux.Path(path).Handler(handler).GetError()
 }
 
 // registerRewriteHandler ensures the handler is registered for all paths
@@ -122,19 +123,18 @@ func (m *multiUpstreamProxy) registerRewriteHandler(upstream options.Upstream, h
 
 	rewrite := newRewritePath(rewriteRegExp, upstream.RewriteTarget, writer)
 	h := alice.New(rewrite).Then(handler)
-	m.serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool {
-		return rewriteRegExp.MatchString(req.URL.Path)
-	}).Handler(h)
 
-	return nil
+	return m.serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool {
+		return rewriteRegExp.MatchString(req.URL.Path)
+	}).Handler(h).GetError()
 }
 
 // registerTrailingSlashHandler creates a new matcher that will check if the
 // requested path would match if it had a trailing slash appended.
 // If the path matches with a trailing slash, we send back a redirect.
 // This allows us to be consistent with the built in go servemux implementation.
-func registerTrailingSlashHandler(serveMux *mux.Router) {
-	serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool {
+func registerTrailingSlashHandler(serveMux *mux.Router) error {
+	return serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool {
 		if strings.HasSuffix(req.URL.Path, "/") {
 			return false
 		}
@@ -148,7 +148,7 @@ func registerTrailingSlashHandler(serveMux *mux.Router) {
 		return serveMux.Match(slashReq, m)
 	}).Handler(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		http.Redirect(rw, req, req.URL.String()+"/", http.StatusMovedPermanently)
-	}))
+	})).GetError()
 }
 
 // sortByPathLongest ensures that the upstreams are sorted by longest path.