Reviewers: mp+170976_code.launchpad.net,

Message:
Please take a look.

Description:
http/client: Set Content-Length

The net/http.Request object takes a Reader
so you have to set ContentLength as an attribute
or it will not set in in the POST request.

Fixes bug #1124561.

Oddly enough, we always buffer everything anyway,
(probably to handle retries?). I would have thought
for at least BinaryRequest we would have not
touched the io.Reader that we got.

https://code.launchpad.net/~jameinel/goose/transfer-content-length-1124561/+merge/170976

(do not edit description out of merge proposal)


Please review this at https://codereview.appspot.com/10465043/

Affected files:
   A [revision details]
   M http/client.go
   M http/client_test.go


Index: [revision details]
=== added file '[revision details]'
--- [revision details]	2012-01-01 00:00:00 +0000
+++ [revision details]	2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: http/client.go
=== modified file 'http/client.go'
--- http/client.go	2013-04-19 09:19:53 +0000
+++ http/client.go	2013-06-23 07:53:30 +0000
@@ -209,6 +209,7 @@
  				req.Header.Add(header, value)
  			}
  		}
+		req.ContentLength = int64(len(reqData))
  		resp, err = c.Do(req)
  		if err != nil {
  			return nil, errors.Newf(err, "failed executing the request %s", URL)


Index: http/client_test.go
=== modified file 'http/client_test.go'
--- http/client_test.go	2013-02-22 00:36:43 +0000
+++ http/client_test.go	2013-06-23 07:53:30 +0000
@@ -1,6 +1,9 @@
  package http

  import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
  	. "launchpad.net/gocheck"
  	"launchpad.net/goose/testing/httpsuite"
  	"net/http"
@@ -52,21 +55,27 @@
  		http.Header{"Foo": []string{"Bar"}, "Content-Type":  
contentTypes, "Accept": contentTypes, "User-Agent": []string{gooseAgent()}})
  }

-func (s *HTTPClientTestSuite) setupLoopbackRequest() (*http.Header,  
*Client) {
+func (s *HTTPClientTestSuite) setupLoopbackRequest() (*http.Header,  
*string, *Client) {
  	headers := http.Header{}
+	emptyBody := "<no-response-yet>"
+	body := &emptyBody
  	handler := func(resp http.ResponseWriter, req *http.Request) {
  		headers = req.Header
+		bodyBytes, _ := ioutil.ReadAll(req.Body)
+		content := string(bodyBytes)
+		*body = content
+		req.Body.Close()
  		resp.Header().Add("Content-Length", "0")
  		resp.WriteHeader(http.StatusNoContent)
  		resp.Write([]byte{})
  	}
  	s.Mux.HandleFunc("/", handler)
  	client := New()
-	return &headers, client
+	return &headers, body, client
  }

  func (s *HTTPClientTestSuite) TestBinaryRequestSetsUserAgent(c *C) {
-	headers, client := s.setupLoopbackRequest()
+	headers, _, client := s.setupLoopbackRequest()
  	req := &RequestData{ExpectedStatus: []int{http.StatusNoContent}}
  	err := client.BinaryRequest("POST", s.Server.URL, "", req, nil)
  	c.Assert(err, IsNil)
@@ -76,7 +85,7 @@
  }

  func (s *HTTPClientTestSuite) TestJSONRequestSetsUserAgent(c *C) {
-	headers, client := s.setupLoopbackRequest()
+	headers, _, client := s.setupLoopbackRequest()
  	req := &RequestData{ExpectedStatus: []int{http.StatusNoContent}}
  	err := client.JsonRequest("POST", s.Server.URL, "", req, nil)
  	c.Assert(err, IsNil)
@@ -85,8 +94,42 @@
  	c.Check(agent, Equals, gooseAgent())
  }

+func (s *HTTPClientTestSuite) TestBinaryRequestSetsContentLength(c *C) {
+	headers, body, client := s.setupLoopbackRequest()
+	content := "binary\ncontent\n"
+	req := &RequestData{
+		ExpectedStatus: []int{http.StatusNoContent},
+		ReqReader:      bytes.NewBufferString(content),
+		ReqLength:      len(content),
+	}
+	err := client.BinaryRequest("POST", s.Server.URL, "", req, nil)
+	c.Assert(err, IsNil)
+	encoding := headers.Get("Transfer-Encoding")
+	c.Check(encoding, Equals, "")
+	length := headers.Get("Content-Length")
+	c.Assert(body, NotNil)
+	c.Check(length, Equals, fmt.Sprintf("%d", len(content)))
+	c.Check(*body, Equals, content)
+}
+
+func (s *HTTPClientTestSuite) TestJSONRequestSetsContentLength(c *C) {
+	headers, body, client := s.setupLoopbackRequest()
+	reqMap := map[string]string{"key": "value"}
+	req := &RequestData{
+		ExpectedStatus: []int{http.StatusNoContent},
+		ReqValue:       reqMap,
+	}
+	err := client.JsonRequest("POST", s.Server.URL, "", req, nil)
+	c.Assert(err, IsNil)
+	encoding := headers.Get("Transfer-Encoding")
+	c.Check(encoding, Equals, "")
+	length := headers.Get("Content-Length")
+	c.Assert(body, NotNil)
+	c.Check(length, Equals, fmt.Sprintf("%d", len(*body)))
+}
+
  func (s *HTTPClientTestSuite) TestBinaryRequestSetsToken(c *C) {
-	headers, client := s.setupLoopbackRequest()
+	headers, _, client := s.setupLoopbackRequest()
  	req := &RequestData{ExpectedStatus: []int{http.StatusNoContent}}
  	err := client.BinaryRequest("POST", s.Server.URL, "token", req, nil)
  	c.Assert(err, IsNil)
@@ -95,7 +138,7 @@
  }

  func (s *HTTPClientTestSuite) TestJSONRequestSetsToken(c *C) {
-	headers, client := s.setupLoopbackRequest()
+	headers, _, client := s.setupLoopbackRequest()
  	req := &RequestData{ExpectedStatus: []int{http.StatusNoContent}}
  	err := client.JsonRequest("POST", s.Server.URL, "token", req, nil)
  	c.Assert(err, IsNil)