Re: [PATCH] wininet: Initialize netconn_stream's content_length

[Jacek Caban]

Hi Andy,

On 11/22/11 5:49 AM, Andy Clayton wrote:
> The size that net_conn_read reads is limited by netconn_stream's
> remaining content_length, so if the content_length is zero it reads
> nothing. It also means that the read size it returns is 0, which
> typically indicates that the socket was closed. As a result of the
> change from 193da884ad9b982d9b740b15453aa8d3c19b054f,
> HTTP_ReceiveRequestData acts on the read size of 0 and closes the
> connection. HttpSendRequest calls HTTP_ReceiveRequestData if there is no
> post data, closing the connection with none of the response data ever
> being read.
>
> This patch fixes this case by initializing netconn_stream's
> content_length to ~0.
>
> Fixes http://bugs.winehq.org/show_bug.cgi?id=27816

This doesn't seem right. At the point where we call 
HTTP_ReceiveRequestData, content_length is supposed to be set based on 
response header. The bug must be somewhere else.

Jacek

[PATCH] wininet: Initialize netconn_stream's content_length

[Andy Clayton]

Hi Jacek,

On Tue, Nov 22, 2011 at 3:59 AM, Jacek Caban  wrote:
> This doesn't seem right. At the point where we call
> HTTP_ReceiveRequestData, content_length is supposed to be set based on
> response header. The bug must be somewhere else.

Thanks for looking over my patch. So it seems then that
HTTP_HttpSendRequestW should not be calling HTTP_ReceiveRequestData if
bEndRequest is false (http.c:4941), since there is no way at that point it
could have read the headers. Is that a better fix?

Thanks again for your help,

Andy

[PATCH] wininet: Initialize netconn_stream's content_length

[Andy Clayton]

On Tue, Nov 22, 2011 at 9:28 AM, Andy Clayton  wrote:
> Hi Jacek,
>
> On Tue, Nov 22, 2011 at 3:59 AM, Jacek Caban wrote:
> > This doesn't seem right. At the point where we call
> > HTTP_ReceiveRequestData, content_length is supposed to be set based on
> > response header. The bug must be somewhere else.
>
> Thanks for looking over my patch. So it seems then that
> HTTP_HttpSendRequestW should not be calling HTTP_ReceiveRequestData if
> bEndRequest is false (http.c:4941), since there is no way at that point it
> could have read the headers. Is that a better fix?
>
> Thanks again for your help,
>
> Andy

I've attached a second attempt showing this. Thanks for any thoughts you
can provide.

Andy

[PATCH] wininet: Initialize netconn_stream's content_length

[Jacek Caban]

On 11/23/11 04:17, Andy Clayton wrote:
> On Tue, Nov 22, 2011 at 9:28 AM, Andy Clayton <[e-mail] 
> > wrote:
> Hi Jacek,
>
> On Tue, Nov 22, 2011 at 3:59 AM, Jacek Caban
> <[e-mail] <mailto:[e-mail]>> wrote:
>
>     This doesn't seem right. At the point where we call
>     HTTP_ReceiveRequestData, content_length is supposed to be set
>     based on response header. The bug must be somewhere else.
>
>
> Thanks for looking over my patch. So it seems then that
> HTTP_HttpSendRequestW should not be calling
> HTTP_ReceiveRequestData if bEndRequest is false (http.c:4941),
> since there is no way at that point it could have read the
> headers. Is that a better fix?
>
> Thanks again for your help,
>
> Andy
>
>
> I've attached a second attempt showing this. Thanks for any thoughts 
> you can provide.

This patch looks good to me.

Thanks,
     Jacek