Commit b029e943 authored by Mikio Hara's avatar Mikio Hara Committed by Brad Fitzpatrick

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: default avatarBrad Fitzpatrick <bradfitz@golang.org>
parent 0b4e8d00
...@@ -606,7 +606,10 @@ func TestHTTP2WriteDeadlineExtendedOnNewRequest(t *testing.T) { ...@@ -606,7 +606,10 @@ func TestHTTP2WriteDeadlineExtendedOnNewRequest(t *testing.T) {
func TestOnlyWriteTimeout(t *testing.T) { func TestOnlyWriteTimeout(t *testing.T) {
setParallel(t) setParallel(t)
defer afterTest(t) defer afterTest(t)
var conn net.Conn var (
mu sync.RWMutex
conn net.Conn
)
var afterTimeoutErrc = make(chan error, 1) var afterTimeoutErrc = make(chan error, 1)
ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, req *Request) { ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, req *Request) {
buf := make([]byte, 512<<10) buf := make([]byte, 512<<10)
...@@ -615,11 +618,17 @@ func TestOnlyWriteTimeout(t *testing.T) { ...@@ -615,11 +618,17 @@ func TestOnlyWriteTimeout(t *testing.T) {
t.Errorf("handler Write error: %v", err) t.Errorf("handler Write error: %v", err)
return return
} }
mu.RLock()
defer mu.RUnlock()
if conn == nil {
t.Error("no established connection found")
return
}
conn.SetWriteDeadline(time.Now().Add(-30 * time.Second)) conn.SetWriteDeadline(time.Now().Add(-30 * time.Second))
_, err = w.Write(buf) _, err = w.Write(buf)
afterTimeoutErrc <- err afterTimeoutErrc <- err
})) }))
ts.Listener = trackLastConnListener{ts.Listener, &conn} ts.Listener = trackLastConnListener{ts.Listener, &mu, &conn}
ts.Start() ts.Start()
defer ts.Close() defer ts.Close()
...@@ -633,6 +642,7 @@ func TestOnlyWriteTimeout(t *testing.T) { ...@@ -633,6 +642,7 @@ func TestOnlyWriteTimeout(t *testing.T) {
return return
} }
_, err = io.Copy(ioutil.Discard, res.Body) _, err = io.Copy(ioutil.Discard, res.Body)
res.Body.Close()
errc <- err errc <- err
}() }()
select { select {
...@@ -651,12 +661,18 @@ func TestOnlyWriteTimeout(t *testing.T) { ...@@ -651,12 +661,18 @@ func TestOnlyWriteTimeout(t *testing.T) {
// trackLastConnListener tracks the last net.Conn that was accepted. // trackLastConnListener tracks the last net.Conn that was accepted.
type trackLastConnListener struct { type trackLastConnListener struct {
net.Listener net.Listener
mu *sync.RWMutex
last *net.Conn // destination last *net.Conn // destination
} }
func (l trackLastConnListener) Accept() (c net.Conn, err error) { func (l trackLastConnListener) Accept() (c net.Conn, err error) {
c, err = l.Listener.Accept() c, err = l.Listener.Accept()
*l.last = c if err == nil {
l.mu.Lock()
*l.last = c
l.mu.Unlock()
}
return return
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment