Changeset 96451 in vbox

Timestamp:

Aug 24, 2022 9:56:54 AM (2 years ago)

Author:

vboxsync

Message:

Add/NT/Inst,Add/NT/VBoxTray,Add/VBoxService: Cleaned up VBoxGuestInstallHelper.cpp (tested) and the VBoxTray IPC interface (not tested). The motivation for the former was to make it compile in no-CRT mode, the latter was buggy code. The IPC interface is not backwards compatible, this is intentional to avoid buggy code. bugref:10261

Location:

trunk/src/VBox/Additions

Files:

• WINNT/Installer/InstallHelper/Makefile.kmk (modified) (1 diff)
• WINNT/Installer/InstallHelper/VBoxGuestInstallHelper.cpp (modified) (9 diffs)
• WINNT/Installer/InstallHelper/exdll.h (modified) (5 diffs)
• WINNT/Installer/InstallHelper/testcase/tstWinAdditionsInstallHelper.cpp (modified) (11 diffs)
• WINNT/Installer/VBoxGuestAdditionsLog.nsh (modified) (1 diff)
• WINNT/VBoxTray/VBoxIPC.cpp (modified) (4 diffs)
• WINNT/VBoxTray/VBoxTrayMsg.h (modified) (1 diff)
• common/VBoxService/VBoxServiceVMInfo-win.cpp (modified) (5 diffs)

trunk/src/VBox/Additions/WINNT/Installer/InstallHelper/Makefile.kmk

@@ -35 +35 @@
 # NSIS plugins always have to be compiled in 32-bit!
 #
+# Note! Installation is required for repacking Guest Additions
+#
 ## @todo kBuild: This needs the WINPSDK fix to work for cross building.
 DLLS.x86 += VBoxGuestInstallHelper
-VBoxGuestInstallHelper_TEMPLATE = VBoxGuestR3Dll
-#Installation is required for repacking Guest Additions
-VBoxGuestInstallHelper_INST     = repackadd/resources/VBoxGuestInstallHelper/
+VBoxGuestInstallHelper_TEMPLATE        := VBoxGuestR3Dll
+VBoxGuestInstallHelper_INST             := repackadd/resources/VBoxGuestInstallHelper/
 ifdef VBOX_SIGN_ADDITIONS # (See the parent makefile.)
- VBoxGuestInstallHelper_INSTTYPE       = stage
- VBoxGuestInstallHelper_DEBUG_INSTTYPE = both
+ VBoxGuestInstallHelper_INSTTYPE       := stage
+ VBoxGuestInstallHelper_DEBUG_INSTTYPE := both
 endif
-VBoxGuestInstallHelper_DEFS         = _WIN32_WINNT=0x0400 WIN32_LEAN_AND_MEAN=1 UNICODE _UNICODE
-VBoxGuestInstallHelper_BLD_TRG_ARCH = x86
-VBoxGuestInstallHelper_SOURCES  = \
+VBoxGuestInstallHelper_DEFS             := _WIN32_WINNT=0x0400 WIN32_LEAN_AND_MEAN=1 UNICODE _UNICODE
+VBoxGuestInstallHelper_BLD_TRG_ARCH    := x86
+VBoxGuestInstallHelper_SOURCES         := \
         VBoxGuestInstallHelper.cpp \
         VBoxGuestInstallHelper.rc

trunk/src/VBox/Additions/WINNT/Installer/InstallHelper/VBoxGuestInstallHelper.cpp

@@ -31 +31 @@
 *   Header Files                                                                                                                 *
 *********************************************************************************************************************************/
+#ifndef UNICODE
+# define UNICODE
+#endif
 #include <iprt/win/windows.h>
-#include <stdlib.h>
-#include <tchar.h>
-#include <strsafe.h>
-#pragma warning(push)
-#pragma warning(disable: 4995) /* warning C4995: 'lstrcpyA': name was marked as #pragma deprecated */
 #include "exdll.h"
-#pragma warning(pop)
 
 #include <iprt/err.h>
@@ -47 +44 @@
 #include <iprt/process.h>
 #include <iprt/string.h>
-#ifdef UNICODE
-# include <iprt/utf16.h>
-#endif
+#include <iprt/utf16.h>
 
 /* Required structures/defines of VBoxTray. */
@@ -58 +53 @@
 *   Defined Constants And Macros                                                                                                 *
 *********************************************************************************************************************************/
-#define VBOXINSTALLHELPER_EXPORT extern "C" void __declspec(dllexport)
+#define VBOXINSTALLHELPER_EXPORT extern "C" DECLEXPORT(void)
 
 
@@ -70 +65 @@
 *   Global Variables                                                                                                             *
 *********************************************************************************************************************************/
-HINSTANCE               g_hInstance;
-HWND                    g_hwndParent;
-
-/**
- * @todo Clean up this DLL, use more IPRT in here!
- */
-
-/**
- * Pops (gets) a value from the internal NSIS stack.
- * Since the supplied popstring() method easily can cause buffer
- * overflows, use vboxPopString() instead!
- *
- * @return  VBox status code.
- * @param   pszDest     Pointer to pre-allocated string to store result.
- * @param   cchDest     Size (in characters) of pre-allocated string.
- */
-static int vboxPopString(TCHAR *pszDest, size_t cchDest)
-{
-    int rc = VINF_SUCCESS;
-
-    if (!g_stacktop || !*g_stacktop)
-    {
-        rc = VERR_NO_DATA;
-    }
-    else
-    {
-        stack_t *pStack = (*g_stacktop);
-        AssertPtr(pStack);
-
-        HRESULT hr = StringCchCopy(pszDest, cchDest, pStack->text);
-        if (SUCCEEDED(hr))
-        {
-            *g_stacktop = pStack->next;
-            GlobalFree((HGLOBAL)pStack);
-        }
-        else
-            rc = VERR_INVALID_PARAMETER;
-    }
+static HINSTANCE    g_hInstance;
+static HWND         g_hwndParent;
+
+
+/**
+ * Frees a popped stack entry after use.
+ */
+DECLINLINE(void) vboxFreeStackEntry(stack_t *pEntry)
+{
+    if (pEntry)
+        GlobalFree((HGLOBAL)pEntry);
+}
+
+
+/**
+ * Allocates a new stack entry for containing a string of the given length
+ * (excluding terminator)
+ */
+DECLINLINE(stack_t *) vboxAllocStackEntry(size_t cwcString)
+{
+    return (stack_t *)GlobalAlloc(GPTR, RT_UOFFSETOF_DYN(stack_t, text[cwcString + 1]));
+}
+
+
+/**
+ * Pops an entry off the stack, return NULL if empty.
+ *
+ * Call vboxFreeStackEntry when done.
+ *
+ */
+DECLINLINE(stack_t *) vboxPopStack(stack_t **ppTopOfStack)
+{
+    stack_t *pEntry = ppTopOfStack ? *ppTopOfStack : NULL;
+    if (pEntry)
+        *ppTopOfStack = pEntry->next;
+    return pEntry;
+}
+
+
+/**
+ * Pushes an entry onto the stack.
+ */
+DECLINLINE(void) vboxPushStack(stack_t **ppTopOfStack, stack_t *pEntry)
+{
+    pEntry->next = *ppTopOfStack;
+    *ppTopOfStack = pEntry;
+}
+
+
+static void vboxPushUtf16N(stack_t **ppTopOfStack, wchar_t const *pwszString, size_t cwcString)
+{
+    stack_t *pEntry = vboxAllocStackEntry(cwcString);
+
+    memcpy(pEntry->text, pwszString, cwcString * sizeof(pEntry->text[0]));
+    pEntry->text[cwcString] = '\0';
+
+    vboxPushStack(ppTopOfStack, pEntry);
+}
+
+
+static void vboxPushUtf16(stack_t **ppTopOfStack, wchar_t const *pwszString)
+{
+    return vboxPushUtf16N(ppTopOfStack, pwszString, RTUtf16Len(pwszString));
+}
+
+
+#define VBOX_PUSH_STRING_LITERAL(a_ppTopOfStack, a_szLiteral) \
+    vboxPushUtf16N(a_ppTopOfStack, RT_CONCAT(L, a_szLiteral), sizeof(RT_CONCAT(L, a_szLiteral)) / sizeof(wchar_t) - 1)
+
+
+static void vboxPushUtf8(stack_t **ppTopOfStack, char const *pszString)
+{
+    size_t cwcUtf16 = RTStrCalcUtf16Len(pszString);
+    stack_t *pEntry = vboxAllocStackEntry(cwcUtf16);
+
+    PRTUTF16 pwszUtf16 = pEntry->text;
+    int rc = RTStrToUtf16Ex(pszString, RTSTR_MAX, &pwszUtf16, cwcUtf16 + 1, NULL);
+    AssertRC(rc);
+
+    vboxPushStack(ppTopOfStack, pEntry);
+}
+
+/**
+ * Pushes a string containing an error message and a VBox status code.
+ */
+static void vboxPushVBoxError(stack_t **ppTopOfStack, char const *pszString, int vrc)
+{
+    RTUTF16 wszTmp[128];
+    RTUtf16Printf(wszTmp, RT_ELEMENTS(wszTmp), "Error: %s! %Rrc", pszString, vrc);
+    vboxPushUtf16(ppTopOfStack, wszTmp);
+}
+
+
+static void vboxPushLastError(stack_t **ppTopOfStack, char const *pszString)
+{
+    DWORD const dwErr = GetLastError();
+    RTUTF16 wszTmp[128];
+    RTUtf16Printf(wszTmp, RT_ELEMENTS(wszTmp), "Error: %s! lasterr=%u (%#x)", pszString, dwErr, dwErr);
+    vboxPushUtf16(ppTopOfStack, wszTmp);
+}
+
+
+static void vboxPushLastErrorF(stack_t **ppTopOfStack, const char *pszFormat, ...)
+{
+    DWORD const dwErr = GetLastError();
+    RTUTF16 wszTmp[128];
+    va_list va;
+    va_start(va, pszFormat);
+    RTUtf16Printf(wszTmp, RT_ELEMENTS(wszTmp), "Error: %N! lasterr=%u (%#x)", pszFormat, &va, dwErr, dwErr);
+    va_end(va);
+    vboxPushUtf16(ppTopOfStack, wszTmp);
+}
+
+
+/**
+ * Convers a parameter to uint32_t.
+ *
+ * @returns IPRT status code.
+ * @param   pwsz                Will be trimmed.
+ * @param   puValue             Where to return the value.
+ */
+static int vboxUtf16ToUInt32(PRTUTF16 pwsz, uint32_t *puValue)
+{
+    *puValue = 0;
+
+    /* Trim the input: */
+    RTUTF16 wc;
+    while ((wc = *pwsz) == ' ' || wc == '\t')
+        pwsz++;
+    size_t cwc = RTUtf16Len(pwsz);
+    while (cwc > 0 && ((wc = pwsz[cwc - 1]) == ' ' || wc == '\t'))
+        pwsz[--cwc] = '\0';
+
+    /* Convert the remains into an UTF-8 string. */
+    char szValue[128];
+    char *pszValue = &szValue[0];
+    int rc = RTUtf16ToUtf8Ex(pwsz, cwc, &pszValue, sizeof(szValue), NULL);
+    if (RT_SUCCESS(rc))
+        rc = RTStrToUInt32Full(pszValue, 0, puValue);
     return rc;
 }
 
-static int vboxPopULong(PULONG pulValue)
-{
-    int rc = VINF_SUCCESS;
-
-    if (!g_stacktop || !*g_stacktop)
-    {
-        rc = VERR_NO_DATA;
-    }
-    else
-    {
-        stack_t *pStack = (*g_stacktop);
-        AssertPtr(pStack);
-
-        *pulValue = _tcstoul(pStack->text, NULL, 10 /* Base */);
-
-        *g_stacktop = pStack->next;
-        GlobalFree((HGLOBAL)pStack);
-    }
-
-    return rc;
-}
-
-static void vboxPushHResultAsString(HRESULT hr)
-{
-    TCHAR szErr[NSIS_MAX_STRLEN];
-    if (FAILED(hr))
-    {
-        if (FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, hr, 0, szErr, MAX_PATH, NULL))
-            szErr[MAX_PATH] = '\0';
-        else
-            StringCchPrintf(szErr, sizeof(szErr),
-                            _T("FormatMessage failed! Error = %ld"), GetLastError());
-    }
-    else
-        StringCchPrintf(szErr, sizeof(szErr), _T("0"));
-    pushstring(szErr);
-}
-
-static void vboxPushRcAsString(int rc)
-{
-    TCHAR szErr[NSIS_MAX_STRLEN];
-    if (RT_FAILURE(rc))
-    {
-        static const char s_szPrefix[] = "Error: ";
-        AssertCompile(NSIS_MAX_STRLEN > sizeof(s_szPrefix) + 32);
-#ifdef UNICODE
-        char szTmp[80];
-        memcpy(szTmp, s_szPrefix, sizeof(s_szPrefix));
-        RTErrQueryDefine(rc, &szTmp[sizeof(s_szPrefix) - 1], sizeof(szTmp) - sizeof(s_szPrefix) - 1, false);
-
-        RT_ZERO(szErr);
-        PRTUTF16 pwszDst = szErr;
-        RTStrToUtf16Ex(szTmp, RTSTR_MAX, &pwszDst, RT_ELEMENTS(szErr), NULL);
-#else
-        memcpy(szErr, s_szPrefix, sizeof(s_szPrefix));
-        RTErrQueryDefine(rc, &szErr[sizeof(s_szPrefix) - 1], sizeof(szErr) - sizeof(s_szPrefix) - 1, false);
-#endif
-    }
-    else
-    {
-        szErr[0] = '0';
-        szErr[1] = '\0';
-    }
-
-    pushstring(szErr);
-}
-
-/**
- * Connects to VBoxTray IPC under the behalf of the user running
- * in the current thread context.
+
+/**
+ * Connects to VBoxTray IPC under the behalf of the user running in the current
+ * thread context.
  *
  * @return  IPRT status code.
@@ -201 +231 @@
 /**
  * Retrieves a file's architecture (x86 or amd64).
+ *
  * Outputs "x86", "amd64" or an error message (if not found/invalid) on stack.
  *
@@ -207 +238 @@
  * @param   variables           The actual variable string.
  * @param   stacktop            Pointer to a pointer to the current stack.
- * @param   extra               Extra parameters. Currently unused.
- */
-VBOXINSTALLHELPER_EXPORT FileGetArchitecture(HWND hwndParent, int string_size, TCHAR *variables, stack_t **stacktop,
+ * @param   extra               Extra parameters.
+ */
+VBOXINSTALLHELPER_EXPORT FileGetArchitecture(HWND hwndParent, int string_size, WCHAR *variables, stack_t **stacktop,
                                              extra_parameters *extra)
 {
-    RT_NOREF(hwndParent, extra);
-
-    EXDLL_INIT();
-
-    TCHAR szFile[MAX_PATH + 1];
-    int rc = vboxPopString(szFile, sizeof(szFile) / sizeof(TCHAR));
-    if (RT_SUCCESS(rc))
+    RT_NOREF(hwndParent, string_size, variables, extra);
+
+    stack_t *pEntry = vboxPopStack(stacktop);
+    if (pEntry)
     {
-#ifdef UNICODE
         char *pszFileUtf8;
-        rc = RTUtf16ToUtf8(szFile, &pszFileUtf8);
+        int rc = RTUtf16ToUtf8(pEntry->text, &pszFileUtf8);
         if (RT_SUCCESS(rc))
         {
-#else
-            char *pszFileUtf8 = szFile;
-#endif
             RTLDRMOD hLdrMod;
             rc = RTLdrOpen(pszFileUtf8, RTLDR_O_FOR_VALIDATION, RTLDRARCH_WHATEVER, &hLdrMod);
@@ -238 +262 @@
                     {
                         case RTLDRARCH_X86_32:
-                            pushstring(_T("x86"));
+                            VBOX_PUSH_STRING_LITERAL(stacktop, "x86");
                             break;
 
                         case RTLDRARCH_AMD64:
-                            pushstring(_T("amd64"));
+                            VBOX_PUSH_STRING_LITERAL(stacktop, "amd64");
                             break;
 
                         default:
-                            pushstring(_T("Error: Unknown / invalid architecture"));
+                            VBOX_PUSH_STRING_LITERAL(stacktop, "Error: Unknown / invalid architecture");
                             break;
                     }
                 }
                 else
-                    pushstring(_T("Error: Unknown / invalid PE signature"));
+                    VBOX_PUSH_STRING_LITERAL(stacktop, "Error: Unknown / invalid PE signature");
 
                 RTLdrClose(hLdrMod);
             }
             else
-                pushstring(_T("Error: Could not open file"));
-#ifdef UNICODE
+                vboxPushVBoxError(stacktop, "RTLdrOpen failed", rc);
             RTStrFree(pszFileUtf8);
         }
-#endif
+        else
+            vboxPushVBoxError(stacktop, "RTUtf16ToUtf8 failed", rc);
     }
     else
-        pushstring(_T("Error: Could not retrieve file name"));
+        VBOX_PUSH_STRING_LITERAL(stacktop, "Error: Could not retrieve file name");
+    vboxFreeStackEntry(pEntry);
 }
 
 /**
  * Retrieves a file's vendor.
+ *
  * Outputs the vendor's name or an error message (if not found/invalid) on stack.
  *
@@ -274 +300 @@
  * @param   variables           The actual variable string.
  * @param   stacktop            Pointer to a pointer to the current stack.
- * @param   extra               Extra parameters. Currently unused.
- */
-VBOXINSTALLHELPER_EXPORT FileGetVendor(HWND hwndParent, int string_size, TCHAR *variables, stack_t **stacktop,
+ * @param   extra               Extra parameters.
+ */
+VBOXINSTALLHELPER_EXPORT FileGetVendor(HWND hwndParent, int string_size, WCHAR *variables, stack_t **stacktop,
                                        extra_parameters *extra)
 {
-    RT_NOREF(hwndParent, extra);
-
-    EXDLL_INIT();
-
-    TCHAR szFile[MAX_PATH + 1];
-    int rc = vboxPopString(szFile, sizeof(szFile) / sizeof(TCHAR));
-    if (RT_SUCCESS(rc))
+    RT_NOREF(hwndParent, string_size, variables, extra);
+
+    stack_t *pEntry = vboxPopStack(stacktop);
+    if (pEntry)
     {
-        DWORD dwInfoSize = GetFileVersionInfoSize(szFile, NULL /* lpdwHandle */);
+        DWORD dwInfoSize = GetFileVersionInfoSizeW(pEntry->text, NULL /* lpdwHandle */);
         if (dwInfoSize)
         {
-            void *pFileInfo = GlobalAlloc(GMEM_FIXED, dwInfoSize);
-            if (pFileInfo)
+            void *pvFileInfo = GlobalAlloc(GMEM_FIXED, dwInfoSize);
+            if (pvFileInfo)
             {
-                if (GetFileVersionInfo(szFile, 0, dwInfoSize, pFileInfo))
+                if (GetFileVersionInfoW(pEntry->text, 0, dwInfoSize, pvFileInfo))
                 {
                     LPVOID pvInfo;
-                    UINT puInfoLen;
-                    if (VerQueryValue(pFileInfo, _T("\\VarFileInfo\\Translation"),
-                                      &pvInfo, &puInfoLen))
+                    UINT   cbInfo;
+                    if (VerQueryValueW(pvFileInfo, L"\\VarFileInfo\\Translation", &pvInfo, &cbInfo))
                     {
-                        WORD wCodePage = LOWORD(*(DWORD*)pvInfo);
-                        WORD wLanguageID = HIWORD(*(DWORD*)pvInfo);
-
-                        TCHAR szQuery[MAX_PATH];
-                        StringCchPrintf(szQuery, sizeof(szQuery), _T("StringFileInfo\\%04X%04X\\CompanyName"),
-                                        wCodePage, wLanguageID);
-
-                        LPCTSTR pcData;
-                        if (VerQueryValue(pFileInfo, szQuery, (void**)&pcData, &puInfoLen))
-                        {
-                            pushstring(pcData);
-                        }
+                        WORD wCodePage   = LOWORD(*(DWORD const *)pvInfo);
+                        WORD wLanguageID = HIWORD(*(DWORD const *)pvInfo);
+
+                        WCHAR wszQuery[80];
+                        RTUtf16Printf(wszQuery, RT_ELEMENTS(wszQuery), "StringFileInfo\\%04X%04X\\CompanyName",
+                                      wCodePage, wLanguageID);
+
+                        LPCWSTR pwszData;
+                        if (VerQueryValueW(pvFileInfo, wszQuery, (void **)&pwszData, &cbInfo))
+                            vboxPushUtf16(stacktop, pwszData);
                         else
-                            rc = VERR_NOT_FOUND;
+                            vboxPushLastErrorF(stacktop, "VerQueryValueW '%ls' failed", wszQuery);
                     }
                     else
-                        rc = VERR_NOT_FOUND;
+                        vboxPushLastError(stacktop, "VerQueryValueW '\\VarFileInfo\\Translation' failed");
                 }
-                GlobalFree(pFileInfo);
+                else
+                    vboxPushLastError(stacktop, "GetFileVersionInfo failed");
+                GlobalFree(pvFileInfo);
             }
             else
-                rc = VERR_NO_MEMORY;
+                VBOX_PUSH_STRING_LITERAL(stacktop, "Error: GlobalAlloc failed");
         }
         else
-            rc = VERR_NOT_FOUND;
+            vboxPushLastError(stacktop, "GetFileVersionInfoSizeW failed");
     }
-
-    if (RT_FAILURE(rc))
-        vboxPushRcAsString(rc);
+    else
+        VBOX_PUSH_STRING_LITERAL(stacktop, "Error: Could not retrieve file name");
+    vboxFreeStackEntry(pEntry);
 }
 
@@ -339 +361 @@
  * @param   variables           The actual variable string.
  * @param   stacktop            Pointer to a pointer to the current stack.
- * @param   extra               Extra parameters. Currently unused.
- */
-VBOXINSTALLHELPER_EXPORT VBoxTrayShowBallonMsg(HWND hwndParent, int string_size, TCHAR *variables, stack_t **stacktop,
+ * @param   extra               Extra parameters.
+ */
+VBOXINSTALLHELPER_EXPORT VBoxTrayShowBallonMsg(HWND hwndParent, int string_size, WCHAR *variables, stack_t **stacktop,
                                                extra_parameters *extra)
 {
-    RT_NOREF(hwndParent, extra);
-
-    EXDLL_INIT();
-
-    TCHAR szMsg[256];
-    TCHAR szTitle[128];
-    int rc = vboxPopString(szMsg, sizeof(szMsg) / sizeof(TCHAR));
-    if (RT_SUCCESS(rc))
-        rc = vboxPopString(szTitle, sizeof(szTitle) / sizeof(TCHAR));
-
-    /** @todo Do we need to restore the stack on failure? */
-
-    if (RT_SUCCESS(rc))
+    RT_NOREF(hwndParent, string_size, variables, extra);
+
+    /*
+     * Get parameters from the stack.
+     */
+    stack_t * const pMsgEntry     = vboxPopStack(stacktop);
+    stack_t * const pTitleEntry   = vboxPopStack(stacktop);
+    stack_t * const pTypeEntry    = vboxPopStack(stacktop);
+    stack_t * const pTimeoutEntry = vboxPopStack(stacktop);
+    if (pTimeoutEntry)
     {
-        RTR3InitDll(0);
-
-#ifdef UNICODE
-        char *pszMsgUtf8   = NULL;
-        char *pszTitleUtf8 = NULL;
-        rc = RTUtf16ToUtf8(szMsg, &pszMsgUtf8);
-        if (RT_SUCCESS(rc))
-            rc = RTUtf16ToUtf8(szTitle, &pszTitleUtf8);
-#else
-        char *pszMsgUtf8   = szMsg;
-        char *pszTitleUtf8 = szTitle;
-#endif
-        if (RT_SUCCESS(rc))
+        /*
+         * Allocate an IPC message payload structure of the right size.
+         */
+        size_t const cchMsg     = RTUtf16CalcUtf8Len(pMsgEntry->text);
+        size_t const cchTitle   = RTUtf16CalcUtf8Len(pTitleEntry->text);
+        size_t const cbPayload  = RT_UOFFSETOF_DYN(VBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T, szzStrings[cchMsg + 1 + cchTitle + 1]);
+        PVBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T pPayload = (PVBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T)RTMemAllocZVar(cbPayload);
+        if (pPayload)
         {
-            /* We use UTF-8 for the IPC data. */
-            uint32_t cbMsg = sizeof(VBOXTRAYIPCMSG_SHOWBALLOONMSG)
-                           + (uint32_t)strlen(pszMsgUtf8)   + 1  /* Include terminating zero */
-                           + (uint32_t)strlen(pszTitleUtf8) + 1; /* Ditto. */
-            Assert(cbMsg);
-            PVBOXTRAYIPCMSG_SHOWBALLOONMSG pIpcMsg = (PVBOXTRAYIPCMSG_SHOWBALLOONMSG)RTMemAlloc(cbMsg);
-            if (pIpcMsg)
+            VBOXTRAYIPCHEADER const MsgHdr =
             {
-                /* Stuff in the strings. */
-                memcpy(pIpcMsg->szMsgContent, pszMsgUtf8,   strlen(pszMsgUtf8)   + 1);
-                memcpy(pIpcMsg->szMsgTitle,   pszTitleUtf8, strlen(pszTitleUtf8) + 1);
-
-                /* Pop off the values in reverse order from the stack. */
-                rc = vboxPopULong((ULONG *)&pIpcMsg->uType);
-                if (RT_SUCCESS(rc))
-                    rc = vboxPopULong((ULONG *)&pIpcMsg->uShowMS);
-
+                VBOXTRAY_IPC_HDR_MAGIC,
+                VBOXTRAY_IPC_HDR_VERSION,
+                VBOXTRAYIPCMSGTYPE_SHOW_BALLOON_MSG,
+                cbPayload
+            };
+
+            /*
+             * Convert the parametes and put them into the payload structure.
+             */
+            pPayload->cchMsg   = cchMsg;
+            pPayload->cchTitle = cchTitle;
+            char *psz = &pPayload->szzStrings[0];
+            int rc = RTUtf16ToUtf8Ex(pMsgEntry->text, RTSTR_MAX, &psz, cchMsg + 1, NULL);
+            if (RT_SUCCESS(rc))
+            {
+                psz = &pPayload->szzStrings[cchMsg + 1];
+                rc = RTUtf16ToUtf8Ex(pTitleEntry->text, RTSTR_MAX, &psz, cchTitle + 1, NULL);
                 if (RT_SUCCESS(rc))
                 {
-                    RTLOCALIPCSESSION hSession = 0;
-                    rc = vboxConnectToVBoxTray(&hSession);
+                    rc = vboxUtf16ToUInt32(pTypeEntry->text, &pPayload->uType);
                     if (RT_SUCCESS(rc))
                     {
-                        VBOXTRAYIPCHEADER ipcHdr = { VBOXTRAY_IPC_HDR_MAGIC, 0 /* Header version */,
-                                                     VBOXTRAYIPCMSGTYPE_SHOWBALLOONMSG, cbMsg };
-
-                        rc = RTLocalIpcSessionWrite(hSession, &ipcHdr, sizeof(ipcHdr));
+                        rc = vboxUtf16ToUInt32(pTypeEntry->text, &pPayload->cMsTimeout);
                         if (RT_SUCCESS(rc))
-                            rc = RTLocalIpcSessionWrite(hSession, pIpcMsg, cbMsg);
-
-                        int rc2 = RTLocalIpcSessionClose(hSession);
-                        if (RT_SUCCESS(rc))
-                            rc = rc2;
+                        {
+                            /*
+                             * Connect to VBoxTray and send the message.
+                             */
+                            RTLOCALIPCSESSION hSession = 0;
+                            rc = vboxConnectToVBoxTray(&hSession);
+                            if (RT_SUCCESS(rc))
+                            {
+                                rc = RTLocalIpcSessionWrite(hSession, &MsgHdr, sizeof(MsgHdr));
+                                if (RT_SUCCESS(rc))
+                                {
+                                    rc = RTLocalIpcSessionWrite(hSession, pPayload, cbPayload);
+                                    if (RT_FAILURE(rc))
+                                        vboxPushVBoxError(stacktop, "Failed to write message payload", rc);
+                                }
+                                else
+                                    vboxPushVBoxError(stacktop, "Failed to write message header", rc);
+
+                                int rc2 = RTLocalIpcSessionClose(hSession);
+                                if (RT_FAILURE(rc2) && RT_SUCCESS(rc))
+                                {
+                                    vboxPushVBoxError(stacktop, "RTLocalIpcSessionClose failed", rc);
+                                    rc = rc2;
+                                }
+                            }
+                            else
+                                vboxPushVBoxError(stacktop, "vboxConnectToVBoxTray failed", rc);
+                        }
+                        else
+                            vboxPushVBoxError(stacktop, "MyUtf16ToUInt32 failed on the timeout value", rc);
                     }
+                    else
+                        vboxPushVBoxError(stacktop, "MyUtf16ToUInt32 failed on the type value", rc);
                 }
-
-                RTMemFree(pIpcMsg);
+                else
+                    vboxPushVBoxError(stacktop, "RTUtf16ToUtf8Ex failed on the title text", rc);
             }
             else
-                rc = VERR_NO_MEMORY;
-#ifdef UNICODE
-            RTStrFree(pszMsgUtf8);
-            RTStrFree(pszTitleUtf8);
-#endif
+                vboxPushVBoxError(stacktop, "RTUtf16ToUtf8Ex failed on the message text", rc);
         }
+        else
+            VBOX_PUSH_STRING_LITERAL(stacktop, "Error: Out of memory!");
     }
-
-    vboxPushRcAsString(rc);
+    else
+        VBOX_PUSH_STRING_LITERAL(stacktop, "Error: Too few parameters on the stack!");
+    vboxFreeStackEntry(pTimeoutEntry);
+    vboxFreeStackEntry(pTypeEntry);
+    vboxFreeStackEntry(pTitleEntry);
+    vboxFreeStackEntry(pMsgEntry);
 }
 

trunk/src/VBox/Additions/WINNT/Installer/InstallHelper/exdll.h

@@ -61 +61 @@
 #endif
 
+#ifndef VBOX
 // only include this file from one place in your DLL.
 // (it is all static, if you use it in two places it will fail)
@@ -68 +69 @@
         g_stacktop=stacktop;      \
         g_variables=variables; }
+#endif /* !VBOX */
 
 typedef struct _stack_t
@@ -104 +106 @@
 } extra_parameters;
 
+#ifndef VBOX
 static unsigned int g_stringsize;
 static stack_t **g_stacktop;
 static TCHAR *g_variables;
+#endif
 
 enum
@@ -137 +141 @@
 __INST_LAST
 };
+
+#ifndef VBOX
 
 // utility functions (not required but often useful)
@@ -173 +179 @@
         lstrcpy(g_variables + varnum*g_stringsize, var);
 }
+
+#endif /* ! VBOX */
+
 #endif /* !GA_INCLUDED_SRC_WINNT_Installer_InstallHelper_exdll_h */

trunk/src/VBox/Additions/WINNT/Installer/InstallHelper/testcase/tstWinAdditionsInstallHelper.cpp

@@ -31 +31 @@
 *********************************************************************************************************************************/
 #include <iprt/win/windows.h>
-
-#include <tchar.h>
-
-#pragma warning(push)
-#pragma warning(disable: 4995) /* warning C4995: 'lstrcpyA': name was marked as #pragma deprecated */
 #include "../exdll.h"
-#pragma warning(pop)
 
 #include <iprt/assert.h>
@@ -49 +43 @@
 
 
-RT_C_DECLS_BEGIN
-
-/** A generic NSIS plugin function. */
-typedef void __cdecl NSIS_PLUGIN_FUNC(HWND hwndParent, int string_size, TCHAR *variables, stack_t **stacktop, extra_parameters *extra);
-/** Pointer to a generic NSIS plugin function. */
-typedef NSIS_PLUGIN_FUNC *PNSIS_PLUGIN_FUNC;
-
-RT_C_DECLS_END
-
+/*********************************************************************************************************************************
+*   Defined Constants And Macros                                                                                                 *
+*********************************************************************************************************************************/
 /** Symbol names to test. */
 #define TST_FILEGETARCHITECTURE_NAME   "FileGetArchitecture"
@@ -64 +52 @@
 
 
+/*********************************************************************************************************************************
+*   Structures and Typedefs                                                                                                      *
+*********************************************************************************************************************************/
+/** A generic NSIS plugin function. */
+typedef void __cdecl NSIS_PLUGIN_FUNC(HWND hwndParent, int string_size, WCHAR *variables, stack_t **stacktop, extra_parameters *extra);
+/** Pointer to a generic NSIS plugin function. */
+typedef NSIS_PLUGIN_FUNC *PNSIS_PLUGIN_FUNC;
+
+
+
 /**
  * Destroys a stack.
@@ -84 +82 @@
  * @returns VBox status code.
  * @param   ppStackTop          Stack to push string to.
- * @param   pcszString          String to push to the stack.
- */
-static int tstStackPushString(stack_t **ppStackTop, const TCHAR *pcszString)
-{
-    int rc = VINF_SUCCESS;
-
-    /* Termination space is part of stack_t. */
-    stack_t *pStack = (stack_t *)GlobalAlloc(GPTR, sizeof(stack_t) + g_stringsize);
+ * @param   pwszString          String to push to the stack.
+ */
+static int tstStackPushString(stack_t **ppStackTop, const WCHAR *pwszString)
+{
+    size_t cwcString = RTUtf16Len(pwszString);
+    stack_t *pStack = (stack_t *)GlobalAlloc(GPTR, RT_UOFFSETOF_DYN(stack_t, text[cwcString + 1]));
     if (pStack)
     {
-        lstrcpyn(pStack->text, pcszString, (int)g_stringsize);
+        AssertCompile(sizeof(pStack->text[0]) == sizeof(*pwszString));
+        memcpy(pStack->text, pwszString, (cwcString + 1) * sizeof(pStack->text[0]));
         pStack->next = ppStackTop ? *ppStackTop : NULL;
 
         *ppStackTop = pStack;
-    }
-    else
-        rc = VERR_NO_MEMORY;
-
-    return rc;
+        return VINF_SUCCESS;
+    }
+    return VERR_NO_MEMORY;
 }
 
@@ -108 +103 @@
  * Pops a string off a stack.
  *
- * @returns Allocated string popped off the stack, or NULL on error / empty stack.
- * @param   ppStackTop          Stack to pop off string from.
- */
-static int tstStackPopString(stack_t **ppStackTop, TCHAR *pszStr, size_t cchStr)
+ * @returns IPRT status code.
+ * @param   ppStackTop      Stack to pop off string from.
+ * @param   pwszDst         Where to return the string.
+ * @param   cwcDst          The size of the destination buffer.
+ */
+static int tstStackPopString(stack_t **ppStackTop, WCHAR *pwszDst, size_t cwcDst)
 {
     stack_t *pStack = *ppStackTop;
-    lstrcpyn(pszStr, pStack->text, cchStr);
-    *ppStackTop = pStack->next;
-    GlobalFree((HGLOBAL)pStack);
-
-    return VINF_SUCCESS;
-}
-
-int main()
+    if (pStack)
+    {
+        int rc = RTUtf16Copy(pwszDst, cwcDst, pStack->text);
+
+        *ppStackTop = pStack->next;
+        GlobalFree((HGLOBAL)pStack);
+
+        return rc;
+    }
+    return VERR_NOT_FOUND;
+}
+
+int main(int argc, char **argv)
 {
     RTTEST hTest;
-    RTEXITCODE rcExit = RTTestInitAndCreate("tstWinAdditionsInstallHelper", &hTest);
+    RTEXITCODE rcExit = RTTestInitExAndCreate(argc, &argv, 0, "tstWinAdditionsInstallHelper", &hTest);
     if (rcExit != RTEXITCODE_SUCCESS)
         return rcExit;
@@ -130 +132 @@
 
     char szGuestInstallHelperDll[RTPATH_MAX];
-    RTProcGetExecutablePath(szGuestInstallHelperDll, sizeof(szGuestInstallHelperDll));
-
-    /* Set the default string size. */
-    g_stringsize = NSIS_MAX_STRLEN;
+    RTPathExecDir(szGuestInstallHelperDll, sizeof(szGuestInstallHelperDll));
 
     /** @todo This ASSUMES that this testcase always is located in the separate "bin/additions" sub directory
@@ -139 +138 @@
      *        Might need some more tweaking ... */
     int rc = RTPathAppend(szGuestInstallHelperDll, sizeof(szGuestInstallHelperDll),
-                          "../../../repack/resources/VBoxGuestInstallHelper/VBoxGuestInstallHelper.dll");
+                          "..\\..\\repackadd\\resources\\VBoxGuestInstallHelper\\VBoxGuestInstallHelper.dll");
     if (RT_SUCCESS(rc))
     {
         RTTestIPrintf(RTTESTLVL_ALWAYS, "Using DLL: %s\n", szGuestInstallHelperDll);
-
-#ifdef UNICODE
-        const bool fUnicode = true;
-#else
-        const bool fUnicode = false;
-#endif
-        RTTestIPrintf(RTTESTLVL_ALWAYS, "Running %s build\n", fUnicode ? "UNICODE" : "ANSI");
 
         RTLDRMOD hLdrMod;
@@ -155 +147 @@
         if (RT_SUCCESS(rc))
         {
-            TCHAR szVars[NSIS_MAX_STRLEN * sizeof(TCHAR)] = { 0 };
+            WCHAR wszVars[NSIS_MAX_STRLEN * sizeof(WCHAR)] = { 0 };
+            WCHAR wszResult[NSIS_MAX_STRLEN];
 
             /*
@@ -165 +158 @@
             {
                 stack_t *pStack = NULL;
-                tstStackPushString(&pStack, _T("c:\\windows\\system32\\kernel32.dll"));
-
-                pfnFileGetArchitecture(NULL /* hWnd */, NSIS_MAX_STRLEN, szVars, &pStack, NULL /* extra */);
-
-                TCHAR szStack[NSIS_MAX_STRLEN];
-                rc = tstStackPopString(&pStack, szStack, sizeof(szStack));
+                tstStackPushString(&pStack, L"c:\\windows\\system32\\kernel32.dll");
+
+                pfnFileGetArchitecture(NULL /* hWnd */, NSIS_MAX_STRLEN, wszVars, &pStack, NULL /* extra */);
+
+                rc = tstStackPopString(&pStack, wszResult, sizeof(wszResult));
                 if (   RT_SUCCESS(rc)
-                    && (   !_tcscmp(szStack, _T("x86"))
-                        || !_tcscmp(szStack, _T("amd64"))))
-                {
-                    if (fUnicode)
-                        RTTestIPrintf(RTTESTLVL_ALWAYS, "Arch: %ls\n", szStack);
-                    else
-                        RTTestIPrintf(RTTESTLVL_ALWAYS, "Arch: %s\n", szStack);
-                }
+                    && (   RTUtf16CmpAscii(wszResult, "x86") == 0
+                        || RTUtf16CmpAscii(wszResult, "amd64") == 0))
+                    RTTestIPrintf(RTTESTLVL_ALWAYS, "Arch: %ls\n", wszResult);
                 else
-                    RTTestIFailed("Getting file arch failed (NSIS API changed?)\n");
+                    RTTestIFailed("Getting file arch on kernel32 failed: %Rrc - '%ls', expected 'x86' or 'amd64'", rc, wszResult);
+
+                if (pStack)
+                    RTTestIFailed("Too many items on the stack!");
                 tstStackDestroy(pStack);
             }
@@ -191 +181 @@
              */
             PNSIS_PLUGIN_FUNC pfnFileGetVendor;
-            rc = RTLdrGetSymbol(hLdrMod, TST_FILEGETVENDOR_NAME, (void**)&pfnFileGetVendor);
+            rc = RTLdrGetSymbol(hLdrMod, TST_FILEGETVENDOR_NAME, (void **)&pfnFileGetVendor);
             if (RT_SUCCESS(rc))
             {
                 stack_t *pStack = NULL;
-                tstStackPushString(&pStack, _T("c:\\windows\\system32\\kernel32.dll"));
-
-                pfnFileGetVendor(NULL /* hWnd */, NSIS_MAX_STRLEN, szVars, &pStack, NULL /* extra */);
-
-                TCHAR szStack[NSIS_MAX_STRLEN];
-                rc = tstStackPopString(&pStack, szStack, RT_ELEMENTS(szStack));
+                tstStackPushString(&pStack, L"c:\\windows\\system32\\kernel32.dll");
+
+                pfnFileGetVendor(NULL /* hWnd */, NSIS_MAX_STRLEN, wszVars, &pStack, NULL /* extra */);
+
+                rc = tstStackPopString(&pStack, wszResult, RT_ELEMENTS(wszResult));
                 if (   RT_SUCCESS(rc)
-                    && !_tcscmp(szStack, _T("Microsoft Corporation")))
-                {
-                    if (fUnicode)
-                        RTTestIPrintf(RTTESTLVL_ALWAYS, "Vendor: %ls\n", szStack);
-                    else
-                        RTTestIPrintf(RTTESTLVL_ALWAYS, "Vendor: %s\n", szStack);
-                }
+                    && RTUtf16CmpAscii(wszResult, "Microsoft Corporation") == 0)
+                    RTTestIPrintf(RTTESTLVL_ALWAYS, "Vendor: %ls\n", wszResult);
                 else
-                    RTTestIFailed("Getting file vendor failed (NSIS API changed?)\n");
+                    RTTestIFailed("Getting file vendor failed: %Rrc - '%ls', expected 'Microsoft Corporation'\n", rc, wszResult);
+
+                if (pStack)
+                    RTTestIFailed("Too many items on the stack!");
                 tstStackDestroy(pStack);
             }
@@ -225 +212 @@
                 stack_t *pStack = NULL;
                 /* Push stuff in reverse order to the stack. */
-                tstStackPushString(&pStack, _T("5000") /* Time to show in ms */);
-                tstStackPushString(&pStack, _T("1") /* Type */);
-                tstStackPushString(&pStack, _T("This is a message from tstWinAdditionsInstallHelper!"));
-                tstStackPushString(&pStack, _T("This is a title from tstWinAdditionsInstallHelper!"));
-
-                pfnVBoxTrayShowBallonMsg(NULL /* hWnd */, NSIS_MAX_STRLEN, szVars, &pStack, NULL /* extra */);
-
-                TCHAR szStack[NSIS_MAX_STRLEN];
-                rc = tstStackPopString(&pStack, szStack, RT_ELEMENTS(szStack));
+                tstStackPushString(&pStack, L"5000" /* Time to show in ms */);
+                tstStackPushString(&pStack, L"1" /* Type - info */);
+                tstStackPushString(&pStack, L"This is a message from tstWinAdditionsInstallHelper!");
+                tstStackPushString(&pStack, L"This is a title from tstWinAdditionsInstallHelper!");
+
+                pfnVBoxTrayShowBallonMsg(NULL /* hWnd */, NSIS_MAX_STRLEN, wszVars, &pStack, NULL /* extra */);
+
+                rc = tstStackPopString(&pStack, wszResult, RT_ELEMENTS(wszResult));
                 if (RT_SUCCESS(rc))
-                {
-                    if (fUnicode)
-                        RTTestIPrintf(RTTESTLVL_ALWAYS, "Reply: %ls\n", szStack);
-                    else
-                        RTTestIPrintf(RTTESTLVL_ALWAYS, "Reply: %s\n", szStack);
-                }
+                    RTTestIPrintf(RTTESTLVL_ALWAYS, "Reply: '%ls'\n", wszResult);
                 else
-                    RTTestIFailed("Sending message to VBoxTray failed (NSIS API changed?)\n");
+                    RTTestIFailed("Sending message to VBoxTray failed: stack pop error - %Rrc", rc);
+
+                if (pStack)
+                    RTTestIFailed("Too many items on the stack!");
                 tstStackDestroy(pStack);
             }

trunk/src/VBox/Additions/WINNT/Installer/VBoxGuestAdditionsLog.nsh

@@ -73 +73 @@
     ; - String: Description / Body
     ; - String: Title / Name of application
-    ; - Integer: Type of message: 0 (Info), 1 (Warning), 2 (Error)
+    ; - Integer: Type of message: 1 (Info), 2 (Warning), 3 (Error)
     ; - Integer: Time (in msec) to show the notification
     VBoxGuestInstallHelper::VBoxTrayShowBallonMsg "${text}" "VirtualBox Guest Additions Setup" ${type} 5000

trunk/src/VBox/Additions/WINNT/VBoxTray/VBoxIPC.cpp

@@ -107 +107 @@
 
 
+/**
+ * Handles VBOXTRAYIPCMSGTYPE_RESTART.
+ */
 static int vboxIPCHandleVBoxTrayRestart(PVBOXIPCSESSION pSession, PVBOXTRAYIPCHEADER pHdr)
 {
-    AssertPtrReturn(pSession, VERR_INVALID_POINTER);
-    AssertPtrReturn(pHdr, VERR_INVALID_POINTER);
+    RT_NOREF(pSession, pHdr);
 
     /** @todo Not implemented yet; don't return an error here. */
@@ -116 +118 @@
 }
 
+/**
+ * Handles VBOXTRAYIPCMSGTYPE_SHOW_BALLOON_MSG.
+ */
 static int vboxIPCHandleShowBalloonMsg(PVBOXIPCSESSION pSession, PVBOXTRAYIPCHEADER pHdr)
 {
-    AssertPtrReturn(pSession, VERR_INVALID_POINTER);
-    AssertPtrReturn(pHdr, VERR_INVALID_POINTER);
-    AssertReturn(pHdr->uMsgLen > 0, VERR_INVALID_PARAMETER);
-
-    VBOXTRAYIPCMSG_SHOWBALLOONMSG ipcMsg;
-    int rc = RTLocalIpcSessionRead(pSession->hSession, &ipcMsg, pHdr->uMsgLen,
-                                   NULL /* Exact read, blocking */);
-    if (RT_SUCCESS(rc))
-    {
-        /* Showing the balloon tooltip is not critical. */
-        int rc2 = hlpShowBalloonTip(g_hInstance, g_hwndToolWindow, ID_TRAYICON,
-                                    ipcMsg.szMsgContent, ipcMsg.szMsgTitle,
-                                    ipcMsg.uShowMS, ipcMsg.uType);
-        LogFlowFunc(("Showing \"%s\" - \"%s\" (type %RU32, %RU32ms), rc=%Rrc\n",
-                     ipcMsg.szMsgTitle, ipcMsg.szMsgContent,
-                     ipcMsg.uType, ipcMsg.uShowMS, rc2));
-        NOREF(rc2);
-    }
-
-    return rc;
-}
-
+    /*
+     * Unmarshal and validate the data.
+     */
+    union
+    {
+        uint8_t                             abBuf[_4K];
+        VBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T   s;
+    } Payload;
+    AssertReturn(pHdr->cbPayload >= RT_UOFFSETOF_DYN(VBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T, szzStrings[2]), VERR_INVALID_PARAMETER);
+    AssertReturn(pHdr->cbPayload < sizeof(Payload), VERR_BUFFER_OVERFLOW);
+
+    int rc = RTLocalIpcSessionRead(pSession->hSession, &Payload, pHdr->cbPayload, NULL /*pcbRead - exact, blocking*/);
+    if (RT_FAILURE(rc))
+        return rc;
+
+    /* String lengths: */
+    AssertReturn(   Payload.s.cchMsg + 1 + Payload.s.cchTitle + 1 + RT_UOFFSETOF(VBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T, szzStrings)
+                 <= pHdr->cbPayload, VERR_INVALID_PARAMETER);
+
+    /* Message text: */
+    const char *pszMsg   = Payload.s.szzStrings;
+    rc = RTStrValidateEncodingEx(pszMsg, Payload.s.cchMsg + 1,
+                                 RTSTR_VALIDATE_ENCODING_EXACT_LENGTH | RTSTR_VALIDATE_ENCODING_ZERO_TERMINATED);
+    AssertRCReturn(rc, rc);
+
+    /* Title text: */
+    const char *pszTitle = &Payload.s.szzStrings[Payload.s.cchMsg + 1];
+    rc = RTStrValidateEncodingEx(pszMsg, Payload.s.cchTitle + 1,
+                                 RTSTR_VALIDATE_ENCODING_EXACT_LENGTH | RTSTR_VALIDATE_ENCODING_ZERO_TERMINATED);
+    AssertRCReturn(rc, rc);
+
+    /* Type/dwInfoFlags: */
+    AssertReturn(   Payload.s.uType == NIIF_NONE
+                 || Payload.s.uType == NIIF_INFO
+                 || Payload.s.uType == NIIF_WARNING
+                 || Payload.s.uType == NIIF_ERROR,
+                 VERR_WRONG_TYPE);
+
+    /* Timeout: */
+    if (!Payload.s.cMsTimeout)
+        Payload.s.cMsTimeout = RT_MS_5SEC;
+    AssertStmt(Payload.s.cMsTimeout >= RT_MS_1SEC, Payload.s.cMsTimeout = RT_MS_1SEC);
+    AssertStmt(Payload.s.cMsTimeout <= RT_MS_1MIN, Payload.s.cMsTimeout = RT_MS_1MIN);
+
+    /*
+     * Showing the balloon tooltip is not critical.
+     */
+    int rc2 = hlpShowBalloonTip(g_hInstance, g_hwndToolWindow, ID_TRAYICON,
+                                pszMsg, pszTitle, Payload.s.cMsTimeout, Payload.s.uType);
+    LogFlowFunc(("Showing \"%s\" - \"%s\" (type %RU32, %RU32ms), rc=%Rrc\n",
+                 pszTitle, pszMsg, Payload.s.cMsTimeout, Payload.s.uType, rc2));
+    RT_NOREF_PV(rc2);
+
+    return VINF_SUCCESS;
+}
+
+/**
+ * Handles VBOXTRAYIPCMSGTYPE_USER_LAST_INPUT.
+ */
 static int vboxIPCHandleUserLastInput(PVBOXIPCSESSION pSession, PVBOXTRAYIPCHEADER pHdr)
 {
-    AssertPtrReturn(pSession, VERR_INVALID_POINTER);
-    AssertPtrReturn(pHdr, VERR_INVALID_POINTER);
-    /* No actual message from client. */
+    RT_NOREF(pHdr);
 
     int rc = VINF_SUCCESS;
-
-    bool fLastInputAvailable = false;
-    VBOXTRAYIPCRES_USERLASTINPUT ipcRes;
+    VBOXTRAYIPCREPLY_USER_LAST_INPUT_T Reply = { UINT32_MAX };
     if (g_pfnGetLastInputInfo)
     {
-        /* Note: This only works up to 49.7 days (= 2^32, 32-bit counter)
-           since Windows was started. */
-        LASTINPUTINFO lastInput;
-        lastInput.cbSize = sizeof(LASTINPUTINFO);
-        BOOL fRc = g_pfnGetLastInputInfo(&lastInput);
-        if (fRc)
-        {
-            ipcRes.uLastInput = (GetTickCount() - lastInput.dwTime) / 1000;
-            fLastInputAvailable = true;
-        }
+        /* Note: This only works up to 49.7 days (= 2^32, 32-bit counter) since Windows was started. */
+        LASTINPUTINFO LastInput;
+        LastInput.cbSize = sizeof(LastInput);
+        if (g_pfnGetLastInputInfo(&LastInput))
+            Reply.cSecSinceLastInput = (GetTickCount() - LastInput.dwTime) / 1000;
         else
             rc = RTErrConvertFromWin32(GetLastError());
     }
 
-    if (!fLastInputAvailable)
-    {
-        /* No last input available. */
-        ipcRes.uLastInput = UINT32_MAX;
-    }
-
-    int rc2 = RTLocalIpcSessionWrite(pSession->hSession, &ipcRes, sizeof(ipcRes));
+    int rc2 = RTLocalIpcSessionWrite(pSession->hSession, &Reply, sizeof(Reply));
     if (RT_SUCCESS(rc))
         rc = rc2;
@@ -355 +382 @@
         /* The next call will be cancelled via VBoxIPCStop if needed. */
         rc = RTLocalIpcSessionWaitForData(hSession, RT_INDEFINITE_WAIT);
-        if (RT_FAILURE(rc))
-        {
-            if (rc == VERR_CANCELLED)
-            {
-                LogFlowFunc(("Session %p: Waiting for data cancelled\n", pThis));
-                rc = VINF_SUCCESS;
+        if (RT_SUCCESS(rc))
+        {
+            /*
+             * Read the message header.
+             */
+            VBOXTRAYIPCHEADER Hdr = {0};
+            rc = RTLocalIpcSessionRead(hSession, &Hdr, sizeof(Hdr), NULL /*pcbRead - exact, blocking*/);
+            if (RT_FAILURE(rc))
                 break;
-            }
-            else
-                LogFlowFunc(("Session %p: Waiting for session data failed with rc=%Rrc\n",
-                         pThis, rc));
+
+            /*
+             * Validate the message header.
+             *
+             * Disconnecting the client if invalid or something we don't grok.
+             * Currently all clients are one-shots, so there is no need to get
+             * in complicated recovery code if we don't understand one another.
+             */
+            if (   Hdr.uMagic   != VBOXTRAY_IPC_HDR_MAGIC
+                || Hdr.uVersion != VBOXTRAY_IPC_HDR_VERSION)
+            {
+                LogRelFunc(("Session %p: Invalid header magic/version: %#x, %#x, %#x, %#x\n",
+                            Hdr.uMagic, Hdr.uVersion, Hdr.enmMsgType, Hdr.cbPayload));
+                rc = VERR_INVALID_MAGIC;
+                break;
+            }
+            if (Hdr.cbPayload > VBOXTRAY_IPC_MAX_PAYLOAD)
+            {
+                LogRelFunc(("Session %p: Payload to big: %#x, %#x, %#x, %#x - max %#x\n",
+                            Hdr.uMagic, Hdr.uVersion, Hdr.enmMsgType, Hdr.cbPayload, VBOXTRAY_IPC_MAX_PAYLOAD));
+                rc = VERR_TOO_MUCH_DATA;
+                break;
+            }
+            if (   Hdr.enmMsgType > VBOXTRAYIPCMSGTYPE_INVALID
+                && Hdr.enmMsgType < VBOXTRAYIPCMSGTYPE_END)
+            {
+                LogRelFunc(("Session %p: Unknown message: %#x, %#x, %#x, %#x\n",
+                            Hdr.uMagic, Hdr.uVersion, Hdr.enmMsgType, Hdr.cbPayload));
+                rc = VERR_INVALID_FUNCTION;
+                break;
+            }
+
+            /*
+             * Handle the message.
+             */
+            switch (Hdr.enmMsgType)
+            {
+                case VBOXTRAYIPCMSGTYPE_RESTART:
+                    rc = vboxIPCHandleVBoxTrayRestart(pThis, &Hdr);
+                    break;
+
+                case VBOXTRAYIPCMSGTYPE_SHOW_BALLOON_MSG:
+                    rc = vboxIPCHandleShowBalloonMsg(pThis, &Hdr);
+                    break;
+
+                case VBOXTRAYIPCMSGTYPE_USER_LAST_INPUT:
+                    rc = vboxIPCHandleUserLastInput(pThis, &Hdr);
+                    break;
+
+                default:
+                    AssertFailedBreakStmt(rc = VERR_IPE_NOT_REACHED_DEFAULT_CASE);
+            }
+            if (RT_FAILURE(rc))
+                LogFlowFunc(("Session %p: Handling command %RU32 failed with rc=%Rrc\n", pThis, Hdr.enmMsgType, rc));
+        }
+        else if (rc == VERR_CANCELLED)
+        {
+            LogFlowFunc(("Session %p: Waiting for data cancelled\n", pThis));
+            rc = VINF_SUCCESS;
+            break;
         }
         else
-        {
-            VBOXTRAYIPCHEADER ipcHdr;
-            rc = RTLocalIpcSessionRead(hSession, &ipcHdr, sizeof(ipcHdr),
-                                       NULL /* Exact read, blocking */);
-            bool fRejected = false; /* Reject current command? */
-            if (RT_SUCCESS(rc))
-                fRejected =    ipcHdr.uMagic != VBOXTRAY_IPC_HDR_MAGIC
-                            || ipcHdr.uHdrVersion != 0; /* We only know version 0 commands for now. */
-
-            if (   !fRejected
-                && RT_SUCCESS(rc))
-            {
-                switch (ipcHdr.uMsgType)
-                {
-                    case VBOXTRAYIPCMSGTYPE_RESTART:
-                        rc = vboxIPCHandleVBoxTrayRestart(pThis, &ipcHdr);
-                        break;
-
-                    case VBOXTRAYIPCMSGTYPE_SHOWBALLOONMSG:
-                        rc = vboxIPCHandleShowBalloonMsg(pThis, &ipcHdr);
-                        break;
-
-                    case VBOXTRAYIPCMSGTYPE_USERLASTINPUT:
-                        rc = vboxIPCHandleUserLastInput(pThis, &ipcHdr);
-                        break;
-
-                    default:
-                    {
-                        /* Unknown command, reject. */
-                        fRejected = true;
-                        break;
-                    }
-                }
-
-                if (RT_FAILURE(rc))
-                    LogFlowFunc(("Session %p: Handling command %RU32 failed with rc=%Rrc\n",
-                             pThis, ipcHdr.uMsgType, rc));
-            }
-
-            if (fRejected)
-            {
-                static int s_cRejectedCmds = 0;
-                if (++s_cRejectedCmds <= 3)
-                {
-                    LogRelFunc(("Session %p: Received invalid/unknown command %RU32 (%RU32 bytes), rejecting (%RU32/3)\n",
-                                pThis, ipcHdr.uMsgType, ipcHdr.uMsgLen, s_cRejectedCmds + 1));
-                    if (ipcHdr.uMsgLen)
-                    {
-                        /* Get and discard payload data. */
-                        size_t cbRead;
-                        uint8_t devNull[_1K];
-                        while (ipcHdr.uMsgLen)
-                        {
-                            rc = RTLocalIpcSessionRead(hSession, &devNull, sizeof(devNull), &cbRead);
-                            if (RT_FAILURE(rc))
-                                break;
-                            AssertRelease(cbRead <= ipcHdr.uMsgLen);
-                            ipcHdr.uMsgLen -= (uint32_t)cbRead;
-                        }
-                    }
-                }
-                else
-                    rc = VERR_INVALID_PARAMETER; /* Enough fun, bail out. */
-            }
-        }
-    }
-
-    LogFlowFunc(("Session %p: Handler ended with rc=%Rrc\n",
-             pThis, rc));
+            LogFlowFunc(("Session %p: Waiting for session data failed with rc=%Rrc\n", pThis, rc));
+    }
+
+    LogFlowFunc(("Session %p: Handler ended with rc=%Rrc\n", pThis, rc));
 
     /*
@@ -461 +481 @@
     }
 
-    LogFlowFunc(("Session %p: Terminated with rc=%Rrc, freeing ...\n",
-             pThis, rc));
+    LogFlowFunc(("Session %p: Terminated with rc=%Rrc, freeing ...\n", pThis, rc));
 
     RTMemFree(pThis);

trunk/src/VBox/Additions/WINNT/VBoxTray/VBoxTrayMsg.h

@@ -34 +34 @@
 /** The IPC pipe's prefix (native).
  * Will be followed by the username VBoxTray runs under. */
-#define VBOXTRAY_IPC_PIPE_PREFIX      "\\\\.\\pipe\\VBoxTrayIPC-"
+#define VBOXTRAY_IPC_PIPE_PREFIX        "\\\\.\\pipe\\VBoxTrayIPC-"
 /** The IPC header's magic. */
-#define VBOXTRAY_IPC_HDR_MAGIC        0x19840804
+#define VBOXTRAY_IPC_HDR_MAGIC          0x19840804
+/** IPC header version number. */
+#define VBOXTRAY_IPC_HDR_VERSION        1
+/** The max payload size accepted by VBoxTray.  Clients trying to send more
+ *  will be disconnected. */
+#define VBOXTRAY_IPC_MAX_PAYLOAD        _16K
 
-enum VBOXTRAYIPCMSGTYPE
+
+/**
+ * VBoxTray IPC message types.
+ */
+typedef enum VBOXTRAYIPCMSGTYPE
 {
-    /** Restarts VBoxTray. */
-    VBOXTRAYIPCMSGTYPE_RESTART        = 10,
-    /** Shows a balloon message in the tray area. */
-    VBOXTRAYIPCMSGTYPE_SHOWBALLOONMSG = 100,
-    /** Retrieves the current user's last input
-     *  time. This will be the user VBoxTray is running
-     *  under. No actual message for this command
-     *  required. */
-    VBOXTRAYIPCMSGTYPE_USERLASTINPUT  = 120
-};
+    /** Customary invalid zero value. */
+    VBOXTRAYIPCMSGTYPE_INVALID = 0,
+    /** Restarts VBoxTray - not implemented.
+     * Payload: None.
+     * Reply: None. */
+    VBOXTRAYIPCMSGTYPE_RESTART,
+    /** Shows a balloon message in the tray area.
+     * Payload: VBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T
+     * Reply: None */
+    VBOXTRAYIPCMSGTYPE_SHOW_BALLOON_MSG,
+    /** Time since the last user input for the user VBoxTray is running as.
+     * Payload: None.
+     * Reply: VBOXTRAYIPCREPLY_USER_LAST_INPUT_T. */
+    VBOXTRAYIPCMSGTYPE_USER_LAST_INPUT,
+    /** End of valid types. */
+    VBOXTRAYIPCMSGTYPE_END,
+    /* Make sure the type is 32-bit wide. */
+    VBOXTRAYIPCMSGTYPE_32BIT_HACK = 0x7fffffff
+} VBOXTRAYIPCMSGTYPE;
 
-/* VBoxTray's IPC header. */
+/**
+ * VBoxTray's IPC header.
+ *
+ * All messages have one of these.  The payload following it is optional and
+ * specific to each individual message type.
+ */
 typedef struct VBOXTRAYIPCHEADER
 {
-    /** The header's magic. */
-    uint32_t uMagic;
+    /** The header's magic (VBOXTRAY_IPC_HDR_MAGIC). */
+    uint32_t            uMagic;
     /** Header version, must be 0 by now. */
-    uint32_t uHdrVersion;
-    /** Message type. Specifies a message
-     *  of VBOXTRAYIPCMSGTYPE. */
-    uint32_t uMsgType;
-    /** Message length (in bytes). This must
-     *  include the overall message length, including
-     *  (eventual) dynamically allocated areas which
-     *  are passed into the message structure.
-     */
-    uint32_t uMsgLen;
-
-} VBOXTRAYIPCHEADER, *PVBOXTRAYIPCHEADER;
+    uint32_t            uVersion;
+    /** Message type, a VBOXTRAYIPCMSGTYPE value. */
+    VBOXTRAYIPCMSGTYPE  enmMsgType;
+    /** Payload length in bytes.
+     * When present, the payload follows this header. */
+    uint32_t            cbPayload;
+} VBOXTRAYIPCHEADER;
+/** Pointer to a VBoxTray IPC header. */
+typedef VBOXTRAYIPCHEADER *PVBOXTRAYIPCHEADER;
 
 /**
- * Tells VBoxTray to show a balloon message in Windows'
- * tray area. This may or may not work depending on the
- * system's configuration / set user preference.
+ * Tells VBoxTray to show a balloon message in Windows' tray area.
+ *
+ * This may or may not work depending on the system's configuration / set user
+ * preference.
  */
-typedef struct VBOXTRAYIPCMSG_SHOWBALLOONMSG
+typedef struct VBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T
 {
-    /** Length of message body (in bytes). */
-    uint32_t cbMsgContent;
-    /** Length of message title (in bytes). */
-    uint32_t cbMsgTitle;
+    /** Length of the message string (no terminator). */
+    uint32_t    cchMsg;
+    /** Length of the title string (no terminator). */
+    uint32_t    cchTitle;
     /** Message type. */
-    uint32_t uType;
+    uint32_t    uType;
     /** Time to show the message (in ms). */
-    uint32_t uShowMS;
-    /** Dynamically allocated stuff.
-     *
-     *  Note: These must come at the end of the
-     *  structure to not overwrite any important
-     *  stuff above.
-     */
-    /** Message body. Can be up to 256 chars
-     *  long. */
-    char     szMsgContent[1];
-        /** Message title. Can be up to 73 chars
-     *  long. */
-    char     szMsgTitle[1];
-} VBOXTRAYIPCMSG_SHOWBALLOONMSG, *PVBOXTRAYIPCMSG_SHOWBALLOONMSG;
+    uint32_t    cMsTimeout;
+    /** Variable length buffer containing two szero terminated strings, first is  */
+    char        szzStrings[RT_FLEXIBLE_ARRAY];
+} VBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T;
+typedef VBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T *PVBOXTRAYIPCMSG_SHOW_BALLOON_MSG_T;
 
 /**
- * Response telling the last input of the current user.
+ * Reply to VBOXTRAYIPCMSGTYPE_USER_LAST_INPUT
  */
-typedef struct VBOXTRAYIPCRES_USERLASTINPUT
+typedef struct VBOXTRAYIPCREPLY_USER_LAST_INPUT_T
 {
-    /** Last occurred user input event (in seconds). */
-    uint32_t uLastInput;
-} VBOXTRAYIPCRES_USERLASTINPUT, *PVBOXTRAYIPCRES_USERLASTINPUT;
+    /** How many seconds since the last user input event.
+     * Set to UINT32_MAX if we don't know. */
+    uint32_t    cSecSinceLastInput;
+} VBOXTRAYIPCREPLY_USER_LAST_INPUT_T;
+typedef VBOXTRAYIPCREPLY_USER_LAST_INPUT_T *PVBOXTRAYIPCREPLY_USER_LAST_INPUT_T;
 
 #endif /* !GA_INCLUDED_SRC_WINNT_VBoxTray_VBoxTrayMsg_h */

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceVMInfo-win.cpp

@@ -58 +58 @@
 #include "../../WINNT/VBoxTray/VBoxTrayMsg.h" /* For IPC. */
 
-static uint32_t s_uDebugGuestPropClientID = 0;
-static uint32_t s_uDebugIter = 0;
-/** Whether to skip the logged-in user detection over RDP or not.
- *  See notes in this section why we might want to skip this. */
-static bool s_fSkipRDPDetection = false;
-
 
 /*********************************************************************************************************************************
@@ -117 +111 @@
 *   Global Variables                                                                                                             *
 *********************************************************************************************************************************/
+static uint32_t s_uDebugGuestPropClientID = 0;
+static uint32_t s_uDebugIter = 0;
+/** Whether to skip the logged-in user detection over RDP or not.
+ *  See notes in this section why we might want to skip this. */
+static bool s_fSkipRDPDetection = false;
+
 static RTONCE                                   g_vgsvcWinVmInitOnce = RTONCE_INITIALIZER;
 
@@ -984 +984 @@
             {
                 /* .uMagic      = */ VBOXTRAY_IPC_HDR_MAGIC,
-                /* .uHdrVersion = */ 0,
-                /* .uMsgType    = */ VBOXTRAYIPCMSGTYPE_USERLASTINPUT,
-                /* .cbMsgData   = */ 0 /* No msg */
+                /* .uVersion    = */ VBOXTRAY_IPC_HDR_VERSION,
+                /* .enmMsgType  = */ VBOXTRAYIPCMSGTYPE_USER_LAST_INPUT,
+                /* .cbPayload   = */ 0 /* No payload */
             };
 
             rc = RTLocalIpcSessionWrite(hSession, &ipcHdr, sizeof(ipcHdr));
-
             if (RT_SUCCESS(rc))
             {
-                VBOXTRAYIPCRES_USERLASTINPUT ipcRes;
-                rc = RTLocalIpcSessionRead(hSession, &ipcRes, sizeof(ipcRes), NULL /* Exact read */);
+                VBOXTRAYIPCREPLY_USER_LAST_INPUT_T ipcReply;
+                rc = RTLocalIpcSessionRead(hSession, &ipcReply, sizeof(ipcReply), NULL /* Exact read */);
                 if (   RT_SUCCESS(rc)
                     /* If uLastInput is set to UINT32_MAX VBoxTray was not able to retrieve the
                      * user's last input time. This might happen when running on Windows NT4 or older. */
-                    && ipcRes.uLastInput != UINT32_MAX)
+                    && ipcReply.cSecSinceLastInput != UINT32_MAX)
                 {
-                    userState = (ipcRes.uLastInput * 1000) < g_uVMInfoUserIdleThresholdMS
+                    userState = ipcReply.cSecSinceLastInput * 1000 < g_uVMInfoUserIdleThresholdMS
                               ? VBoxGuestUserState_InUse
                               : VBoxGuestUserState_Idle;
@@ -1014 +1013 @@
                     fReportToHost = rc == VINF_SUCCESS;
                     VGSvcVerbose(4, "User '%s' (domain '%s') is idle for %RU32, fReportToHost=%RTbool\n",
-                                 pszUser, pszDomain ? pszDomain : "<None>", ipcRes.uLastInput, fReportToHost);
+                                 pszUser, pszDomain ? pszDomain : "<None>", ipcReply.cSecSinceLastInput, fReportToHost);
 
 #if 0 /* Do we want to write the idle time as well? */
                         /* Also write the user's current idle time, if there is any. */
                         if (userState == VBoxGuestUserState_Idle)
-                            rc = vgsvcUserUpdateF(pCache, pszUser, pszDomain, "IdleTimeMs", "%RU32", ipcRes.uLastInputMs);
+                            rc = vgsvcUserUpdateF(pCache, pszUser, pszDomain, "IdleTimeMs", "%RU32", ipcReply.cSecSinceLastInput);
                         else
                             rc = vgsvcUserUpdateF(pCache, pszUser, pszDomain, "IdleTimeMs", NULL /* Delete property */);
@@ -1027 +1026 @@
                 }
 #ifdef DEBUG
-                else if (RT_SUCCESS(rc) && ipcRes.uLastInput == UINT32_MAX)
+                else if (RT_SUCCESS(rc) && ipcReply.cSecSinceLastInput == UINT32_MAX)
                     VGSvcVerbose(4, "Last input for user '%s' is not supported, skipping\n", pszUser, rc);
 #endif