diff --git a/CHANGELOG.md b/CHANGELOG.md index f2c9b441..703cb5eb 100644 --- a/CHANGELOG.md +++ b/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 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. diff --git a/pkg/upstream/proxy_test.go b/pkg/upstream/proxy_test.go index 5252bd42..0c35d67c 100644 --- a/pkg/upstream/proxy_test.go +++ b/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