http/client: Set Content-Length

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)

[jameinel, 2013-06-23 09:15:26]

Please take a look.

[gz, 2013-06-26 14:59:43]

LGTM, let's land this so it's in the next juju-core release.

https://codereview.appspot.com/10465043/diff/1/http/client_test.go
File http/client_test.go (right):

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode60
http/client_test.go:60: emptyBody := "<no-response-yet>"
Why not just the empty string here?

[dave_cheney.net, 2013-06-26 23:48:37]

https://codereview.appspot.com/10465043/diff/1/http/client_test.go
File http/client_test.go (right):

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode59
http/client_test.go:59: headers := http.Header{}
var headers http.Header

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode110
http/client_test.go:110: c.Assert(body, NotNil)
this is weird, i cannot see how body will ever not be nil

[dave_cheney.net, 2013-06-26 23:50:05]

> this is weird, i cannot see how body will ever not be nil

Urgh. Should read, I cannot see how body will ever not be not nil (lets not not
have more double negatives)

[jameinel, 2013-06-27 06:12:15]

Please take a look.

https://codereview.appspot.com/10465043/diff/1/http/client_test.go
File http/client_test.go (right):

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode59
http/client_test.go:59: headers := http.Header{}
On 2013/06/26 23:48:37, dfc wrote:
> var headers http.Header

Care to explain why one would be preferred to the other?

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode60
http/client_test.go:60: emptyBody := "<no-response-yet>"
On 2013/06/26 14:59:43, gz wrote:
> Why not just the empty string here?

I was distinguishing between "the body read from remote was empty" and "I didn't
get to the point of reading the content".

I can probably switch back, though.

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode110
http/client_test.go:110: c.Assert(body, NotNil)
On 2013/06/26 23:48:37, dfc wrote:
> this is weird, i cannot see how body will ever not be nil

body is a pointer to a string, and when we get the response from the server, we
point that to the contents of the request.

Why would it be nil?

[jameinel, 2013-06-27 06:17:24]

Please take a look.

[jameinel, 2013-06-27 06:50:35]

Please take a look.

[dimitern, 2013-06-27 12:07:06]

LGTM