use readerfrom optimization by forking Go reverseproxy

This PR also updates dependencies, credits and also
does not treat idle connection errors as backend down

.github/workflows/vulncheck.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go-version: [ 1.20.x ]
+        go-version: [ 1.21.1 ]
     steps:
     - name: Check out code into the Go module directory
       uses: actions/checkout@v3

.gitignore

@@ -1,4 +1,4 @@
 *~
 sidekick
 dist/
-*.test
+*.test

go.mod

@@ -10,12 +10,14 @@ require (
 	github.com/minio/cli v1.24.2
 	github.com/minio/dnscache v0.1.1
 	github.com/minio/pkg v1.6.5
+	github.com/minio/pkg/v2 v2.0.0
 	github.com/prometheus/client_golang v1.15.0
 	github.com/rivo/tview v0.0.0-20230406072732-e22ce9588bb4
 	github.com/sirupsen/logrus v1.9.0
 	go.uber.org/atomic v1.10.0
-	golang.org/x/sys v0.7.0
-	golang.org/x/term v0.7.0
+	golang.org/x/net v0.10.0
+	golang.org/x/sys v0.8.0
+	golang.org/x/term v0.8.0
 )
 
 require (
@@ -32,7 +34,7 @@ require (
 	github.com/mattn/go-isatty v0.0.18 // indirect
 	github.com/mattn/go-runewidth v0.0.14 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
-	github.com/minio/madmin-go/v2 v2.0.19 // indirect
+	github.com/minio/madmin-go/v3 v3.0.1 // indirect
 	github.com/minio/md5-simd v1.1.2 // indirect
 	github.com/minio/minio-go/v7 v7.0.52 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
@@ -52,8 +54,7 @@ require (
 	github.com/tklauser/go-sysconf v0.3.11 // indirect
 	github.com/tklauser/numcpus v0.6.0 // indirect
 	github.com/yusufpapurcu/wmi v1.2.2 // indirect
-	golang.org/x/crypto v0.8.0 // indirect
-	golang.org/x/net v0.9.0 // indirect
+	golang.org/x/crypto v0.9.0 // indirect
 	golang.org/x/sync v0.1.0 // indirect
 	golang.org/x/text v0.9.0 // indirect
 	google.golang.org/protobuf v1.30.0 // indirect

go.sum

@@ -51,14 +51,16 @@ github.com/minio/cli v1.24.2 h1:J+fCUh9mhPLjN3Lj/YhklXvxj8mnyE/D6FpFduXJ2jg=
 github.com/minio/cli v1.24.2/go.mod h1:bYxnK0uS629N3Bq+AOZZ+6lwF77Sodk4+UL9vNuXhOY=
 github.com/minio/dnscache v0.1.1 h1:AMYLqomzskpORiUA1ciN9k7bZT1oB3YZN4cEIi88W5o=
 github.com/minio/dnscache v0.1.1/go.mod h1:WCumm6offO4rQ/82oTCSoHnlhTmc81+vXgKpBtSYRbg=
-github.com/minio/madmin-go/v2 v2.0.19 h1:XznxdMVCTyr0A88JrZFhdxWY8KLfJcrs0TTmFiE9cc8=
-github.com/minio/madmin-go/v2 v2.0.19/go.mod h1:8bL1RMNkblIENFSgGYjeHrzUx9PxROb7OqfNuMU9ivE=
+github.com/minio/madmin-go/v3 v3.0.1 h1:+WuNw0q8gYTNHUmV5X1nCox28uYmJkeMT75vh9VKPkA=
+github.com/minio/madmin-go/v3 v3.0.1/go.mod h1:lPrMoc1aeiIWmmrxBthkDqzMPQwC/Lu9ByuyM2wenJk=
 github.com/minio/md5-simd v1.1.2 h1:Gdi1DZK69+ZVMoNHRXJyNcxrMA4dSxoYHZSQbirFg34=
 github.com/minio/md5-simd v1.1.2/go.mod h1:MzdKDxYpY2BT9XQFocsiZf/NKVtR7nkE4RoEpN+20RM=
 github.com/minio/minio-go/v7 v7.0.52 h1:8XhG36F6oKQUDDSuz6dY3rioMzovKjW40W6ANuN0Dps=
 github.com/minio/minio-go/v7 v7.0.52/go.mod h1:IbbodHyjUAguneyucUaahv+VMNs/EOTV9du7A7/Z3HU=
 github.com/minio/pkg v1.6.5 h1:T9cRNcCLJTFFgQGH0Rzr1CtAWLAIchTsbE0lSztCf40=
 github.com/minio/pkg v1.6.5/go.mod h1:0iX1IuJGSCnMvIvrEJauk1GgQSX9JdU6Kh0P3EQRGkI=
+github.com/minio/pkg/v2 v2.0.0 h1:IbbRstuf4rCKuOlTaa/azoXXHME9XMAH6UBhKxRbRqM=
+github.com/minio/pkg/v2 v2.0.0/go.mod h1:6xTAr5M9yobpUroXAAaTrGJ9fhOZIqKYOT0I87u2yZ4=
 github.com/minio/sha256-simd v1.0.0 h1:v1ta+49hkWZyvaKwrQB8elexRqm6Y0aMLjCNsrYxo6g=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=
@@ -124,17 +126,17 @@ go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
-golang.org/x/crypto v0.8.0 h1:pd9TJtTueMTVQXzk8E2XESSMQDj/U7OUu0PqJqPXQjQ=
-golang.org/x/crypto v0.8.0/go.mod h1:mRqEX+O9/h5TFCrQhkgjo2yKi0yYA+9ecGkdQoHrywE=
+golang.org/x/crypto v0.9.0 h1:LF6fAI+IutBocDJ2OT0Q1g8plpYljMZ4+lty+dsqw3g=
+golang.org/x/crypto v0.9.0/go.mod h1:yrmDGqONDYtNj3tH8X9dzUun2m2lzPa9ngI6/RUPGR0=
 golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
 golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
 golang.org/x/net v0.3.0/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE=
-golang.org/x/net v0.9.0 h1:aWJ/m6xSmxWBx+V0XRHTlrYrPG56jKsLdTFmsSsCzOM=
-golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns=
+golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M=
+golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
@@ -156,14 +158,14 @@ golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU=
-golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
+golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.3.0/go.mod h1:q750SLmJuPmVoN1blW3UFBPREJfb1KmY3vwxfr+nFDA=
 golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
-golang.org/x/term v0.7.0 h1:BEvjmm5fURWqcfbSKTdpkDXYBrUS1c0m8agp14W48vQ=
-golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY=
+golang.org/x/term v0.8.0 h1:n5xxQn2i3PC0yLAbjTpNT85q/Kgzcr2gIoX9OrJUols=
+golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=

http-tracer.go

@@ -21,15 +21,14 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"net"
 	"net/http"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/dustin/go-humanize"
 
-	"github.com/minio/pkg/console"
+	"github.com/minio/pkg/v2/console"
 )
 
 // recordRequest - records the first recLen bytes
@@ -87,6 +86,8 @@ type ResponseWriter struct {
 	// Internal recording buffer
 	headers bytes.Buffer
 	body    bytes.Buffer
+
+	readerFrom bool
 	// Indicate if headers are written in the log
 	headersLogged bool
 }
@@ -101,6 +102,40 @@ func NewResponseWriter(w http.ResponseWriter) *ResponseWriter {
 	}
 }
 
+type ttfbReader struct {
+	io.Reader
+	FirstRead time.Time
+}
+
+func (t *ttfbReader) Read(b []byte) (int, error) {
+	n, err := t.Reader.Read(b)
+	if t.FirstRead.IsZero() {
+		t.FirstRead = time.Now().UTC()
+	}
+	return n, err
+}
+
+func (lrw *ResponseWriter) ReadFrom(src io.Reader) (int64, error) {
+	tr := &ttfbReader{Reader: src}
+	n, err := lrw.ResponseWriter.(io.ReaderFrom).ReadFrom(tr)
+	lrw.bytesWritten += int(n)
+	if lrw.TimeToFirstByte == 0 {
+		lrw.TimeToFirstByte = tr.FirstRead.Sub(lrw.StartTime)
+	}
+
+	if !lrw.headersLogged {
+		// We assume the response code to be '200 OK' when WriteHeader() is not called,
+		// that way following Golang HTTP response behavior.
+		lrw.writeHeaders(&lrw.headers, http.StatusOK, lrw.Header())
+		lrw.headersLogged = true
+	}
+	if lrw.StatusCode >= http.StatusBadRequest || lrw.LogBody {
+		// Always logging error responses.
+		io.Copy(&lrw.body, src)
+	}
+	return n, err
+}
+
 func (lrw *ResponseWriter) Write(p []byte) (int, error) {
 	n, err := lrw.ResponseWriter.Write(p)
 	lrw.bytesWritten += n
@@ -178,17 +213,14 @@ func (r *recordRequest) Data() []byte {
 }
 
 func traceHealthCheckReq(req *http.Request, resp *http.Response, reqTime, respTime time.Time, backend *Backend) {
-	ti := InternalTrace(req, resp, reqTime, respTime)
+	ti := InternalTrace(req, resp, reqTime, respTime, backend.endpoint)
 	doTrace(ti, backend)
 }
 
 // InternalTrace returns trace for sidekick http requests
-func InternalTrace(req *http.Request, resp *http.Response, reqTime, respTime time.Time) TraceInfo {
+func InternalTrace(req *http.Request, resp *http.Response, reqTime, respTime time.Time, endpoint string) TraceInfo {
 	t := TraceInfo{}
-	t.NodeName = req.Host
-	if host, _, err := net.SplitHostPort(t.NodeName); err == nil {
-		t.NodeName = host
-	}
+	t.NodeName = endpoint
 	reqHeaders := req.Header.Clone()
 	reqHeaders.Set("Host", req.Host)
 	if len(req.TransferEncoding) == 0 {
@@ -245,10 +277,6 @@ func Trace(f http.HandlerFunc, logBody bool, w http.ResponseWriter, r *http.Requ
 	t := TraceInfo{}
 	reqBodyRecorder = &recordRequest{Reader: r.Body, logBody: logBody, headers: reqHeaders}
 	r.Body = ioutil.NopCloser(reqBodyRecorder)
-	t.NodeName = r.Host
-	if host, _, err := net.SplitHostPort(t.NodeName); err == nil {
-		t.NodeName = host
-	}
 
 	rw := NewResponseWriter(w)
 	rw.LogBody = logBody
@@ -276,7 +304,6 @@ func Trace(f http.HandlerFunc, logBody bool, w http.ResponseWriter, r *http.Requ
 
 	t.ReqInfo = rq
 	t.RespInfo = rs
-
 	t.NodeName = endpoint
 	t.CallStats = traceCallStats{
 		Latency:         rs.Time.Sub(rw.StartTime),
@@ -367,7 +394,7 @@ type shortTraceMsg struct {
 
 func (s shortTraceMsg) String() string {
 	b := &strings.Builder{}
-	fmt.Fprintf(b, " %5s: ", TraceMsgType)
+	fmt.Fprintf(b, " %5s: ", LogMsgType)
 	fmt.Fprintf(b, "%s ", s.Time.Format(timeFormat))
 	statusStr := console.Colorize("RespStatus", fmt.Sprintf("%d %s", s.StatusCode, s.StatusMsg))
 	if s.StatusCode >= http.StatusBadRequest {

main.go

@@ -28,7 +28,6 @@ import (
 	"math/rand"
 	"net"
 	"net/http"
-	"net/http/httputil"
 	"net/http/pprof"
 	"net/url"
 	"os"
@@ -40,14 +39,15 @@ import (
 	"time"
 
 	"github.com/gorilla/mux"
-	"github.com/minio/dnscache"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/term"
 
 	"github.com/minio/cli"
-	"github.com/minio/pkg/console"
-	"github.com/minio/pkg/ellipses"
-	xnet "github.com/minio/pkg/net"
+	"github.com/minio/dnscache"
+	"github.com/minio/pkg/v2/console"
+	"github.com/minio/pkg/v2/ellipses"
+	xnet "github.com/minio/pkg/v2/net"
+	"github.com/minio/sidekick/reverse"
 )
 
 // Use e.g.: go build -ldflags "-X main.version=v1.0.0"
@@ -146,7 +146,7 @@ func (l logMessage) String() string {
 type Backend struct {
 	siteNumber          int
 	endpoint            string
-	proxy               *httputil.ReverseProxy
+	proxy               *reverse.Proxy
 	httpClient          *http.Client
 	up                  int32
 	healthCheckURL      string
@@ -198,11 +198,30 @@ type BackendStats struct {
 // ErrorHandler called by httputil.ReverseProxy for errors.
 // Avoid canceled context error since it means the client disconnected.
 func (b *Backend) ErrorHandler(_ http.ResponseWriter, _ *http.Request, err error) {
-	if err != nil && !errors.Is(err, context.Canceled) {
-		if globalLoggingEnabled {
-			logMsg(logMessage{Endpoint: b.endpoint, Status: "down", Error: err})
+	if err != nil {
+		offline := true
+		for _, nerr := range []error{
+			context.Canceled,
+			io.EOF,
+			io.ErrClosedPipe,
+			io.ErrUnexpectedEOF,
+			errors.New("http: server closed idle connection"),
+		} {
+			if errors.Is(err, nerr) {
+				offline = false
+				break
+			}
+			if err.Error() == nerr.Error() {
+				offline = false
+				break
+			}
+		}
+		if offline {
+			if globalLoggingEnabled {
+				logMsg(logMessage{Endpoint: b.endpoint, Status: "down", Error: err})
+			}
+			b.setOffline()
 		}
-		b.setOffline()
 	}
 }
 
@@ -273,6 +292,14 @@ func (b *Backend) healthCheck() {
 	}
 }
 
+func drainBody(resp *http.Response) {
+	if resp != nil {
+		// Drain the connection.
+		io.Copy(ioutil.Discard, resp.Body)
+		resp.Body.Close()
+	}
+}
+
 func (b *Backend) doHealthCheck() error {
 	// Set up a maximum timeout time for the healtcheck operation
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
@@ -286,11 +313,7 @@ func (b *Backend) doHealthCheck() error {
 	reqTime := time.Now().UTC()
 	resp, err := b.httpClient.Do(req)
 	respTime := time.Now().UTC()
-	if err == nil {
-		// Drain the connection.
-		io.Copy(ioutil.Discard, resp.Body)
-		resp.Body.Close()
-	}
+	drainBody(resp)
 	if err != nil || (err == nil && resp.StatusCode != http.StatusOK) {
 		if globalLoggingEnabled && (!b.Online() || b.Stats.UpSince.IsZero()) {
 			logMsg(logMessage{Endpoint: b.endpoint, Status: "down", Error: err})
@@ -588,28 +611,6 @@ func modifyResponse() func(*http.Response) error {
 	}
 }
 
-type bufPool struct {
-	pool sync.Pool
-}
-
-func (b *bufPool) Put(buf []byte) {
-	b.pool.Put(&buf)
-}
-
-func (b *bufPool) Get() []byte {
-	bufp := b.pool.Get().(*[]byte)
-	return *bufp
-}
-
-func newBufPool(sz int) httputil.BufferPool {
-	return &bufPool{pool: sync.Pool{
-		New: func() interface{} {
-			buf := make([]byte, sz)
-			return &buf
-		},
-	}}
-}
-
 // sortIPs - sort ips based on higher octects.
 // The logic to sort by last octet is implemented to
 // prefer CIDRs with higher octects, this in-turn skips the
@@ -731,7 +732,7 @@ func configureSite(ctx *cli.Context, siteNum int, siteStrs []string, healthCheck
 		// this is only used if r.RemoteAddr is localhost which means that
 		// sidekick endpoint being accessed is 127.0.0.x
 		realIP := getPublicIP()
-		proxy := &httputil.ReverseProxy{
+		proxy := &reverse.Proxy{
 			Director: func(r *http.Request) {
 				r.Header.Add("X-Forwarded-Host", r.Host)
 				host := realIP
@@ -744,7 +745,6 @@ func configureSite(ctx *cli.Context, siteNum int, siteStrs []string, healthCheck
 				r.URL.Host = target.Host
 			},
 			Transport:      transport,
-			BufferPool:     newBufPool(128 << 10),
 			ModifyResponse: modifyResponse(),
 		}
 		stats := BackendStats{MinLatency: 24 * time.Hour, MaxLatency: 0}