On 05/31/2011 02:17 AM, Hans Leidekker wrote:
> On Mon, 2011-05-30 at 21:13 -0700, Dan Kegel wrote:
>
>> @@ -91,7 +91,7 @@ GetUserNameW( LPWSTR lpszName, LPDWORD lpSize )
>>
>> if (len > *lpSize)
>> {
>> - SetLastError(ERROR_MORE_DATA);
>> + SetLastError(ERROR_INSUFFICIENT_BUFFER);
>
> GetUserNameA also needs to be fixed. It sets ERROR_MORE_DATA when
> WideCharToMultiByte fails, which can simply be removed because that
> function will set ERROR_INSUFFICIENT_BUFFER itself if the buffer is too
> small.
>
> And there are at least two cases, in dlls/crypt32/protectdata.c and
> dlls/msi/package.c, where we depend on the wrong error.
>
>
>
> I didn't intend to scoop Dan's work, but I have a version of his fix, which is attached, with deeper tests which I wrote before Hans expressed his concerns. Should I just send in the tests with todo_wine and let Dan fix those, or should I send a revised version and allow a "winner" to be picked?
From 7bdccf239e3accc598eaf9b372a70e28b18f77df Mon Sep 17 00:00:00 2001
From: Andrew Nguyen <angu...@codeweavers.com>
Date: Tue, 31 May 2011 01:05:15 -0500
Subject: advapi32: Fix last error value of GetUserNameA/W and output buffer
handling in GetUserNameA.
To: wine-patches <wine-patc...@winehq.org>
Reply-To: wine-devel <wine-devel@winehq.org>
---
dlls/advapi32/advapi.c | 20 ++++--
dlls/advapi32/tests/security.c | 145 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 159 insertions(+), 6 deletions(-)
diff --git a/dlls/advapi32/advapi.c b/dlls/advapi32/advapi.c
index 47286a1..309acb0 100644
--- a/dlls/advapi32/advapi.c
+++ b/dlls/advapi32/advapi.c
@@ -55,24 +55,32 @@ GetUserNameA( LPSTR lpszName, LPDWORD lpSize )
{
WCHAR *buffer;
BOOL ret;
- DWORD sizeW = *lpSize * 2;
+ DWORD sizeW = 0;
+
+ GetUserNameW( NULL, &sizeW );
if (!(buffer = HeapAlloc( GetProcessHeap(), 0, sizeW * sizeof(WCHAR) )))
{
SetLastError( ERROR_NOT_ENOUGH_MEMORY );
return FALSE;
}
+
ret = GetUserNameW( buffer, &sizeW );
if (ret)
{
- if (!(*lpSize = WideCharToMultiByte( CP_ACP, 0, buffer, -1, lpszName,
*lpSize, NULL, NULL )))
+ DWORD sizeA = WideCharToMultiByte( CP_ACP, 0, buffer, -1, NULL, 0,
NULL, NULL );
+
+ if (sizeA > *lpSize)
{
- *lpSize = WideCharToMultiByte( CP_ACP, 0, buffer, -1, NULL, 0,
NULL, NULL );
- SetLastError( ERROR_MORE_DATA );
+ SetLastError( ERROR_INSUFFICIENT_BUFFER );
ret = FALSE;
}
+ else
+ WideCharToMultiByte( CP_ACP, 0, buffer, -1, lpszName, *lpSize,
NULL, NULL );
+
+ *lpSize = sizeA;
}
- else *lpSize = sizeW * 2;
+
HeapFree( GetProcessHeap(), 0, buffer );
return ret;
}
@@ -91,7 +99,7 @@ GetUserNameW( LPWSTR lpszName, LPDWORD lpSize )
if (len > *lpSize)
{
- SetLastError(ERROR_MORE_DATA);
+ SetLastError( ERROR_INSUFFICIENT_BUFFER );
*lpSize = len;
return FALSE;
}
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index 4b7473f..6a9a1d2 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -3723,6 +3723,149 @@ static void test_EqualSid(void)
FreeSid(sid2);
}
+static void test_GetUserNameA(void)
+{
+ char buffer[UNLEN + 1], filler[UNLEN + 1];
+ DWORD required_len, buffer_len;
+ BOOL ret;
+
+ /* Test crashes on Windows. */
+ if (0)
+ {
+ SetLastError(0xdeadbeef);
+ GetUserNameA(NULL, NULL);
+ }
+
+ SetLastError(0xdeadbeef);
+ required_len = 0;
+ ret = GetUserNameA(NULL, &required_len);
+ ok(ret == FALSE, "GetUserNameA returned %d\n", ret);
+ ok(required_len != 0, "Outputted buffer length was %u\n", required_len);
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Last error was %u\n",
GetLastError());
+
+ SetLastError(0xdeadbeef);
+ required_len = 1;
+ ret = GetUserNameA(NULL, &required_len);
+ ok(ret == FALSE, "GetUserNameA returned %d\n", ret);
+ ok(required_len != 0 && required_len != 1, "Outputted buffer length was
%u\n", required_len);
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Last error was %u\n",
GetLastError());
+
+ /* Tests crashes on Windows. */
+ if (0)
+ {
+ SetLastError(0xdeadbeef);
+ required_len = UNLEN + 1;
+ GetUserNameA(NULL, &required_len);
+
+ SetLastError(0xdeadbeef);
+ GetUserNameA(buffer, NULL);
+ }
+
+ memset(filler, 'x', sizeof(filler));
+
+ /* Note that GetUserNameA on XP and newer outputs the number of bytes
+ * required for a Unicode string, which affects a test in the next block.
*/
+ SetLastError(0xdeadbeef);
+ memcpy(buffer, filler, sizeof(filler));
+ required_len = 0;
+ ret = GetUserNameA(buffer, &required_len);
+ ok(ret == FALSE, "GetUserNameA returned %d\n", ret);
+ ok(!memcmp(buffer, filler, sizeof(filler)), "Output buffer was altered\n");
+ ok(required_len != 0, "Outputted buffer length was %u\n", required_len);
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Last error was %u\n",
GetLastError());
+
+ SetLastError(0xdeadbeef);
+ memcpy(buffer, filler, sizeof(filler));
+ buffer_len = required_len;
+ ret = GetUserNameA(buffer, &buffer_len);
+ ok(ret == TRUE, "GetUserNameA returned %d, last error %u\n", ret,
GetLastError());
+ ok(memcmp(buffer, filler, sizeof(filler)) != 0, "Output buffer was
untouched\n");
+ ok(buffer_len == required_len ||
+ broken(buffer_len == required_len / sizeof(WCHAR)), /* XP+ */
+ "Outputted buffer length was %u\n", buffer_len);
+
+ /* Use the reported buffer size from the last GetUserNameA call and pass
+ * a length that is one less than the required value. */
+ SetLastError(0xdeadbeef);
+ memcpy(buffer, filler, sizeof(filler));
+ buffer_len--;
+ ret = GetUserNameA(buffer, &buffer_len);
+ ok(ret == FALSE, "GetUserNameA returned %d\n", ret);
+ ok(!memcmp(buffer, filler, sizeof(filler)), "Output buffer was
untouched\n");
+ ok(buffer_len == required_len, "Outputted buffer length was %u\n",
buffer_len);
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Last error was %u\n",
GetLastError());
+}
+
+static void test_GetUserNameW(void)
+{
+ WCHAR buffer[UNLEN + 1], filler[UNLEN + 1];
+ DWORD required_len, buffer_len;
+ BOOL ret;
+
+ /* Test crashes on Windows. */
+ if (0)
+ {
+ SetLastError(0xdeadbeef);
+ GetUserNameW(NULL, NULL);
+ }
+
+ SetLastError(0xdeadbeef);
+ required_len = 0;
+ ret = GetUserNameW(NULL, &required_len);
+ ok(ret == FALSE, "GetUserNameW returned %d\n", ret);
+ ok(required_len != 0, "Outputted buffer length was %u\n", required_len);
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Last error was %u\n",
GetLastError());
+
+ SetLastError(0xdeadbeef);
+ required_len = 1;
+ ret = GetUserNameW(NULL, &required_len);
+ ok(ret == FALSE, "GetUserNameW returned %d\n", ret);
+ ok(required_len != 0 && required_len != 1, "Outputted buffer length was
%u\n", required_len);
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Last error was %u\n",
GetLastError());
+
+ /* Tests crash on Windows. */
+ if (0)
+ {
+ SetLastError(0xdeadbeef);
+ required_len = UNLEN + 1;
+ GetUserNameW(NULL, &required_len);
+
+ SetLastError(0xdeadbeef);
+ GetUserNameW(buffer, NULL);
+ }
+
+ memset(filler, 'x', sizeof(filler));
+
+ SetLastError(0xdeadbeef);
+ memcpy(buffer, filler, sizeof(filler));
+ required_len = 0;
+ ret = GetUserNameW(buffer, &required_len);
+ ok(ret == FALSE, "GetUserNameW returned %d\n", ret);
+ ok(!memcmp(buffer, filler, sizeof(filler)), "Output buffer was altered\n");
+ ok(required_len != 0, "Outputted buffer length was %u\n", required_len);
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Last error was %u\n",
GetLastError());
+
+ SetLastError(0xdeadbeef);
+ memcpy(buffer, filler, sizeof(filler));
+ buffer_len = required_len;
+ ret = GetUserNameW(buffer, &buffer_len);
+ ok(ret == TRUE, "GetUserNameW returned %d, last error %u\n", ret,
GetLastError());
+ ok(memcmp(buffer, filler, sizeof(filler)) != 0, "Output buffer was
untouched\n");
+ ok(buffer_len == required_len, "Outputted buffer length was %u\n",
buffer_len);
+
+ /* GetUserNameW on XP and newer writes a truncated portion of the username
string to the buffer. */
+ SetLastError(0xdeadbeef);
+ memcpy(buffer, filler, sizeof(filler));
+ buffer_len--;
+ ret = GetUserNameW(buffer, &buffer_len);
+ ok(ret == FALSE, "GetUserNameW returned %d\n", ret);
+ ok(!memcmp(buffer, filler, sizeof(filler)) ||
+ broken(memcmp(buffer, filler, sizeof(filler)) != 0), /* XP+ */
+ "Output buffer was altered\n");
+ ok(buffer_len == required_len, "Outputted buffer length was %u\n",
buffer_len);
+ ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Last error was %u\n",
GetLastError());
+}
+
START_TEST(security)
{
init();
@@ -3756,4 +3899,6 @@ START_TEST(security)
test_GetSidSubAuthority();
test_CheckTokenMembership();
test_EqualSid();
+ test_GetUserNameA();
+ test_GetUserNameW();
}
--
1.7.5.2
signature.asc
Description: OpenPGP digital signature
