Commit ecde0bfa authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

net/http: fix timeout race in Transport proxy CONNECT

Fixes #36070

Change-Id: I99742aa153202436d802634c9e019a14b9ef9185
Reviewed-on: https://go-review.googlesource.com/c/go/+/210738Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
parent 41348081
...@@ -1568,38 +1568,46 @@ func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (pconn *pers ...@@ -1568,38 +1568,46 @@ func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (pconn *pers
if pa := cm.proxyAuth(); pa != "" { if pa := cm.proxyAuth(); pa != "" {
connectReq.Header.Set("Proxy-Authorization", pa) connectReq.Header.Set("Proxy-Authorization", pa)
} }
didReadResponse := make(chan struct{}) // closed after reading CONNECT response
// If there's no deadline, at least set some (long) timeout here. // If there's no done channel (no deadline or cancellation
// This will make sure we don't block here forever and leak a goroutine // from the caller possible), at least set some (long)
// if the connection stops replying after the TCP connect. // timeout here. This will make sure we don't block forever
// and leak a goroutine if the connection stops replying
// after the TCP connect.
connectCtx := ctx connectCtx := ctx
if _, ok := ctx.Deadline(); !ok { if ctx.Done() == nil {
newCtx, cancel := context.WithTimeout(ctx, 1*time.Minute) newCtx, cancel := context.WithTimeout(ctx, 1*time.Minute)
defer cancel() defer cancel()
connectCtx = newCtx connectCtx = newCtx
} }
didReadResponse := make(chan struct{}) // closed after CONNECT write+read is done or fails
var (
resp *Response
err error // write or read error
)
// Write the CONNECT request & read the response.
go func() { go func() {
select { defer close(didReadResponse)
case <-connectCtx.Done(): err = connectReq.Write(conn)
conn.Close() if err != nil {
case <-didReadResponse: return
} }
// Okay to use and discard buffered reader here, because
// TLS server will not speak until spoken to.
br := bufio.NewReader(conn)
resp, err = ReadResponse(br, connectReq)
}() }()
select {
connectReq.Write(conn) case <-connectCtx.Done():
conn.Close()
// Read response. <-didReadResponse
// Okay to use and discard buffered reader here, because return nil, connectCtx.Err()
// TLS server will not speak until spoken to. case <-didReadResponse:
br := bufio.NewReader(conn) // resp or err now set
resp, err := ReadResponse(br, connectReq) }
close(didReadResponse)
if err != nil { if err != nil {
conn.Close() conn.Close()
if err := connectCtx.Err(); err != nil {
return nil, err
}
return nil, err return nil, err
} }
if resp.StatusCode != 200 { if resp.StatusCode != 200 {
......
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