Merge lp:~dave-cheney/goose/003-fix-fd-leak into lp:goose

[Dave Cheney]

goose/http: always close response body

https://codereview.appspot.com/8804047/

Unmerged revisions

86. By Dave Cheney on 2013-04-19

always close response body before returning from sendRequest

=== modified file 'http/client.go'
--- http/client.go	2013-04-17 04:38:44 +0000
+++ http/client.go	2013-04-19 08:31:29 +0000
@@ -50,7 +50,7 @@
 	RespValue      interface{}
 	ReqReader      io.Reader
 	ReqLength      int
-	RespReader     io.ReadCloser
+	RespReader     io.Reader
 }
 
 const (
@@ -107,18 +107,11 @@
 		}
 	}
 	headers := createHeaders(reqData.ReqHeaders, contentTypeJSON, token)
-	respBody, err := c.sendRequest(
+	respData, err := c.sendRequest(
 		method, url, bytes.NewReader(body), len(body), headers, reqData.ExpectedStatus, logger)
 	if err != nil {
 		return
 	}
-	defer respBody.Close()
-	respData, err := ioutil.ReadAll(respBody)
-	if err != nil {
-		err = errors.Newf(err, "failed reading the response body")
-		return
-	}
-
 	if len(respData) > 0 {
 		if reqData.RespValue != nil {
 			err = json.Unmarshal(respData, &reqData.RespValue)
@@ -150,7 +143,7 @@
 		return
 	}
 	if reqData.RespReader != nil {
-		reqData.RespReader = respBody
+		reqData.RespReader = bytes.NewReader(respBody)
 	}
 	return
 }
@@ -161,19 +154,19 @@
 // headers: HTTP headers to include with the request.
 // expectedStatus: a slice of allowed response status codes.
 func (c *Client) sendRequest(method, URL string, reqReader io.Reader, length int, headers http.Header,
-	expectedStatus []int, logger *log.Logger) (rc io.ReadCloser, err error) {
+	expectedStatus []int, logger *log.Logger) (body []byte, err error) {
 	reqData := make([]byte, length)
 	if reqReader != nil {
 		nrRead, err := io.ReadFull(reqReader, reqData)
 		if err != nil {
-			err = errors.Newf(err, "failed reading the request data, read %v of %v bytes", nrRead, length)
-			return rc, err
+			return nil, errors.Newf(err, "failed reading the request data, read %v of %v bytes", nrRead, length)
 		}
 	}
 	rawResp, err := c.sendRateLimitedRequest(method, URL, headers, reqData, logger)
 	if err != nil {
 		return
 	}
+	defer rawResp.Body.Close()
 	foundStatus := false
 	if len(expectedStatus) == 0 {
 		expectedStatus = []int{http.StatusOK}
@@ -186,10 +179,13 @@
 	}
 	if !foundStatus && len(expectedStatus) > 0 {
 		err = handleError(URL, rawResp)
-		rawResp.Body.Close()
 		return
 	}
-	return rawResp.Body, err
+	body, err = ioutil.ReadAll(rawResp.Body)
+	if err != nil {
+		err = errors.Newf(err, "failed reading the response body")
+	}
+	return body, err
 }
 
 func (c *Client) sendRateLimitedRequest(method, URL string, headers http.Header, reqData []byte,

=== modified file 'swift/live_test.go'
--- swift/live_test.go	2013-02-20 02:30:14 +0000
+++ swift/live_test.go	2013-04-19 08:31:29 +0000
@@ -90,7 +90,6 @@
 	c.Check(err, IsNil)
 	readData, err := ioutil.ReadAll(r)
 	c.Check(err, IsNil)
-	r.Close()
 	c.Check(string(readData), Equals, data)
 	err = s.swift.DeleteObject(s.containerName, object)
 	c.Assert(err, IsNil)
@@ -182,7 +181,6 @@
 	c.Check(err, IsNil)
 	readData, err := ioutil.ReadAll(r)
 	c.Check(err, IsNil)
-	r.Close()
 	c.Check(string(readData), Equals, data)
 	err = s.swift.DeleteObject(s.containerName, object)
 	c.Assert(err, IsNil)

=== modified file 'swift/swift.go'
--- swift/swift.go	2013-02-11 05:01:34 +0000
+++ swift/swift.go	2013-04-19 08:31:29 +0000
@@ -80,7 +80,6 @@
 	if err != nil {
 		return
 	}
-	defer rc.Close()
 	return ioutil.ReadAll(rc)
 }
 
@@ -93,7 +92,7 @@
 }
 
 // GetObject retrieves the specified object's data.
-func (c *Client) GetReader(containerName, objectName string) (rc io.ReadCloser, err error) {
+func (c *Client) GetReader(containerName, objectName string) (rc io.Reader, err error) {
 	requestData := goosehttp.RequestData{RespReader: &emptyReadCloser}
 	err = c.touchObject(&requestData, client.GET, containerName, objectName)
 	return requestData.RespReader, err