From e1fcac3e4682b27880b1d0d312cc947a59bd0086 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Sat, 9 Sep 2023 20:12:37 +0700
Subject: [PATCH] set the handshake timeout to twice the handshake idle timeout
 (#4063)

---
 config.go                             | 3 +--
 config_test.go                        | 7 +------
 connection_test.go                    | 4 ++--
 integrationtests/self/timeout_test.go | 2 +-
 interface.go                          | 3 ++-
 internal/protocol/params.go           | 3 ---
 6 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/config.go b/config.go
index 59a4d922..35b7c897 100644
--- a/config.go
+++ b/config.go
@@ -6,7 +6,6 @@ import (
 	"time"
 
 	"github.com/quic-go/quic-go/internal/protocol"
-	"github.com/quic-go/quic-go/internal/utils"
 	"github.com/quic-go/quic-go/quicvarint"
 )
 
@@ -17,7 +16,7 @@ func (c *Config) Clone() *Config {
 }
 
 func (c *Config) handshakeTimeout() time.Duration {
-	return utils.Max(protocol.DefaultHandshakeTimeout, 2*c.HandshakeIdleTimeout)
+	return 2 * c.HandshakeIdleTimeout
 }
 
 func validateConfig(config *Config) error {
diff --git a/config_test.go b/config_test.go
index 7208b4ad..87386122 100644
--- a/config_test.go
+++ b/config_test.go
@@ -115,12 +115,7 @@ var _ = Describe("Config", func() {
 		return c
 	}
 
-	It("uses 10s handshake timeout for short handshake idle timeouts", func() {
-		c := &Config{HandshakeIdleTimeout: time.Second}
-		Expect(c.handshakeTimeout()).To(Equal(protocol.DefaultHandshakeTimeout))
-	})
-
-	It("uses twice the handshake idle timeouts for the handshake timeout, for long handshake idle timeouts", func() {
+	It("uses twice the handshake idle timeouts for the handshake timeout", func() {
 		c := &Config{HandshakeIdleTimeout: time.Second * 11 / 2}
 		Expect(c.handshakeTimeout()).To(Equal(11 * time.Second))
 	})
diff --git a/connection_test.go b/connection_test.go
index 8b7c763f..4c02c492 100644
--- a/connection_test.go
+++ b/connection_test.go
@@ -2201,7 +2201,7 @@ var _ = Describe("Connection", func() {
 
 		It("times out due to non-completed handshake", func() {
 			conn.handshakeComplete = false
-			conn.creationTime = time.Now().Add(-protocol.DefaultHandshakeTimeout).Add(-time.Second)
+			conn.creationTime = time.Now().Add(-2 * protocol.DefaultHandshakeIdleTimeout).Add(-time.Second)
 			connRunner.EXPECT().Remove(gomock.Any()).Times(2)
 			cryptoSetup.EXPECT().Close()
 			gomock.InOrder(
@@ -2261,7 +2261,7 @@ var _ = Describe("Connection", func() {
 		})
 
 		It("closes the connection due to the idle timeout before handshake", func() {
-			conn.config.HandshakeIdleTimeout = 0
+			conn.config.HandshakeIdleTimeout = scaleDuration(25 * time.Millisecond)
 			packer.EXPECT().PackCoalescedPacket(false, gomock.Any(), conn.version).AnyTimes()
 			connRunner.EXPECT().Remove(gomock.Any()).AnyTimes()
 			cryptoSetup.EXPECT().Close()
diff --git a/integrationtests/self/timeout_test.go b/integrationtests/self/timeout_test.go
index c5ec46d4..47569b77 100644
--- a/integrationtests/self/timeout_test.go
+++ b/integrationtests/self/timeout_test.go
@@ -57,7 +57,7 @@ var _ = Describe("Timeout tests", func() {
 				context.Background(),
 				"localhost:12345",
 				getTLSClientConfig(),
-				getQuicConfig(&quic.Config{HandshakeIdleTimeout: 10 * time.Millisecond}),
+				getQuicConfig(&quic.Config{HandshakeIdleTimeout: scaleDuration(50 * time.Millisecond)}),
 			)
 			errChan <- err
 		}()
diff --git a/interface.go b/interface.go
index 5d5ab5b0..fadd868d 100644
--- a/interface.go
+++ b/interface.go
@@ -250,7 +250,8 @@ type Config struct {
 	// If not set, it uses all versions available.
 	Versions []VersionNumber
 	// HandshakeIdleTimeout is the idle timeout before completion of the handshake.
-	// Specifically, if we don't receive any packet from the peer within this time, the connection attempt is aborted.
+	// If we don't receive any packet from the peer within this time, the connection attempt is aborted.
+	// Additionally, if the handshake doesn't complete in twice this time, the connection attempt is also aborted.
 	// If this value is zero, the timeout is set to 5 seconds.
 	HandshakeIdleTimeout time.Duration
 	// MaxIdleTimeout is the maximum duration that may pass without any incoming network activity.
diff --git a/internal/protocol/params.go b/internal/protocol/params.go
index fe3a7562..0f118c49 100644
--- a/internal/protocol/params.go
+++ b/internal/protocol/params.go
@@ -108,9 +108,6 @@ const DefaultIdleTimeout = 30 * time.Second
 // DefaultHandshakeIdleTimeout is the default idle timeout used before handshake completion.
 const DefaultHandshakeIdleTimeout = 5 * time.Second
 
-// DefaultHandshakeTimeout is the default timeout for a connection until the crypto handshake succeeds.
-const DefaultHandshakeTimeout = 10 * time.Second
-
 // MaxKeepAliveInterval is the maximum time until we send a packet to keep a connection alive.
 // It should be shorter than the time that NATs clear their mapping.
 const MaxKeepAliveInterval = 20 * time.Second