auth and data set results in invalid http header

https://stackoverflow.com/questions/36216274/uploading-a-0-bytes-file-to-owncloud-with-python-requests-hangs/36217781#36217781

When auth is set and a file object is passed to data, the header contains a content-length AND a Transfer-Encoding: chunked is set in the header and additionally the last chunk is not send.


Yup, this is a real bug. `prepare_auth` unconditionally calls `prepare_content_length`, which does the wrong thing for situations where it doesn't know the content-length.


Working out exactly how best to fix this is tricky: `prepare_content_length` does weirdly quite a lot, and it's not entirely apparent what parts of its logic are actually mandatory.


Ok, I think the best way to do this is to have `prepare_content_length` short-circuit out if there's a `Transfer-Encoding` header present in the headers already: if we already concluded we want to send via chunked transfer encoding, we probably shouldn't change our minds about that.


In fact, we maybe want to go higher level than that: perhaps `prepare_auth` should choose not to delegate to `prepare_content_length` when `Transfer-Encoding` is present in the headers.


@Lukasa has anyone started work on this? I have some free time today, I was thinking I'd take a look at it. If I come up with something I can put together a PR.


@davidsoncasey I don't believe anyone has taken this up yet, no.


@Lukasa great, I'm working now on writing a test to replicate the issue. Once I've got that, I may ask you to take a look to verify that I'm correctly capturing the issue, and then I'll proceed with a fix.


@Lukasa I just made PR #3181, with the solution to not call `prepare_content_length` if the `Transfer-Encoding` header is present. I went back and forth on whether to do the check there or to short-circuit out of `prepare_content_length` and I'm still not sure which I like better, so if you take a look and think it would be better to do within `prepare_content_length` then by all means.

I was thinking it may be more clear to refactor `prepare_body` slightly to not set the `Content-Length`  header at all in `prepare_body` (line 442 of models.py), and to call `prepare_content_length` regardless of if the data is a stream. I think then it could be a bit more explicit that the two headers should never coexist in the same request. Let me know if you have thoughts.


I had a brief chat about this issue with @Lukasa this morning, and wanted to update the status. Due to some recent improvements to `super_len` and `prepare_content_length`, it seems we've inadvertently solved @julian-r's original issue. You'll find the code below (a minor variant of the Stackoverflow question) now works.

``` python
f = io.BytesIO(b'')
resp = requests.put('http://httpbin.org/put', auth=('asdf', 'fdsa'), data=f)
assert 'Transfer-Encoding' in resp.request.headers
assert 'Content-Length' not in resp.request.headers
```

~~Note I use the word _works_ loosely. Due to an oversight I made, Requests is currently setting all streamable bodies with a length of 0 to 'Transfer-Encoding: chunked'. This doesn't cause any severely negative behaviour but is likely better if we used 'Content-Length' instead. We can either integrate that into the work in #3338, or I'll open a separate issue/PR for it.~~

---

@davidsoncasey, you've done a lot of great work in #3184 and #3338 that would be beneficial for Requests. Would you be willing to reframe a few parts of it around these recent changes?
1. I think it would be helpful to move your three non-exception tests from #3338 into the master branch. This would show that the functionality is currently working and will prevent us from regressing it moving forward.
2. We can then integrate your simplification of `prepare_body` and `prepare_content_length`, along with your exceptions, with the current changes on master. That would be an excellent addition in 3.0.0.

How does that sound?


The move of streamable bodies of length 0 to `Transfer-Encoding: chunked` is actually somewhat deliberate: it clarifies the code a great deal, and also handles edge cases where we accidentally decide that a file object has length zero even though it does not.


Yep, I realized what a rats nest that was going to be when I started digging yesterday. I guess I should have paid more attention to jseabold's work before extending it. Disregard my comment on that above.


This should be resolved with #3754.