Re: [PATCH] wininet: moved cookie loading from HttpOpenRequest() to HttpSendRequest()

[Hans Leidekker]

On Wednesday 02 July 2008 19:27:43 Yann Droneaud wrote:
> This patch does the following:
>
> - Move cookie retrieval code to HTTP_HttpSendRequest()
> - Don't allocate memory for url if INTERNET_FLAG_NO_COOKIES is enabled
> - Be sure to not overflow lpszCookies:
>   buffer size was nCookieSize + 1 + 8,
>   written size is 8 + (nCookieSize - 1) + 2 + 1
> - Optimized request header generation:
>    sprintfW() replaced by strcpyW()
>    strcatW() replaced by strcpyW()
>    removed call to strlenW() for HTTP_HttpAddRequestHeadersW()
> - Use HTTP_ADDREQ_FLAG_ADD instead of HTTP_ADDHDR_FLAG_ADD
> - Use HTTP_ADDREQ_FLAG_REPLACE to replace the cookie instead of adding

Thanks Yann, good to see you move on to fix more bugs. To be accepted
in git each of those items should be a separate patch though, and the
impact of moving the cookie code is such that it needs a test case.

 -Hans

[PATCH] wininet: moved cookie loading from HttpOpenRequest() to HttpSendRequest()

[Yann Droneaud]

> On Wednesday 02 July 2008 19:27:43 Yann Droneaud wrote:
> > This patch does the following:
> >
> > - Move cookie retrieval code to HTTP_HttpSendRequest()
> > - Don't allocate memory for url if INTERNET_FLAG_NO_COOKIES is enabled
> > - Be sure to not overflow lpszCookies:
> >   buffer size was nCookieSize + 1 + 8,
> >   written size is 8 + (nCookieSize - 1) + 2 + 1
> > - Optimized request header generation:
> >    sprintfW() replaced by strcpyW()
> >    strcatW() replaced by strcpyW()
> >    removed call to strlenW() for HTTP_HttpAddRequestHeadersW()
> > - Use HTTP_ADDREQ_FLAG_ADD instead of HTTP_ADDHDR_FLAG_ADD
> > - Use HTTP_ADDREQ_FLAG_REPLACE to replace the cookie instead of adding
>
> Thanks Yann, good to see you move on to fix more bugs. To be accepted
> in git each of those items should be a separate patch though, and the
> impact of moving the cookie code is such that it needs a test case.

Please, don't ask me to split the patch in more than two patches:
- one for moving the code and using FLAG_REPLACE
- one for cleanup/optimization
Doing more will took me too much time.

And I can't write the test case alone: it requires a HTTP server with some
pages that set cookies, redirect and check the cookies.

Regards.

-- 
Yann Droneaud

[PATCH] wininet: moved cookie loading from HttpOpenRequest() to HttpSendRequest()

[Hans Leidekker]

On Thursday 03 July 2008 12:18:14 Yann Droneaud wrote:
> > Thanks Yann, good to see you move on to fix more bugs. To be accepted
> > in git each of those items should be a separate patch though, and the
> > impact of moving the cookie code is such that it needs a test case.
>
> Please, don't ask me to split the patch in more than two patches:
> - one for moving the code and using FLAG_REPLACE
> - one for cleanup/optimization
> Doing more will took me too much time.

If time is a problem you should probably concentrate on the first issue.
> And I can't write the test case alone: it requires a HTTP server with some
> pages that set cookies, redirect and check the cookies.

There's an embedded server in the test suite already, adding redirection
should be straightforward.

 -Hans