From a42df3d364d70f56d27017ef5a77c74abb420c75 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Thu, 11 Jun 2020 08:19:55 -0700
Subject: [PATCH] Allow idiomatic usage of middlewares in gorilla/mux (#9802)

Historically due to lack of support for middlewares
we ended up writing wrapped handlers for all
middlewares on top of the gorilla/mux, this causes
multiple issues when we want to let's say

- Overload r.Body with some custom implementation
  to track the incoming Reads()
- Add other sort of top level checks to avoid
  DDOSing the server with large incoming HTTP
  bodies.

Since 1.7.x release gorilla/mux provides proper
use of middlewares, which are honored by the muxer
directly. This makes sure that Go can honor its
own internal ServeHTTP(w, r) implementation where
Go net/http can wrap into its own customer readers.

This PR as a side-affect fixes rare issues of client
hangs which were reported in the wild but never really
understood or fixed in our codebase.

Fixes #9759
Fixes #7266
Fixes #6540
Fixes #5455
Fixes #5150

Refer https://github.com/boto/botocore/pull/1328 for
one variation of the same issue in #9759
---
 cmd/gateway-main.go     |  5 ++++-
 cmd/generic-handlers.go | 13 +++++++------
 cmd/routers.go          |  6 +++---
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/cmd/gateway-main.go b/cmd/gateway-main.go
index 067b6269a..23f8417a2 100644
--- a/cmd/gateway-main.go
+++ b/cmd/gateway-main.go
@@ -203,13 +203,16 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 	// Add API router.
 	registerAPIRouter(router, encryptionEnabled, allowSSEKMS)
 
+	// Use all the middlewares
+	router.Use(registerMiddlewares)
+
 	var getCert certs.GetCertificateFunc
 	if globalTLSCerts != nil {
 		getCert = globalTLSCerts.GetCertificate
 	}
 
 	httpServer := xhttp.NewServer([]string{globalCLIContext.Addr},
-		criticalErrorHandler{registerHandlers(router, globalHandlers...)}, getCert)
+		criticalErrorHandler{router}, getCert)
 	httpServer.BaseContext = func(listener net.Listener) context.Context {
 		return GlobalContext
 	}
diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go
index b42e23d05..0b07ac62f 100644
--- a/cmd/generic-handlers.go
+++ b/cmd/generic-handlers.go
@@ -34,14 +34,14 @@ import (
 	"github.com/rs/cors"
 )
 
-// HandlerFunc - useful to chain different middleware http.Handler
-type HandlerFunc func(http.Handler) http.Handler
+// MiddlewareFunc - useful to chain different http.Handler middlewares
+type MiddlewareFunc func(http.Handler) http.Handler
 
-func registerHandlers(h http.Handler, handlerFns ...HandlerFunc) http.Handler {
-	for _, hFn := range handlerFns {
-		h = hFn(h)
+func registerMiddlewares(next http.Handler) http.Handler {
+	for _, handlerFn := range globalHandlers {
+		next = handlerFn(next)
 	}
-	return h
+	return next
 }
 
 // Adds limiting body size middleware
@@ -795,6 +795,7 @@ func (h criticalErrorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	defer func() {
 		if err := recover(); err == logger.ErrCritical { // handle
 			writeErrorResponse(r.Context(), w, errorCodes.ToAPIErr(ErrInternalError), r.URL, guessIsBrowserReq(r))
+			return
 		} else if err != nil {
 			panic(err) // forward other panic calls
 		}
diff --git a/cmd/routers.go b/cmd/routers.go
index 7e4237024..e78e72628 100644
--- a/cmd/routers.go
+++ b/cmd/routers.go
@@ -38,7 +38,7 @@ func registerDistXLRouters(router *mux.Router, endpointZones EndpointZones) {
 }
 
 // List of some generic handlers which are applied for all incoming requests.
-var globalHandlers = []HandlerFunc{
+var globalHandlers = []MiddlewareFunc{
 	// set x-amz-request-id header.
 	addCustomHeaders,
 	// set HTTP security headers such as Content-Security-Policy.
@@ -118,6 +118,6 @@ func configureServerHandler(endpointZones EndpointZones) (http.Handler, error) {
 	router.NotFoundHandler = http.HandlerFunc(httpTraceAll(errorResponseHandler))
 	router.MethodNotAllowedHandler = http.HandlerFunc(httpTraceAll(errorResponseHandler))
 
-	// Register rest of the handlers.
-	return registerHandlers(router, globalHandlers...), nil
+	router.Use(registerMiddlewares)
+	return router, nil
 }