Some issues on identifying user.name on Windows

[Dmytro Sheyko]

Hi,

I noticed following code

openjdk-7-ea-src-b127-27_jan_2011\jdk\src\windows\native\java\lang\java_props_md.c

    /*
     * User name
     * We try to avoid calling GetUserName as it turns out to
     * be surprisingly expensive on NT.  It pulls in an extra
     * 100 K of footprint.
     */
    {
        WCHAR *uname = _wgetenv(L"USERNAME");
        if (uname != NULL && wcslen(uname) > 0) {
            sprops.user_name = _wcsdup(uname);
        } else {
            WCHAR buf[100];
            int buflen = sizeof(buf);
            sprops.user_name =
                GetUserNameW(buf, &buflen) ? _wcsdup(buf) : L"unknown";
        }
    }

In my opinion there are some issues in it.

1. We identify buffer length in bytes but GetUserNameW expects size in wchars. This can lead to buffer overflow.

2. In normal cases user name is allocated in heap (_wcsdup does this), but in problematic case user_name points to the static string "unknown".
This can lead to problems once we decide to free memory.

3. Solaris and Linux return "?" in problematic case, but Windows returns "unknown". This seems inconsistent. I believe that Windows should also return "?"
in order to make it consistent with Linux and Solaris and to let know whether username is definitely unknown. It's impossible to create user with name "?"
because it has a forbidden character. On the other hand it is possible have user with name "unknown" literally, so if java says that user name is "unknown"
we can't definitely know whether user is really unknown or he has such a name.

I propose following code instead:

    /*
     * User name
     * We try to avoid calling GetUserName as it turns out to
     * be surprisingly expensive on NT.  It pulls in an extra
     * 100 K of footprint.
     */
    {
        WCHAR *uname = _wgetenv(L"USERNAME");
        if (uname != NULL && wcslen(uname) > 0) {
            sprops.user_name = _wcsdup(uname);
        } else {
            sprops.user_name = IdentifyUserName();
        }
    }


static LPWSTR IdentifyUserName()
{
    DWORD buflen = 0;
    GetUserNameW(0, &buflen); // identify buffer length
    if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
         // 'buflen' receives the required buffer size in WCHARs, including the terminating null character.
        LPWSTR result = (LPWSTR) malloc(buflen * sizeof(WCHAR));
        if (result) {
            if (GetUserNameW(result, &buflen)) {
                return result;
            } else {
                free(result);
            }
        }
    }
    return _wcsdup(L"?"); // allocate username in heap, use impossible username
}

Regards,
Dmytro
 		 	   		  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20110208/66e32e06/attachment.html>