commit 7719016ee297dd4960bb66ed265038f2d75b3c56
author Sergey Zagursky <gvozdoder@gmail.com> Thu Nov 14 23:07:01 2019 +0300
committer Ian Lance Taylor <iant@golang.org> Fri Nov 15 00:03:57 2019 +0000
tree 7da35441ae214c17a33bf6a6c03696b754ca2725
parent 4de3c7d30b82deb1824d5caf3d1542d6b0cc990b

net: fix improper Context.Deadline usage in DialContext

The existing implementation is erroneously assume that having no
deadline in context.Context means that time returned from Deadline
method will have IsZero() == true. But technically speaking this is an
invalid assumption. The context.Context interface specification doesn't
specify what time should be returned from Deadline method when there is
no deadline set. It only specifies that second result of Deadline should
be false.

Fixes #35594

Change-Id: Ife00aad77ab3585e469f15017550ac6c0431b140
Reviewed-on: https://go-review.googlesource.com/c/go/+/207297
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

src/net/dial.go

diff --git a/src/net/dial.go b/src/net/dial.go
index 60ab0f2..d8be1c2 100644
--- a/src/net/dial.go
+++ b/src/net/dial.go
@@ -529,20 +529,21 @@
 		default:
 		}
 
-		deadline, _ := ctx.Deadline()
-		partialDeadline, err := partialDeadline(time.Now(), deadline, len(ras)-i)
-		if err != nil {
-			// Ran out of time.
-			if firstErr == nil {
-				firstErr = &OpError{Op: "dial", Net: sd.network, Source: sd.LocalAddr, Addr: ra, Err: err}
-			}
-			break
-		}
 		dialCtx := ctx
-		if partialDeadline.Before(deadline) {
-			var cancel context.CancelFunc
-			dialCtx, cancel = context.WithDeadline(ctx, partialDeadline)
-			defer cancel()
+		if deadline, hasDeadline := ctx.Deadline(); hasDeadline {
+			partialDeadline, err := partialDeadline(time.Now(), deadline, len(ras)-i)
+			if err != nil {
+				// Ran out of time.
+				if firstErr == nil {
+					firstErr = &OpError{Op: "dial", Net: sd.network, Source: sd.LocalAddr, Addr: ra, Err: err}
+				}
+				break
+			}
+			if partialDeadline.Before(deadline) {
+				var cancel context.CancelFunc
+				dialCtx, cancel = context.WithDeadline(ctx, partialDeadline)
+				defer cancel()
+			}
 		}
 
 		c, err := sd.dialSingle(dialCtx, ra)

src/net/dial_test.go

diff --git a/src/net/dial_test.go b/src/net/dial_test.go
index 2eddac8..4312a6d 100644
--- a/src/net/dial_test.go
+++ b/src/net/dial_test.go
@@ -980,3 +980,32 @@
 		testenv.MustHaveExternalNetwork(t)
 	}
 }
+
+type contextWithNonZeroDeadline struct {
+	context.Context
+}
+
+func (contextWithNonZeroDeadline) Deadline() (time.Time, bool) {
+	// Return non-zero time.Time value with false indicating that no deadline is set.
+	return time.Unix(0, 0), false
+}
+
+func TestDialWithNonZeroDeadline(t *testing.T) {
+	ln, err := newLocalListener("tcp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ln.Close()
+	_, port, err := SplitHostPort(ln.Addr().String())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ctx := contextWithNonZeroDeadline{Context: context.Background()}
+	var dialer Dialer
+	c, err := dialer.DialContext(ctx, "tcp", JoinHostPort("", port))
+	if err != nil {
+		t.Fatal(err)
+	}
+	c.Close()
+}

src/net/fd_unix.go

diff --git a/src/net/fd_unix.go b/src/net/fd_unix.go
index a6d6453..da88c74 100644
--- a/src/net/fd_unix.go
+++ b/src/net/fd_unix.go
@@ -96,7 +96,7 @@
 	if err := fd.pfd.Init(fd.net, true); err != nil {
 		return nil, err
 	}
-	if deadline, _ := ctx.Deadline(); !deadline.IsZero() {
+	if deadline, hasDeadline := ctx.Deadline(); hasDeadline {
 		fd.pfd.SetWriteDeadline(deadline)
 		defer fd.pfd.SetWriteDeadline(noDeadline)
 	}