[Bug 14183] New: HttpSendRequest() should fetch connection parameters instead of HttpOpenRequest(), especially Cookies.

http://bugs.winehq.org/show_bug.cgi?id=14183

Summary: HttpSendRequest() should fetch connection parameters
                    instead of HttpOpenRequest(), especially Cookies.
           Product: Wine
           Version: 1.0-rc2
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P2
         Component: wininet
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=14452)
 --> (http://bugs.winehq.org/attachment.cgi?id=14452)
Debug Logs for relay,+wininet (filtered):

I have an application used to download a game from an online store which set a
cookie using InternetSetCookie() between calls to HttpOpenRequest() and
HttpSendRequest(), but this cookie is not sent on the wire, as the application
except it.

This application works well under MS Windows and under Wine with MS Internet
Explorer 6 installed, but using the wine wininet implementation, the cookie is
not sent.

The exact behavior of wininet functions is not documented on MSDN, but it seems
that parameters (proxy, auth-data, cookies) have to be fetched only when
calling HttpSendRequest(), especially Cookies, not when calling
HttpOpenRequest().

See attached traces.

[Bug 14183] HttpSendRequest() should fetch connection parameters instead of HttpOpenRequest(), especially Cookies.

http://bugs.winehq.org/show_bug.cgi?id=14183

--- Comment #1 from Yann Droneaud   2008-06-29 13:37:08 ---
Created an attachment (id=14455)
 --> (http://bugs.winehq.org/attachment.cgi?id=14455)
Load cookie in HTTP_HttpSendRequest() instead of HTTP_HttpOpenRequest()

Here is a patch which fix my problem, by moving cookie fetch code from
HTTP_HttpOpenRequestW() to HTTP_HttpSendRequestW().

The patch must be reviewed with care, it was only tested with the application
described above.

Note that loading the cookies inside the loop of HTTP_HttpSendRequestW() could
help to use cookies set by the server in a redirect error, see bug #3307 .

[Bug 14183] HttpSendRequest() should fetch connection parameters instead of HttpOpenRequest(), especially Cookies.

http://bugs.winehq.org/show_bug.cgi?id=14183

Hans Leidekker  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from Hans Leidekker   2008-06-29 15:24:52 ---
Thank you for the patch Yann. I think you're right, processing cookies
should be moved to HttpSendRequest and inside the loop as you suggest,
although we'll need tests to confirm that. Why did you move retrieval of
the hostname outside the loop? I think that will break external redirects.

[Bug 14183] HttpSendRequest() should fetch connection parameters instead of HttpOpenRequest(), especially Cookies.

http://bugs.winehq.org/show_bug.cgi?id=14183

--- Comment #3 from Yann Droneaud   2008-06-30 01:25:43 ---
(In reply to comment #2)
> Thank you for the patch Yann. I think you're right, processing cookies
> should be moved to HttpSendRequest and inside the loop as you suggest,
> although we'll need tests to confirm that. Why did you move retrieval of
> the hostname outside the loop? I think that will break external redirects.

Very interesting, I think you've spotted another problem.
In fact, the call to retrieve Host: was in a debug clause:

if (TRACE_ON(wininet))
        {
            LPHTTPHEADERW Host = HTTP_GetHeader(lpwhr,szHost);
            TRACE("Going to url %s %s\n", debugstr_w(Host->lpszValue),
debugstr_w(lpwhr->lpszPath));
        }

To retrieve the cookie, the Host: value is needed unconditionnaly, so I moved
the call to HTTP_GetHeader() earlier. But I didn't thought about redirect here.
I will try to fix that.

[Bug 14183] HttpSendRequest() should fetch connection parameters instead of HttpOpenRequest(), especially Cookies.

http://bugs.winehq.org/show_bug.cgi?id=14183

Yann Droneaud  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #14455|0                           |1
        is obsolete|                            |

--- Comment #4 from Yann Droneaud   2008-07-01 15:50:40 ---
Created an attachment (id=14512)
 --> (http://bugs.winehq.org/attachment.cgi?id=14512)
[PATCH] wininet: move cookie loading from HttpOpenRequest() to
HttpSendRequest()

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

I'm still wondering about HTTP_ADDREQ_FLAG_* versus HTTP_ADDHDR_FLAG_*,
but I think the patch is ready for testing.

This patch also fix the problem spotted in comment #2

Patch generated with git format-patch

[Bug 14183] HttpSendRequest() should fetch connection parameters instead of HttpOpenRequest(), especially Cookies.

http://bugs.winehq.org/show_bug.cgi?id=14183

Hans Leidekker  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #5 from Hans Leidekker   2008-07-26 11:09:57 ---
Cookies are fixed in git. Please open new bug reports for any other issues
you find.

[Bug 14183] HttpSendRequest() should fetch connection parameters instead of HttpOpenRequest(), especially Cookies.

http://bugs.winehq.org/show_bug.cgi?id=14183

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #6 from Alexandre Julliard   2008-08-22 10:47:30 ---
Closing bugs fixed in 1.1.3.