Cookie Rewrite
Robert Shearman
rob at codeweavers.com
Wed Jul 21 06:40:58 CDT 2004
Hi,
The previous cookie code had some bugs, including corruption of list
when removing a cookie and a memory leak. This patch rewrites the code
to use the shared list.h list code, which is well-tested.
This includes Mike's tracking code disable patch.
Rob
Changelog:
Rewrite cookies to use shared list.h list code.
-------------- next part --------------
Index: wine/dlls/wininet/cookie.c
===================================================================
RCS file: /home/wine/wine/dlls/wininet/cookie.c,v
retrieving revision 1.8
diff -u -r1.8 cookie.c
--- wine/dlls/wininet/cookie.c 19 Jul 2004 19:32:36 -0000 1.8
+++ wine/dlls/wininet/cookie.c 21 Jul 2004 11:31:20 -0000
@@ -38,6 +38,8 @@
#include "wine/debug.h"
#include "internet.h"
+#include "wine/list.h"
+
#define RESPONSE_TIMEOUT 30 /* FROM internet.c */
@@ -55,8 +57,7 @@
struct _cookie
{
- struct _cookie *next;
- struct _cookie *prev;
+ struct list entry;
struct _cookie_domain *parent;
@@ -67,25 +68,19 @@
struct _cookie_domain
{
- struct _cookie_domain *next;
- struct _cookie_domain *prev;
+ struct list entry;
LPWSTR lpCookieDomain;
LPWSTR lpCookiePath;
- cookie *cookie_tail;
+ struct list cookie_list;
};
-static cookie_domain *cookieDomainTail;
+static struct list domain_list = LIST_INIT(domain_list);
static cookie *COOKIE_addCookie(cookie_domain *domain, LPCWSTR name, LPCWSTR data);
static cookie *COOKIE_findCookie(cookie_domain *domain, LPCWSTR lpszCookieName);
static void COOKIE_deleteCookie(cookie *deadCookie, BOOL deleteDomain);
static cookie_domain *COOKIE_addDomain(LPCWSTR domain, LPCWSTR path);
-static cookie_domain *COOKIE_addDomainFromUrl(LPCWSTR lpszUrl);
-static cookie_domain *COOKIE_findNextDomain(LPCWSTR lpszCookieDomain, LPCWSTR lpszCookiePath,
- cookie_domain *prev_domain, BOOL allow_partial);
-static cookie_domain *COOKIE_findNextDomainFromUrl(LPCWSTR lpszUrl, cookie_domain *prev_domain,
- BOOL allow_partial);
static void COOKIE_deleteDomain(cookie_domain *deadDomain);
@@ -94,8 +89,7 @@
{
cookie *newCookie = HeapAlloc(GetProcessHeap(), 0, sizeof(cookie));
- newCookie->next = NULL;
- newCookie->prev = NULL;
+ list_init(&newCookie->entry);
newCookie->lpCookieName = NULL;
newCookie->lpCookieData = NULL;
@@ -112,9 +106,8 @@
TRACE("added cookie %p (data is %s)\n", newCookie, debugstr_w(data) );
- newCookie->prev = domain->cookie_tail;
+ list_add_tail(&domain->cookie_list, &newCookie->entry);
newCookie->parent = domain;
- domain->cookie_tail = newCookie;
return newCookie;
}
@@ -122,11 +115,12 @@
/* finds a cookie in the domain matching the cookie name */
static cookie *COOKIE_findCookie(cookie_domain *domain, LPCWSTR lpszCookieName)
{
- cookie *searchCookie = domain->cookie_tail;
+ struct list * cursor;
TRACE("(%p, %s)\n", domain, debugstr_w(lpszCookieName));
- while (searchCookie)
+ LIST_FOR_EACH(cursor, &domain->cookie_list)
{
+ cookie *searchCookie = LIST_ENTRY(cursor, cookie, entry);
BOOL candidate = TRUE;
if (candidate && lpszCookieName)
{
@@ -137,7 +131,6 @@
}
if (candidate)
return searchCookie;
- searchCookie = searchCookie->prev;
}
return NULL;
}
@@ -149,18 +142,12 @@
HeapFree(GetProcessHeap(), 0, deadCookie->lpCookieName);
if (deadCookie->lpCookieData)
HeapFree(GetProcessHeap(), 0, deadCookie->lpCookieData);
- if (deadCookie->prev)
- deadCookie->prev->next = deadCookie->next;
- if (deadCookie->next)
- deadCookie->next->prev = deadCookie->prev;
-
- if (deadCookie == deadCookie->parent->cookie_tail)
- {
- /* special case: last cookie, lets remove the domain to save memory */
- deadCookie->parent->cookie_tail = deadCookie->prev;
- if (!deadCookie->parent->cookie_tail && deleteDomain)
- COOKIE_deleteDomain(deadCookie->parent);
- }
+ list_remove(&deadCookie->entry);
+
+ /* special case: last cookie, lets remove the domain to save memory */
+ if (list_empty(&deadCookie->parent->cookie_list) && deleteDomain)
+ COOKIE_deleteDomain(deadCookie->parent);
+ HeapFree(GetProcessHeap(), 0, deadCookie);
}
/* allocates a domain and adds it to the end */
@@ -168,9 +155,8 @@
{
cookie_domain *newDomain = HeapAlloc(GetProcessHeap(), 0, sizeof(cookie_domain));
- newDomain->next = NULL;
- newDomain->prev = NULL;
- newDomain->cookie_tail = NULL;
+ list_init(&newDomain->entry);
+ list_init(&newDomain->cookie_list);
newDomain->lpCookieDomain = NULL;
newDomain->lpCookiePath = NULL;
@@ -185,15 +171,14 @@
lstrcpyW(newDomain->lpCookiePath, path);
}
- newDomain->prev = cookieDomainTail;
- cookieDomainTail = newDomain;
+ list_add_tail(&domain_list, &newDomain->entry);
+
TRACE("Adding domain: %p\n", newDomain);
return newDomain;
}
-static cookie_domain *COOKIE_addDomainFromUrl(LPCWSTR lpszUrl)
+static void COOKIE_crackUrlSimple(LPCWSTR lpszUrl, LPWSTR hostName, int hostNameLen, LPWSTR path, int pathLen)
{
- WCHAR hostName[2048], path[2048];
URL_COMPONENTSW UrlComponents;
UrlComponents.lpszExtraInfo = NULL;
@@ -202,119 +187,59 @@
UrlComponents.lpszUrlPath = path;
UrlComponents.lpszUserName = NULL;
UrlComponents.lpszHostName = hostName;
- UrlComponents.dwHostNameLength = 2048;
- UrlComponents.dwUrlPathLength = 2048;
+ UrlComponents.dwHostNameLength = hostNameLen;
+ UrlComponents.dwUrlPathLength = pathLen;
InternetCrackUrlW(lpszUrl, 0, 0, &UrlComponents);
-
- TRACE("Url cracked. Domain: %s, Path: %s.\n", debugstr_w(UrlComponents.lpszHostName),
- debugstr_w(UrlComponents.lpszUrlPath));
-
- /* hack for now - FIXME - There seems to be a bug in InternetCrackUrl?? */
- UrlComponents.lpszUrlPath = NULL;
-
- return COOKIE_addDomain(UrlComponents.lpszHostName, UrlComponents.lpszUrlPath);
}
-/* find a domain. domain must match if its not NULL. path must match if its not NULL */
-static cookie_domain *COOKIE_findNextDomain(LPCWSTR lpszCookieDomain, LPCWSTR lpszCookiePath,
- cookie_domain *prev_domain, BOOL allow_partial)
+/* match a domain. domain must match if the domain is not NULL. path must match if the path is not NULL */
+static BOOL COOKIE_matchDomain(LPCWSTR lpszCookieDomain, LPCWSTR lpszCookiePath,
+ cookie_domain *searchDomain, BOOL allow_partial)
{
- cookie_domain *searchDomain;
-
- if (prev_domain)
- {
- if(!prev_domain->prev)
+ TRACE("searching on domain %p\n", searchDomain);
+ if (lpszCookieDomain)
{
- TRACE("no more domains available, it would seem.\n");
- return NULL;
- }
- searchDomain = prev_domain->prev;
- }
- else searchDomain = cookieDomainTail;
+ if (!searchDomain->lpCookieDomain)
+ return FALSE;
- while (searchDomain)
- {
- BOOL candidate = TRUE;
- TRACE("searching on domain %p\n", searchDomain);
- if (candidate && lpszCookieDomain)
- {
- if (candidate && !searchDomain->lpCookieDomain)
- candidate = FALSE;
- TRACE("candidate! (%p)\n", searchDomain->lpCookieDomain);
TRACE("comparing domain %s with %s\n",
- debugstr_w(lpszCookieDomain),
- debugstr_w(searchDomain->lpCookieDomain));
- if (candidate && allow_partial && !strstrW(lpszCookieDomain, searchDomain->lpCookieDomain))
- candidate = FALSE;
- else if (candidate && !allow_partial &&
- lstrcmpW(lpszCookieDomain, searchDomain->lpCookieDomain) != 0)
- candidate = FALSE;
+ debugstr_w(lpszCookieDomain),
+ debugstr_w(searchDomain->lpCookieDomain));
+
+ if (allow_partial && !strstrW(lpszCookieDomain, searchDomain->lpCookieDomain))
+ return FALSE;
+ else if (!allow_partial && lstrcmpW(lpszCookieDomain, searchDomain->lpCookieDomain) != 0)
+ return FALSE;
}
- if (candidate && lpszCookiePath)
- {
- TRACE("comparing paths\n");
- if (candidate && !searchDomain->lpCookiePath)
- candidate = FALSE;
- if (candidate &&
- strcmpW(lpszCookiePath, searchDomain->lpCookiePath))
- candidate = FALSE;
- }
- if (candidate)
- {
- TRACE("returning the domain %p\n", searchDomain);
- return searchDomain;
+ if (lpszCookiePath)
+ {
+ TRACE("comparing paths: %s with %s\n", debugstr_w(lpszCookiePath), debugstr_w(searchDomain->lpCookiePath));
+ if (!searchDomain->lpCookiePath)
+ return FALSE;
+ if (strcmpW(lpszCookiePath, searchDomain->lpCookiePath))
+ return FALSE;
}
- searchDomain = searchDomain->prev;
- }
- TRACE("found no domain, returning NULL\n");
- return NULL;
-}
-
-static cookie_domain *COOKIE_findNextDomainFromUrl(LPCWSTR lpszUrl, cookie_domain *previous_domain,
- BOOL allow_partial)
-{
- WCHAR hostName[2048], path[2048];
- URL_COMPONENTSW UrlComponents;
-
- UrlComponents.lpszExtraInfo = NULL;
- UrlComponents.lpszPassword = NULL;
- UrlComponents.lpszScheme = NULL;
- UrlComponents.lpszUrlPath = path;
- UrlComponents.lpszUserName = NULL;
- UrlComponents.lpszHostName = hostName;
- UrlComponents.dwHostNameLength = 2048;
- UrlComponents.dwUrlPathLength = 2048;
-
- InternetCrackUrlW(lpszUrl, 0, 0, &UrlComponents);
-
- TRACE("Url cracked. Domain: %s, Path: %s.\n",
- debugstr_w(UrlComponents.lpszHostName),
- debugstr_w(UrlComponents.lpszUrlPath));
-
- /* hack for now - FIXME - There seems to be a bug in InternetCrackUrl?? */
- UrlComponents.lpszUrlPath = NULL;
-
- return COOKIE_findNextDomain(UrlComponents.lpszHostName, UrlComponents.lpszUrlPath,
- previous_domain, allow_partial);
+ return TRUE;
}
/* remove a domain from the list and delete it */
static void COOKIE_deleteDomain(cookie_domain *deadDomain)
{
- while (deadDomain->cookie_tail)
- COOKIE_deleteCookie(deadDomain->cookie_tail, FALSE);
+ struct list * cursor;
+ while ((cursor = list_tail(&deadDomain->cookie_list)))
+ {
+ COOKIE_deleteCookie(LIST_ENTRY(cursor, cookie, entry), FALSE);
+ list_remove(cursor);
+ }
+
if (deadDomain->lpCookieDomain)
HeapFree(GetProcessHeap(), 0, deadDomain->lpCookieDomain);
if (deadDomain->lpCookiePath)
HeapFree(GetProcessHeap(), 0, deadDomain->lpCookiePath);
- if (deadDomain->prev)
- deadDomain->prev->next = deadDomain->next;
- if (deadDomain->next)
- deadDomain->next->prev = deadDomain->prev;
- if (cookieDomainTail == deadDomain)
- cookieDomainTail = deadDomain->prev;
+ list_remove(&deadDomain->entry);
+
HeapFree(GetProcessHeap(), 0, deadDomain);
}
@@ -334,58 +259,50 @@
BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
LPWSTR lpCookieData, LPDWORD lpdwSize)
{
- cookie_domain *cookiesDomain = NULL;
- cookie *thisCookie;
+ struct list * cursor;
int cnt = 0, domain_count = 0;
- /* Ok, this is just ODD!. During my tests, it appears M$ like to send out
- * a cookie called 'MtrxTracking' to some urls. Also returns it from InternetGetCookie.
- * I'm not exactly sure what to make of this, so its here for now.
- * It'd be nice to know what exactly is going on, M$ tracking users? Does this need
- * to be unique? Should I generate a random number here? etc.
- */
- static const WCHAR TrackingString[] = {
- 'M','t','r','x','T','r','a','c','k','i','n','g','I','D','=',
- '0','1','2','3','4','5','6','7','8','9','0','1','2','3','4','5',
- '6','7','8','9','0','1','2','3','4','5','6','7','8','9','0','1', 0 };
- static const WCHAR szps[] = { '%','s',0 };
+ int cookie_count = 0;
+ WCHAR hostName[2048], path[2048];
TRACE("(%s, %s, %p, %p)\n", debugstr_w(lpszUrl),debugstr_w(lpszCookieName),
lpCookieData, lpdwSize);
- if (lpCookieData)
- cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szps, TrackingString);
- else
- cnt += strlenW(TrackingString);
+ COOKIE_crackUrlSimple(lpszUrl, hostName, sizeof(hostName)/sizeof(hostName[0]), path, sizeof(path)/sizeof(path[0]));
- while ((cookiesDomain = COOKIE_findNextDomainFromUrl(lpszUrl, cookiesDomain, TRUE)))
+ LIST_FOR_EACH(cursor, &domain_list)
{
- domain_count++;
- TRACE("found domain %p\n", cookiesDomain);
-
- thisCookie = cookiesDomain->cookie_tail;
- if (lpCookieData == NULL) /* return the size of the buffer required to lpdwSize */
- {
- while (thisCookie)
- {
- cnt += 2; /* '; ' */
- cnt += strlenW(thisCookie->lpCookieName);
- cnt += 1; /* = */
- cnt += strlenW(thisCookie->lpCookieData);
-
- thisCookie = thisCookie->prev;
- }
- }
- while (thisCookie)
- {
- static const WCHAR szsc[] = { ';',' ',0 };
- static const WCHAR szpseq[] = { '%','s','=','%','s',0 };
- cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szsc);
- cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szpseq,
- thisCookie->lpCookieName,
- thisCookie->lpCookieData);
-
- thisCookie = thisCookie->prev;
- }
+ cookie_domain *cookiesDomain = LIST_ENTRY(cursor, cookie_domain, entry);
+ if (COOKIE_matchDomain(hostName, NULL /* FIXME: path */, cookiesDomain, TRUE))
+ {
+ struct list * cursor;
+ domain_count++;
+ TRACE("found domain %p\n", cookiesDomain);
+
+ LIST_FOR_EACH(cursor, &cookiesDomain->cookie_list)
+ {
+ cookie *thisCookie = LIST_ENTRY(cursor, cookie, entry);
+ if (lpCookieData == NULL) /* return the size of the buffer required to lpdwSize */
+ {
+ if (cookie_count != 0)
+ cnt += 2; /* '; ' */
+ cnt += strlenW(thisCookie->lpCookieName);
+ cnt += 1; /* = */
+ cnt += strlenW(thisCookie->lpCookieData);
+ }
+ else
+ {
+ static const WCHAR szsc[] = { ';',' ',0 };
+ static const WCHAR szpseq[] = { '%','s','=','%','s',0 };
+ if (cookie_count != 0)
+ cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szsc);
+ cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szpseq,
+ thisCookie->lpCookieName,
+ thisCookie->lpCookieData);
+ TRACE("Cookie: %s=%s\n", debugstr_w(thisCookie->lpCookieName), debugstr_w(thisCookie->lpCookieData));
+ }
+ cookie_count++;
+ }
+ }
}
if (lpCookieData == NULL)
{
@@ -478,8 +395,10 @@
BOOL WINAPI InternetSetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
LPCWSTR lpCookieData)
{
+ cookie_domain *thisCookieDomain = NULL;
cookie *thisCookie;
- cookie_domain *thisCookieDomain;
+ WCHAR hostName[2048], path[2048];
+ struct list * cursor;
TRACE("(%s,%s,%s)\n", debugstr_w(lpszUrl),
debugstr_w(lpszCookieName), debugstr_w(lpCookieData));
@@ -516,8 +435,17 @@
return ret;
}
- if (!(thisCookieDomain = COOKIE_findNextDomainFromUrl(lpszUrl, NULL, FALSE)))
- thisCookieDomain = COOKIE_addDomainFromUrl(lpszUrl);
+ COOKIE_crackUrlSimple(lpszUrl, hostName, sizeof(hostName)/sizeof(hostName[0]), path, sizeof(path)/sizeof(path[0]));
+
+ LIST_FOR_EACH(cursor, &domain_list)
+ {
+ thisCookieDomain = LIST_ENTRY(cursor, cookie_domain, entry);
+ if (COOKIE_matchDomain(hostName, NULL /* FIXME: path */, thisCookieDomain, FALSE))
+ break;
+ thisCookieDomain = NULL;
+ }
+ if (!thisCookieDomain)
+ thisCookieDomain = COOKIE_addDomain(hostName, path);
if ((thisCookie = COOKIE_findCookie(thisCookieDomain, lpszCookieName)))
COOKIE_deleteCookie(thisCookie, FALSE);
More information about the wine-patches
mailing list