wininet: Don't start the next chunk if the read is satisfied.

[Hans Leidekker]

On Wed, 2013-09-04 at 13:11 +0200, Jacek Caban wrote:
> On 09/04/13 09:22, Hans Leidekker wrote:
> > ---
> >  dlls/wininet/http.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
> > index 1832b4f..521e4aa 100644
> > --- a/dlls/wininet/http.c
> > +++ b/dlls/wininet/http.c
> > @@ -2795,7 +2795,7 @@ static DWORD chunked_read(data_stream_t *stream, http_request_t *req, BYTE *buf,
> >      chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
> >      DWORD read_bytes = 0, ret_read = 0, res = ERROR_SUCCESS;
> >  
> > -    if(chunked_stream->chunk_size == ~0u) {
> > +    if(!chunked_stream->chunk_size || chunked_stream->chunk_size == ~0u) {
> >          res = start_next_chunk(chunked_stream, req);
> >          if(res != ERROR_SUCCESS)
> >              return res;
> > @@ -2835,7 +2835,7 @@ static DWORD chunked_read(data_stream_t *stream, http_request_t *req, BYTE *buf,
> >          chunked_stream->chunk_size -= read_bytes;
> >          size -= read_bytes;
> >          ret_read += read_bytes;
> > -        if(!chunked_stream->chunk_size) {
> > +        if(size && !chunked_stream->chunk_size) {
> >              assert(read_mode != READMODE_NOBLOCK);
> >              res = start_next_chunk(chunked_stream, req);
> >              if(res != ERROR_SUCCESS)
> 
> This patch breaks two assumptions:
> - chunk_size outside chunked_read means the end of the stream

That's strange, considering that there could always be another chunk. We can't
know if we're at the end of the stream in chunked mode, so I wonder if we should
always try the next chunk (and block) if we're being asked to read more than the
current chunk gives us.

The bug I'm seeing is when the current chunk is exactly the size of the read
request (or has that many bytes left). In that case the current code still tries
to start the next chunk and potentially blocks.

> - READMODE_SYNC should read as much data as possible

But no more than asked for? I don't see how my patch breaks this assumption.