Merge 765028a338 into 9168731c7a

CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ## Changes since v7.12.0
 
+- [#2767](https://github.com/oauth2-proxy/oauth2-proxy/pull/2767) Propagate errors during route building (@sybereal)
+
 # V7.12.0
 
 ## Release Highlights

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 m.registerSimpleHandler(upstream.Path, handler)
-		return nil
 	}
 
 	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)
+		return m.serveMux.PathPrefix(path).Handler(handler).GetError()
-	} else {
-		m.serveMux.Path(path).Handler(handler)
 	}
+
+	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) {
+func registerTrailingSlashHandler(serveMux *mux.Router) error {
-	serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool {
+	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.

pkg/upstream/proxy_test.go

@@ -382,6 +382,38 @@ var _ = Describe("Proxy Suite", func() {
 		)
 	})
 
+	Context("multiUpstreamProxy errors", func() {
+		type proxyErrorTableInput struct {
+			upstreams     options.UpstreamConfig
+			expectedError string
+		}
+
+		DescribeTable("NewProxy", func(in *proxyErrorTableInput) {
+			sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"}
+
+			writer := &pagewriter.WriterFuncs{
+				ProxyErrorFunc: func(rw http.ResponseWriter, _ *http.Request, _ error) {
+					rw.WriteHeader(502)
+					rw.Write([]byte("Proxy Error"))
+				},
+			}
+
+			_, err := NewProxy(in.upstreams, sigData, writer)
+			Expect(err).To(MatchError(in.expectedError))
+		},
+			Entry("regex matcher without rewrite target", &proxyErrorTableInput{
+				upstreams: options.UpstreamConfig{
+					Upstreams: []options.Upstream{{
+						ID:   "api",
+						Path: "^/api/$",
+						URI:  "http://example.com",
+					}},
+				},
+				expectedError: `could not register http upstream "api": mux: path must start with a slash, got "^/api/$"`,
+			}),
+		)
+	})
+
 	Context("sortByPathLongest", func() {
 		type sortByPathLongestTableInput struct {
 			input          []options.Upstream