wininet: fixed the handling of insufficient buffers in InternetGetCookie*().
Eric van Beurden
ericvb at transgaming.com
Tue Jun 29 10:23:17 CDT 2010
---
dlls/wininet/cookie.c | 181 ++++++++++++++++++++++++++++++++---------
dlls/wininet/tests/internet.c | 181 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 324 insertions(+), 38 deletions(-)
diff --git a/dlls/wininet/cookie.c b/dlls/wininet/cookie.c
index 75cf049..09e4161 100644
--- a/dlls/wininet/cookie.c
+++ b/dlls/wininet/cookie.c
@@ -265,22 +265,21 @@ static void COOKIE_deleteDomain(cookie_domain *deadDomain)
*
* Retrieve cookie from the specified url
*
- * It should be noted that on windows the lpszCookieName parameter is "not implemented".
- * So it won't be implemented here.
- *
* RETURNS
* TRUE on success
* FALSE on failure
*
*/
-BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
- LPWSTR lpCookieData, LPDWORD lpdwSize)
+static BOOL DoInternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
+ LPWSTR lpCookieData, LPDWORD lpdwSize, BOOL ignoreName)
{
BOOL ret;
struct list * cursor;
unsigned int cnt = 0, domain_count = 0, cookie_count = 0;
WCHAR hostName[2048], path[2048];
FILETIME tm;
+ BOOL overflow = FALSE;
+ BOOL justCount;
TRACE("(%s, %s, %p, %p)\n", debugstr_w(lpszUrl),debugstr_w(lpszCookieName),
lpCookieData, lpdwSize);
@@ -291,6 +290,12 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
return FALSE;
}
+ justCount = (lpCookieData == NULL);
+
+ /* no point in checking the cookie names if none is given */
+ if (lpszCookieName == NULL)
+ ignoreName = TRUE;
+
hostName[0] = 0;
ret = COOKIE_crackUrlSimple(lpszUrl, hostName, sizeof(hostName)/sizeof(hostName[0]), path, sizeof(path)/sizeof(path[0]));
if (!ret || !hostName[0]) return FALSE;
@@ -309,6 +314,10 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
LIST_FOR_EACH(cursor, &cookiesDomain->cookie_list)
{
cookie *thisCookie = LIST_ENTRY(cursor, cookie, entry);
+ DWORD len;
+ size_t cookieNameLen;
+ size_t cookieValueLen = 0;
+
/* check for expiry */
if ((thisCookie->expiry.dwLowDateTime != 0 || thisCookie->expiry.dwHighDateTime != 0) && CompareFileTime(&tm,&thisCookie->expiry) > 0)
{
@@ -317,32 +326,61 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
continue;
}
- if (lpCookieData == NULL) /* return the size of the buffer required to lpdwSize */
+ /* this is not the cookie we're looking for => skip to the next one */
+ /* note that cookie names are *not* case sensitive */
+ if (!ignoreName && strcmpiW(thisCookie->lpCookieName, lpszCookieName))
+ continue;
+
+ /* figure out how many characters this cookie and value will occupy */
+ len = 0;
+ if (cookie_count) len += 2; /* '; ' */
+ cookieNameLen = strlenW(thisCookie->lpCookieName);
+ len += cookieNameLen;
+ if (thisCookie->lpCookieData[0])
{
- unsigned int len;
+ len += 1; /* = */
+ cookieValueLen = strlenW(thisCookie->lpCookieData);
+ len += cookieValueLen;
+ }
+
+ /* attempt to write to the buffer. Testing shows that the native version does not attempt to
+ write partial cookie name/value pairs to the buffer if it runs out of space. */
+ if (!justCount && cnt + len < *lpdwSize)
+ {
+ static const WCHAR szsc[] = {';', ' ', 0};
+
- if (cookie_count) cnt += 2; /* '; ' */
- cnt += strlenW(thisCookie->lpCookieName);
- if ((len = strlenW(thisCookie->lpCookieData)))
+ if (cookie_count)
{
- cnt += 1; /* = */
- cnt += len;
+ lstrcpyW(lpCookieData + cnt, szsc);
+ cnt += strlenW(szsc);
}
- }
- else
- {
- static const WCHAR szsc[] = { ';',' ',0 };
- static const WCHAR szname[] = { '%','s',0 };
- static const WCHAR szdata[] = { '=','%','s',0 };
- if (cookie_count) cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szsc);
- cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szname, thisCookie->lpCookieName);
+ lstrcpyW(lpCookieData + cnt, thisCookie->lpCookieName);
+ cnt += cookieNameLen;
if (thisCookie->lpCookieData[0])
- cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szdata, thisCookie->lpCookieData);
+ {
+ lpCookieData[cnt++] = '=';
+ lstrcpyW(lpCookieData + cnt, thisCookie->lpCookieData);
+ cnt += cookieValueLen;
+ }
TRACE("Cookie: %s\n", debugstr_w(lpCookieData));
}
+
+ else
+ {
+ /* the buffer would have overflowed when adding this cookie => mark the error and continue counting */
+ if (!justCount)
+ {
+ overflow = TRUE;
+ justCount = TRUE;
+ }
+
+ cnt += len;
+ }
+
cookie_count++;
}
}
@@ -355,6 +393,15 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
return FALSE;
}
+ /* insufficent buffer to store the result => return the required amount */
+ if (overflow)
+ {
+ TRACE("ran out of buffer space. Given %u, required %d\n", *lpdwSize, cnt + 1);
+ *lpdwSize = (cnt + 1) * sizeof(WCHAR);
+ SetLastError(ERROR_INSUFFICIENT_BUFFER);
+ return FALSE;
+ }
+
if (lpCookieData == NULL)
{
*lpdwSize = (cnt + 1) * sizeof(WCHAR);
@@ -364,12 +411,19 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
*lpdwSize = cnt + 1;
- TRACE("Returning %u (from %u domains): %s\n", cnt, domain_count,
- debugstr_w(lpCookieData));
+ TRACE("Returning %u (from %u domains): %s {cookie_count = %d}\n", cnt, domain_count,
+ debugstr_w(lpCookieData), cookie_count);
return (cnt ? TRUE : FALSE);
}
+BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
+ LPWSTR lpCookieData, LPDWORD lpdwSize)
+{
+ /* testing shows that the wide version of this function does actually respect the provided
+ cookie name and will only return information about that specific cookie. */
+ return DoInternetGetCookieW(lpszUrl, lpszCookieName, lpCookieData, lpdwSize, FALSE);
+}
/***********************************************************************
* InternetGetCookieA (WININET.@)
@@ -385,35 +439,86 @@ BOOL WINAPI InternetGetCookieA(LPCSTR lpszUrl, LPCSTR lpszCookieName,
LPSTR lpCookieData, LPDWORD lpdwSize)
{
DWORD len;
- LPWSTR szCookieData = NULL, url, name;
- BOOL r;
+ LPWSTR szCookieData = NULL, url = NULL, name = NULL;
+ BOOL r = FALSE;
+ WCHAR dummy;
TRACE("(%s,%s,%p)\n", debugstr_a(lpszUrl), debugstr_a(lpszCookieName),
lpCookieData);
+ if (lpdwSize == NULL)
+ {
+ SetLastError(ERROR_INVALID_PARAMETER);
+ return FALSE;
+ }
+
url = heap_strdupAtoW(lpszUrl);
name = heap_strdupAtoW(lpszCookieName);
- r = InternetGetCookieW( url, name, NULL, &len );
- if( r )
+ if ((url == NULL && lpszUrl != NULL) ||
+ (name == NULL && lpszCookieName != NULL))
{
- szCookieData = HeapAlloc( GetProcessHeap(), 0, len * sizeof(WCHAR) );
- if( !szCookieData )
+ ERR("not enough memory for the URL or cookie name\n");
+ SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+ goto Done;
+ }
+ if (lpCookieData)
+ {
+ /* a non-zero length buffer was provided => allocate a buffer for the wide version */
+ if (*lpdwSize)
{
- r = FALSE;
+ szCookieData = HeapAlloc(GetProcessHeap(), 0, (*lpdwSize) * sizeof(WCHAR));
+
+ if (szCookieData == NULL)
+ {
+ ERR("not enough memory for the output buffer\n");
+ SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+ goto Done;
+ }
}
+
+ /* a buffer was provided but its size was reported as 0 => use a dummy buffer so that we
+ aren't passing NULL to the function. */
else
- {
- r = InternetGetCookieW( url, name, szCookieData, &len );
+ szCookieData = &dummy;
+ }
- *lpdwSize = WideCharToMultiByte( CP_ACP, 0, szCookieData, len,
- lpCookieData, *lpdwSize, NULL, NULL );
- }
+ /* save the original input buffer length */
+ len = *lpdwSize;
+
+ /* testing shows that the ANSI version of the function *always* returns the full cookie list
+ even if a valid cookie name is specified. */
+ r = DoInternetGetCookieW( url, name, szCookieData, lpdwSize, TRUE);
+
+ /* function succeeded and possibly returned a string */
+ if (r)
+ {
+ /* no output buffer provided => just correct the required length. When no buffer is given,
+ the wide version of the function returns the size in bytes, not characters. */
+ if (lpCookieData == NULL)
+ *lpdwSize /= sizeof(WCHAR);
+
+ /* an output buffer was provided => convert the output string. Note that on success, the
+ <lpdwSize> parameter already returns the resulting string size in characters, so there
+ is no need to convert it. */
+ else
+ WideCharToMultiByte(CP_ACP, 0, szCookieData, *lpdwSize, lpCookieData, len, NULL, NULL);
}
- HeapFree( GetProcessHeap(), 0, szCookieData );
- HeapFree( GetProcessHeap(), 0, name );
- HeapFree( GetProcessHeap(), 0, url );
+ /* function failed due to a buffer overflow => correct the required length. On this failure,
+ the <lpdwSize> parameter is returned as the number of bytes, not characters. */
+ else if (GetLastError() == ERROR_INSUFFICIENT_BUFFER)
+ *lpdwSize /= sizeof(WCHAR);
+
+Done:
+ if (name)
+ HeapFree( GetProcessHeap(), 0, name );
+
+ if (url)
+ HeapFree( GetProcessHeap(), 0, url );
+
+ if (szCookieData && szCookieData != &dummy)
+ HeapFree( GetProcessHeap(), 0, szCookieData );
return r;
}
diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c
index 4da9c21..1144948 100644
--- a/dlls/wininet/tests/internet.c
+++ b/dlls/wininet/tests/internet.c
@@ -50,6 +50,25 @@ static int strcmp_ww(const WCHAR *str1, const WCHAR *str2)
return memcmp(str1, str2, len1 * sizeof(WCHAR));
}
+static const WCHAR *strstr_ww(const WCHAR *str, const WCHAR *sub)
+{
+ DWORD subLen = lstrlenW(sub) * sizeof(WCHAR);
+
+ while (*str)
+ {
+ /* found the start of the sub string => check if the string matches */
+ if (str[0] == sub[0])
+ {
+ if (!memcmp(str, sub, subLen))
+ return str;
+ }
+
+ str++;
+ }
+
+ return NULL;
+}
+
/* ############################### */
static void test_InternetCanonicalizeUrlA(void)
@@ -268,6 +287,20 @@ static void test_complicated_cookie(void)
BOOL ret;
CHAR buffer[1024];
+ WCHAR bufferw[1024];
+ WCHAR namew[] = {'C', 0};
+ WCHAR name2w[] = {'c', 0};
+ WCHAR cookieAw[] = {'A', '=', 'B', 0};
+ WCHAR cookieCw[] = {'C', '=', 'D', 0};
+ WCHAR cookieEw[] = {'E', '=', 'F', 0};
+ WCHAR cookieGw[] = {'G', '=', 'H', 0};
+ WCHAR cookieIw[] = {'I', '=', 'J', 0};
+ WCHAR cookieKw[] = {'K', '=', 'L', 0};
+ WCHAR cookieMw[] = {'M', '=', 'N', 0};
+ WCHAR cookieOw[] = {'O', '=', 'P', 0};
+ WCHAR urlw[] = {'h', 't', 't', 'p', ':', '/', '/', 't', 'e', 's', 't', 'i', 'n', 'g', '.',
+ 'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm', '/', 'b', 'a', 'r',
+ '/', 'f', 'o', 'o', 0};
ret = InternetSetCookie("http://www.example.com/bar",NULL,"A=B; domain=.example.com");
ok(ret == TRUE,"InternetSetCookie failed\n");
@@ -364,6 +397,154 @@ static void test_complicated_cookie(void)
ok(strstr(buffer,"K=L")!=NULL,"K=L missing\n");
ok(strstr(buffer,"M=N")==NULL,"M=N present\n");
ok(strstr(buffer,"O=P")==NULL,"O=P present\n");
+
+ /* test searching only for a specific cookie (should find all cookies still) */
+ len = 1024;
+ ret = InternetGetCookieA("http://testing.example.com/bar/foo", "C", buffer, &len);
+ ok(strstr(buffer,"A=B")!=NULL,"A=B missing\n");
+ ok(strstr(buffer,"C=D")!=NULL,"C=D missing\n");
+ ok(strstr(buffer,"E=F")!=NULL,"E=F missing\n");
+ ok(strstr(buffer,"G=H")==NULL,"G=H present\n");
+ ok(strstr(buffer,"I=J")!=NULL,"I=J missing\n");
+ ok(strstr(buffer,"K=L")!=NULL,"K=L missing\n");
+ ok(strstr(buffer,"M=N")==NULL,"M=N present\n");
+ ok(strstr(buffer,"O=P")==NULL,"O=P present\n");
+
+ /* test searching only for a specific cookie with the wrong case (should find all cookies still) */
+ len = 1024;
+ ret = InternetGetCookieA("http://testing.example.com/bar/foo", "c", buffer, &len);
+ ok(strstr(buffer,"A=B")!=NULL,"A=B missing\n");
+ ok(strstr(buffer,"C=D")!=NULL,"C=D missing\n");
+ ok(strstr(buffer,"E=F")!=NULL,"E=F missing\n");
+ ok(strstr(buffer,"G=H")==NULL,"G=H present\n");
+ ok(strstr(buffer,"I=J")!=NULL,"I=J missing\n");
+ ok(strstr(buffer,"K=L")!=NULL,"K=L missing\n");
+ ok(strstr(buffer,"M=N")==NULL,"M=N present\n");
+ ok(strstr(buffer,"O=P")==NULL,"O=P present\n");
+
+ /* test the wide version of the function */
+ len = 1024;
+ ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+ ok(strstr_ww(bufferw,cookieAw)!=NULL,"A=B missing\n");
+ ok(strstr_ww(bufferw,cookieCw)!=NULL,"C=D missing\n");
+ ok(strstr_ww(bufferw,cookieEw)!=NULL,"E=F missing\n");
+ ok(strstr_ww(bufferw,cookieGw)==NULL,"G=H present\n");
+ ok(strstr_ww(bufferw,cookieIw)!=NULL,"I=J missing\n");
+ ok(strstr_ww(bufferw,cookieKw)!=NULL,"K=L missing\n");
+ ok(strstr_ww(bufferw,cookieMw)==NULL,"M=N present\n");
+ ok(strstr_ww(bufferw,cookieOw)==NULL,"O=P present\n");
+
+ /* test searching only for a specific cookie (should only find the requested cookie) */
+ len = 1024;
+ ret = InternetGetCookieW(urlw, namew, bufferw, &len);
+ ok(strstr_ww(bufferw,cookieAw)==NULL,"A=B present\n");
+ ok(strstr_ww(bufferw,cookieCw)!=NULL,"C=D missing\n");
+ ok(strstr_ww(bufferw,cookieEw)==NULL,"E=F present\n");
+ ok(strstr_ww(bufferw,cookieGw)==NULL,"G=H present\n");
+ ok(strstr_ww(bufferw,cookieIw)==NULL,"I=J present\n");
+ ok(strstr_ww(bufferw,cookieKw)==NULL,"K=L present\n");
+ ok(strstr_ww(bufferw,cookieMw)==NULL,"M=N present\n");
+ ok(strstr_ww(bufferw,cookieOw)==NULL,"O=P present\n");
+
+ /* test searching only for a specific cookie with the wrong case (should only find the requested cookie) */
+ len = 1024;
+ ret = InternetGetCookieW(urlw, name2w, bufferw, &len);
+ ok(strstr_ww(bufferw,cookieAw)==NULL,"A=B present\n");
+ ok(strstr_ww(bufferw,cookieCw)!=NULL,"C=D missing\n");
+ ok(strstr_ww(bufferw,cookieEw)==NULL,"E=F present\n");
+ ok(strstr_ww(bufferw,cookieGw)==NULL,"G=H present\n");
+ ok(strstr_ww(bufferw,cookieIw)==NULL,"I=J present\n");
+ ok(strstr_ww(bufferw,cookieKw)==NULL,"K=L present\n");
+ ok(strstr_ww(bufferw,cookieMw)==NULL,"M=N present\n");
+ ok(strstr_ww(bufferw,cookieOw)==NULL,"O=P present\n");
+
+ /* test with a valid buffer and 0 length */
+ len = 0;
+ SetLastError(-1);
+ ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, &len);
+ ok(!ret, "the function succeeded\n");
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+ /* test with a valid length but no buffer */
+ len = 1024;
+ SetLastError(-1);
+ ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, NULL, &len);
+ ok(ret, "the function succeeded\n");
+ ok(GetLastError() == -1, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+ /* test with a NULL length */
+ SetLastError(-1);
+ ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, NULL);
+ ok(!ret, "the function succeeded\n");
+ ok(GetLastError() == ERROR_INVALID_PARAMETER, "bad last error {error = %d}\n", GetLastError());
+
+ /* test with a buffer that is too short */
+ len = 17;
+ SetLastError(-1);
+ ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, &len);
+ ok(!ret, "the function succeeded\n");
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+ len = 23;
+ SetLastError(-1);
+ ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, &len);
+ ok(!ret, "the function succeeded\n");
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+ len = 24;
+ SetLastError(-1);
+ ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, &len);
+ ok(ret, "the function failed\n");
+ ok(GetLastError() == -1, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+ len = 0;
+ SetLastError(-1);
+ ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+ ok(!ret, "the function succeeded\n");
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 48, "unexpected required length {len = %d}\n", len);
+
+ /* test with a valid length but no buffer */
+ len = 1024;
+ SetLastError(-1);
+ ret = InternetGetCookieW(urlw, NULL, NULL, &len);
+ ok(ret, "the function succeeded\n");
+ ok(GetLastError() == -1, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 48, "unexpected required length {len = %d}\n", len);
+
+ /* test with a NULL length */
+ /* this test crashes on native */
+ /*SetLastError(-1);
+ ret = InternetGetCookieW(urlw, NULL, bufferw, NULL);
+ ok(!ret, "the function succeeded\n");
+ ok(GetLastError() == ERROR_INVALID_PARAMETER, "bad last error {error = %d}\n", GetLastError());*/
+
+ /* test with a buffer that is too short */
+ len = 17;
+ SetLastError(-1);
+ ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+ ok(!ret, "the function succeeded\n");
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 48, "unexpected required length {len = %d}\n", len);
+
+ len = 23;
+ SetLastError(-1);
+ ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+ ok(!ret, "the function succeeded\n");
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 48, "unexpected required length {len = %d}\n", len);
+
+ len = 24;
+ SetLastError(-1);
+ ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+ ok(ret, "the function failed\n");
+ ok(GetLastError() == -1, "bad last error {error = %d}\n", GetLastError());
+ ok(len == 24, "unexpected required length {len = %d}\n", len);
}
static void test_null(void)
--
1.6.6.1
--------------070506090407020705050909--
More information about the wine-patches
mailing list