Handle empty chunks

Empty chunk in request body could prematurely signal end of chunked
transmission. As a result, the terminating zero-size chunk sent by
'requests' can be interpretted as bad request by the recepient. I
have used the same logic used by httplib to handle such cases.
Please review this and comment.


Thanks for this!

The logic doesn't look quite right. Here, if we get an empty section from a generator, we stop sending the chunked body altogether. That doesn't seem like the correct logic to me. We should really do one of two things:
1. Throw an exception. This means that users need to not send us empty chunks. This has the advantage of correctness, in that users are forced to choose what the correct behaviour is for empty chunks. It has the disadvantage of being annoying.
2. Skip the chunk. We cannot emit it, that would be incorrect, so the only way to 'muddle on through' is to skip it. This has the advantage of continuing to work in the majority of cases, and the disadvantage of being a bit surprising in the remainder, where we send something other than what the user provided us.

If we want to do option 2, that means changing `break` for `continue`. @sigmavirus24, thoughts?


Thanks for your comments.

I agree that throwing an exception would be annoying. I noticed this issue when we were uploading a file with size being an exact multiple of our chunk size. We processed the EOF as an empty chunk and noticed broken-pipe errors all over the place in the server logs. We noticed this behavior after switching to requests and immediately fixed our code to stop sending EOF as a chunk. 

I do not think skipping the chunk is right either for the same reason you mentioned. We were earlier using httplib and hence proposed to a similar solution to stop sending altogether.

Let me wait for more comments.


So I had to re-read the [RFC section on Chunked Transfer Coding](http://tools.ietf.org/html/rfc7230#section-4.1) to make sure I remembered it correctly. The ABNF states: 

```
last-chunk     = 1*("0") [ chunk-ext ] CRLF
```

In other words, the chunk that terminates a connect looks like

```
0\r\n
```

What we're essentially sending now is

```
0\r\n\r\n
0\r\n\r\n
```

Which is wrong.

Frankly, I _really_ want this to throw an exception but I know that no one will like it. I think this especially causes problems if people are simply passing data through from a consumer of theirs. I think right now using `break` makes sense until we can come up with a more sensible way of doing this. ~~Also, I think we need to edit L398 to stop sending `0\r\n\r\n` because that doesn't appear correct (based on the RFC).~~ Edit: I just reread the ABNF for the entirety of the chunked body and the second `CRLF` is correct. Ignore me.


I think I want this to throw an exception as well. It's annoying, but it's very Zen of Python (resist the temptation to guess etc. etc.).


So either way we do this, the behaviour is _technically_ backwards incompatible. Right now, someone (trust me, I don't know who but this is oh so very plausible) is relying on us to:
1. Not throw an exception, and
2. Not skip empty chunks

I really want this to be something we address but I don't think we can do so before 3.0. We should document this before 3.0 and perhaps start issuing a warning if we see an empty chunk so people can adjust, etc. before then. That said, I'm marking this as "Do Not Merge", "Planned", and adding the 3.0 milestone to it.


Okay, we currently have 10 pull requests open for this project, many of which are labelled "do not merge". 

Let's get this cleaned up. 


@neosab can you resubmit this against the `proposed/3.0` branch?


@sigmavirus24 Resubmitted this against proposed/3.0.0 branch in https://github.com/kennethreitz/requests/pull/2631


Thanks @neosab! I'm closing this as a duplicate of #2631. Cheers! :sparkles: