From: Mikio Hara <mikioh.mikioh@gmail.com>
Date: Thu, 9 Feb 2017 21:22:39 +0000 (+0900)
Subject: net/http: fix possible nil pointer dereference in TestOnlyWriteTimeout
X-Git-Tag: go1.9beta1~1043
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=b029e943448c2ddf6faa9007a8ba4ab76238f18b;p=gostls13.git

net/http: fix possible nil pointer dereference in TestOnlyWriteTimeout

TestOnlyWriteTimeout assumes wrongly that:
- the Accept method of trackLastConnListener is called only once
- the shared variable conn never becomes nil
and crashes on some circumstances.

Updates #19032.

Change-Id: I61de22618cd90b84a2b6401afdb6e5d9b3336b12
Reviewed-on: https://go-review.googlesource.com/36735
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
---

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 451b3b6467..a421fb0709 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -606,7 +606,10 @@ func TestHTTP2WriteDeadlineExtendedOnNewRequest(t *testing.T) {
 func TestOnlyWriteTimeout(t *testing.T) {
 	setParallel(t)
 	defer afterTest(t)
-	var conn net.Conn
+	var (
+		mu   sync.RWMutex
+		conn net.Conn
+	)
 	var afterTimeoutErrc = make(chan error, 1)
 	ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, req *Request) {
 		buf := make([]byte, 512<<10)
@@ -615,11 +618,17 @@ func TestOnlyWriteTimeout(t *testing.T) {
 			t.Errorf("handler Write error: %v", err)
 			return
 		}
+		mu.RLock()
+		defer mu.RUnlock()
+		if conn == nil {
+			t.Error("no established connection found")
+			return
+		}
 		conn.SetWriteDeadline(time.Now().Add(-30 * time.Second))
 		_, err = w.Write(buf)
 		afterTimeoutErrc <- err
 	}))
-	ts.Listener = trackLastConnListener{ts.Listener, &conn}
+	ts.Listener = trackLastConnListener{ts.Listener, &mu, &conn}
 	ts.Start()
 	defer ts.Close()
 
@@ -633,6 +642,7 @@ func TestOnlyWriteTimeout(t *testing.T) {
 			return
 		}
 		_, err = io.Copy(ioutil.Discard, res.Body)
+		res.Body.Close()
 		errc <- err
 	}()
 	select {
@@ -651,12 +661,18 @@ func TestOnlyWriteTimeout(t *testing.T) {
 // trackLastConnListener tracks the last net.Conn that was accepted.
 type trackLastConnListener struct {
 	net.Listener
+
+	mu   *sync.RWMutex
 	last *net.Conn // destination
 }
 
 func (l trackLastConnListener) Accept() (c net.Conn, err error) {
 	c, err = l.Listener.Accept()
-	*l.last = c
+	if err == nil {
+		l.mu.Lock()
+		*l.last = c
+		l.mu.Unlock()
+	}
 	return
 }