fix: don't set read/write timeout if timeout middleware disabled (#4895)

Signed-off-by: kevin <wanjunfeng@gmail.com>

Author: kevwan

rest/engine.go

@@ -228,6 +228,10 @@ func (ng *engine) getShedder(priority bool) load.Shedder {
 	return ng.shedder
 }
 
+func (ng *engine) hasTimeout() bool {
+	return ng.conf.Middlewares.Timeout && ng.timeout > 0
+}
+
 // notFoundHandler returns a middleware that handles 404 not found requests.
 func (ng *engine) notFoundHandler(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -354,16 +358,17 @@ func (ng *engine) use(middleware Middleware) {
 
 func (ng *engine) withTimeout() internal.StartOption {
 	return func(svr *http.Server) {
-		timeout := ng.timeout
-		if timeout > 0 {
-			// factor 0.8, to avoid clients send longer content-length than the actual content,
-			// without this timeout setting, the server will time out and respond 503 Service Unavailable,
-			// which triggers the circuit breaker.
-			svr.ReadTimeout = 4 * timeout / 5
-			// factor 1.1, to avoid servers don't have enough time to write responses.
-			// setting the factor less than 1.0 may lead clients not receiving the responses.
-			svr.WriteTimeout = 11 * timeout / 10
+		if !ng.hasTimeout() {
+			return
 		}
+
+		// factor 0.8, to avoid clients send longer content-length than the actual content,
+		// without this timeout setting, the server will time out and respond 503 Service Unavailable,
+		// which triggers the circuit breaker.
+		svr.ReadTimeout = 4 * ng.timeout / 5
+		// factor 1.1, to avoid servers don't have enough time to write responses.
+		// setting the factor less than 1.0 may lead clients not receiving the responses.
+		svr.WriteTimeout = 11 * ng.timeout / 10
 	}
 }
 

rest/engine_test.go

@@ -394,7 +394,12 @@ func TestEngine_withTimeout(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			ng := newEngine(RestConf{Timeout: test.timeout})
+			ng := newEngine(RestConf{
+				Timeout: test.timeout,
+				Middlewares: MiddlewaresConf{
+					Timeout: true,
+				},
+			})
 			svr := &http.Server{}
 			ng.withTimeout()(svr)
 
@@ -406,6 +411,62 @@ func TestEngine_withTimeout(t *testing.T) {
 	}
 }
 
+func TestEngine_ReadWriteTimeout(t *testing.T) {
+	logx.Disable()
+
+	tests := []struct {
+		name       string
+		timeout    int64
+		middleware bool
+	}{
+		{
+			name:       "0/false",
+			timeout:    0,
+			middleware: false,
+		},
+		{
+			name:       "0/true",
+			timeout:    0,
+			middleware: true,
+		},
+		{
+			name:       "set/false",
+			timeout:    1000,
+			middleware: false,
+		},
+		{
+			name:       "both set",
+			timeout:    1000,
+			middleware: true,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			ng := newEngine(RestConf{
+				Timeout: test.timeout,
+				Middlewares: MiddlewaresConf{
+					Timeout: test.middleware,
+				},
+			})
+			svr := &http.Server{}
+			ng.withTimeout()(svr)
+
+			assert.Equal(t, time.Duration(0), svr.ReadHeaderTimeout)
+			assert.Equal(t, time.Duration(0), svr.IdleTimeout)
+
+			if test.timeout > 0 && test.middleware {
+				assert.Equal(t, time.Duration(test.timeout)*time.Millisecond*4/5, svr.ReadTimeout)
+				assert.Equal(t, time.Duration(test.timeout)*time.Millisecond*11/10, svr.WriteTimeout)
+			} else {
+				assert.Equal(t, time.Duration(0), svr.ReadTimeout)
+				assert.Equal(t, time.Duration(0), svr.WriteTimeout)
+			}
+		})
+	}
+}
+
 func TestEngine_start(t *testing.T) {
 	logx.Disable()