Changeset 92662 in vbox for trunk/src/VBox

Timestamp:

Dec 1, 2021 2:42:28 AM (3 years ago)

Author:

vboxsync

Message:

VBoxService: Sketch for a trick for passing UTF-8 argv to child VBoxServices process - disabled and untested. bugref:10153

Location:

trunk/src/VBox/Additions/common/VBoxService

Files:

• VBoxService.cpp (modified) (1 diff)
• VBoxServiceControlProcess.cpp (modified) (13 diffs)
• VBoxServiceControlSession.cpp (modified) (2 diffs)
• VBoxServiceInternal.h (modified) (1 diff)

trunk/src/VBox/Additions/common/VBoxService/VBoxService.cpp

@@ -896 +896 @@
     /*
      * Init globals and such.
-     */
-    int rc = RTR3InitExe(argc, &argv, 0);
+     *
+     * Note! The --utf8-argv stuff is an internal hack to avoid locale configuration
+     *       issues preventing us from passing non-ASCII string to child processes.
+     */
+    uint32_t fIprtFlags = 0;
+#ifdef VBOXSERVICE_ARG1_UTF8_ARGV
+    if (argc > 1 && strcmp(argv[1], VBOXSERVICE_ARG1_UTF8_ARGV) == 0)
+    {
+        argv[1] = argv[0];
+        argv++;
+        argc--;
+        fIprtFlags |= RTR3INIT_FLAGS_UTF8_ARGV;
+    }
+#endif
+    int rc = RTR3InitExe(argc, &argv, fIprtFlags);
     if (RT_FAILURE(rc))
         return RTMsgInitFailure(rc);
+
     g_pszProgName = RTPathFilename(argv[0]);
 #ifdef RT_OS_WINDOWS

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControlProcess.cpp

@@ -1036 +1036 @@
  * @param  papszArgs        Original argv command line from the host, starting at argv[1].
  * @param  fFlags           The process creation flags pass to us from the host.
+ * @param  fExecutingSelf   Set if we're executing the VBoxService executable
+ *                          and should inject the --utf8-argv trick.
  * @param  ppapszArgv       Pointer to a pointer with the new argv command line.
  *                          Needs to be freed with RTGetOptArgvFree.
  */
 static int vgsvcGstCtrlProcessAllocateArgv(const char *pszArgv0, const char * const *papszArgs, uint32_t fFlags,
-                                           char ***ppapszArgv)
-{
-    VGSvcVerbose(3, "VGSvcGstCtrlProcessPrepareArgv: pszArgv0=%p, papszArgs=%p, fFlags=%#x, ppapszArgv=%p\n",
-                 pszArgv0, papszArgs, fFlags, ppapszArgv);
+                                           bool fExecutingSelf, char ***ppapszArgv)
+{
+    VGSvcVerbose(3, "VGSvcGstCtrlProcessPrepareArgv: pszArgv0=%p, papszArgs=%p, fFlags=%#x, fExecutingSelf=%d, ppapszArgv=%p\n",
+                 pszArgv0, papszArgs, fFlags, fExecutingSelf, ppapszArgv);
 
     AssertPtrReturn(pszArgv0,   VERR_INVALID_POINTER);
     AssertPtrReturn(ppapszArgv, VERR_INVALID_POINTER);
 
+#ifndef VBOXSERVICE_ARG1_UTF8_ARGV
+    fExecutingSelf = false;
+#endif
+
+    /* Count arguments: */
     int rc = VINF_SUCCESS;
     uint32_t cArgs;
@@ -1057 +1064 @@
 
     /* Allocate new argv vector (adding + 2 for argv0 + termination). */
-    size_t cbSize = (cArgs + 2) * sizeof(char *);
+    size_t cbSize = (fExecutingSelf + cArgs + 2) * sizeof(char *);
     char **papszNewArgv = (char **)RTMemAlloc(cbSize);
     if (!papszNewArgv)
@@ -1075 +1082 @@
                    a little in the unquoted argument case to deal with executables
                    containing spaces. */
+    /** @todo r=bird: WTF!?? This makes absolutely no sense on non-windows.  An
+     * on windows the first flag test must be inverted, as it's when RTProcCreateEx
+     * doesn't do any quoting that we have to do it here, isn't it?
+     * Aaaaaaaaaaaaaaaaaaarrrrrrrrrrrrrrrrrrrrrrrrrrgggggggggggggggggggggggg! */
     if (   !(fFlags & EXECUTEPROCESSFLAG_UNQUOTED_ARGS)
         || !strpbrk(pszArgv0, " \t\n\r")
@@ -1099 +1110 @@
     if (RT_SUCCESS(rc))
     {
-        size_t i;
-        for (i = 0; i < cArgs; i++)
-        {
-            char *pszArg;
+        size_t iDst = 1;
+
+#ifdef VBOXSERVICE_ARG1_UTF8_ARGV
+        /* Insert --utf8-argv as the first argument if executing the VBoxService binary. */
+        if (fExecutingSelf)
+        {
+            rc = RTStrDupEx(&papszNewArgv[iDst], VBOXSERVICE_ARG1_UTF8_ARGV);
+            if (RT_SUCCESS(rc))
+                iDst++;
+        }
+#endif
+        /* Copy over the other arguments. */
+        if (RT_SUCCESS(rc))
+            for (size_t iSrc = 0; iSrc < cArgs; iSrc++)
+            {
 #if 0 /* Arguments expansion -- untested. */
-            if (fFlags & EXECUTEPROCESSFLAG_EXPAND_ARGUMENTS)
-            {
+                if (fFlags & EXECUTEPROCESSFLAG_EXPAND_ARGUMENTS)
+                {
 /** @todo r=bird: If you want this, we need a generic implementation, preferably in RTEnv or somewhere like that.  The marking
  * up of the variables must be the same on all platforms.  */
-                /* According to MSDN the limit on older Windows version is 32K, whereas
-                 * Vista+ there are no limits anymore. We still stick to 4K. */
-                char szExpanded[_4K];
+                    /* According to MSDN the limit on older Windows version is 32K, whereas
+                     * Vista+ there are no limits anymore. We still stick to 4K. */
+                    char szExpanded[_4K];
 # ifdef RT_OS_WINDOWS
-                if (!ExpandEnvironmentStrings(papszArgs[i], szExpanded, sizeof(szExpanded)))
-                    rc = RTErrConvertFromWin32(GetLastError());
+                    if (!ExpandEnvironmentStrings(papszArgs[i], szExpanded, sizeof(szExpanded)))
+                        rc = RTErrConvertFromWin32(GetLastError());
 # else
-                /* No expansion for non-Windows yet. */
-                rc = RTStrCopy(papszArgs[i], sizeof(szExpanded), szExpanded);
+                    /* No expansion for non-Windows yet. */
+                    rc = RTStrCopy(papszArgs[i], sizeof(szExpanded), szExpanded);
 # endif
+                    if (RT_SUCCESS(rc))
+                        rc = RTStrDupEx(&pszArg, szExpanded);
+                }
+                else
+#endif
+                rc = RTStrDupEx(&papszNewArgv[iDst], papszArgs[iSrc]);
                 if (RT_SUCCESS(rc))
-                    rc = RTStrDupEx(&pszArg, szExpanded);
+                    iDst++;
+                else
+                    break;
             }
-            else
-#endif
-                rc = RTStrDupEx(&pszArg, papszArgs[i]);
-
-            if (RT_FAILURE(rc))
-                break;
-
-            papszNewArgv[i + 1] = pszArg;
-        }
 
         if (RT_SUCCESS(rc))
         {
             /* Terminate array. */
-            papszNewArgv[cArgs + 1] = NULL;
+            papszNewArgv[iDst] = NULL;
+            Assert(iDst < cArgs);
 
             *ppapszArgv = papszNewArgv;
@@ -1141 +1163 @@
 
         /* Failed, bail out. */
-        for (; i > 0; i--)
-            RTStrFree(papszNewArgv[i]);
+        while (iDst-- > 0)
+            RTStrFree(papszNewArgv[iDst]);
     }
     RTMemFree(papszNewArgv);
@@ -1306 +1328 @@
         {
             char **papszArgsExp;
-            rc = vgsvcGstCtrlProcessAllocateArgv(szSysprepCmd /* argv0 */, papszArgs, fFlags, &papszArgsExp);
+            rc = vgsvcGstCtrlProcessAllocateArgv(szSysprepCmd /* argv0 */, papszArgs, fFlags,
+                                                 false /*fExecutingSelf*/, &papszArgsExp);
             if (RT_SUCCESS(rc))
             {
@@ -1326 +1349 @@
 #endif /* RT_OS_WINDOWS */
 
+    bool fExecutingSelf = false;
 #ifdef VBOX_WITH_VBOXSERVICE_TOOLBOX
     if (RTStrStr(pszExec, "vbox_") == pszExec) /** @todo WTF search the whole string for "vbox_" when all you want is to know if whether string starts with "vbox_" or not. geee^2 */
@@ -1331 +1355 @@
         /* We want to use the internal toolbox (all internal
          * tools are starting with "vbox_" (e.g. "vbox_cat"). */
+        fExecutingSelf = true;
         rc = vgsvcGstCtrlProcessResolveExecutable(VBOXSERVICE_NAME, szExecExp, sizeof(szExecExp));
     }
@@ -1367 +1392 @@
 
         char **papszArgsExp;
-        rc = vgsvcGstCtrlProcessAllocateArgv(pcszArgv0, &papszArgs[uArgvIdx], fFlags, &papszArgsExp);
+        rc = vgsvcGstCtrlProcessAllocateArgv(pcszArgv0, &papszArgs[uArgvIdx], fFlags, fExecutingSelf, &papszArgsExp);
         if (RT_FAILURE(rc))
         {
@@ -1375 +1400 @@
         else
         {
-            uint32_t uProcFlags = 0;
+            uint32_t fProcCreateFlags = 0;
+            if (fExecutingSelf)
+                fProcCreateFlags |= VBOXSERVICE_PROC_F_UTF8_ARGV;
             if (fFlags)
             {
                 if (fFlags & EXECUTEPROCESSFLAG_HIDDEN)
-                    uProcFlags |= RTPROC_FLAGS_HIDDEN;
+                    fProcCreateFlags |= RTPROC_FLAGS_HIDDEN;
                 if (fFlags & EXECUTEPROCESSFLAG_PROFILE)
-                    uProcFlags |= RTPROC_FLAGS_PROFILE;
+                    fProcCreateFlags |= RTPROC_FLAGS_PROFILE;
                 if (fFlags & EXECUTEPROCESSFLAG_UNQUOTED_ARGS)
-                    uProcFlags |= RTPROC_FLAGS_UNQUOTED_ARGS;
+                    fProcCreateFlags |= RTPROC_FLAGS_UNQUOTED_ARGS;
             }
 
@@ -1393 +1420 @@
              * started from our system service. */
             if (pszAsUser && *pszAsUser)
-                uProcFlags |= RTPROC_FLAGS_SERVICE;
+                fProcCreateFlags |= RTPROC_FLAGS_SERVICE;
 #ifdef DEBUG
             VGSvcVerbose(3, "Command: %s\n", szExecExp);
             for (size_t i = 0; papszArgsExp[i]; i++)
-                VGSvcVerbose(3, "\targv[%ld]: %s\n", i, papszArgsExp[i]);
+                VGSvcVerbose(3, "  argv[%zu]: %s\n", i, papszArgsExp[i]);
 #endif
             VGSvcVerbose(3, "Starting process '%s' ...\n", szExecExp);
@@ -1407 +1434 @@
             char *pszUserUPN = NULL;
             if (   pszDomain
-                && strlen(pszDomain))
+                && *pszDomain != '\0')
             {
                 int cbUserUPN = RTStrAPrintf(&pszUserUPN, "%s@%s", pszAsUser, pszDomain);
@@ -1419 +1446 @@
 
             /* Do normal execution. */
-            rc = RTProcCreateEx(szExecExp, papszArgsExp, hEnv, uProcFlags,
+            rc = RTProcCreateEx(szExecExp, papszArgsExp, hEnv, fProcCreateFlags,
                                 phStdIn, phStdOut, phStdErr,
                                 pszUser,

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControlSession.cpp

@@ -2312 +2312 @@
 
     apszArgs[idxArg++] = pszExeName;
+#ifdef VBOXSERVICE_ARG1_UTF8_ARGV
+    apszArgs[idxArg++] = VBOXSERVICE_ARG1_UTF8_ARGV; Assert(idxArg == 2);
+#endif
     apszArgs[idxArg++] = "guestsession";
     apszArgs[idxArg++] = szParmSessionID;
@@ -2408 +2411 @@
                          | RTPROC_FLAGS_HIDDEN
 #endif
-                         ;
+                         | VBOXSERVICE_PROC_F_UTF8_ARGV;
 
     /*

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceInternal.h

@@ -35 +35 @@
 #include <VBox/VBoxGuestLib.h>
 #include <VBox/HostServices/GuestControlSvc.h>
+
+
+#if 0 //!defined(RT_OS_WINDOWS) || defined(DOXYGEN_RUNNING)
+/** Special argv[1] value that indicates that argv is UTF-8.
+ * This causes RTR3Init to be called with RTR3INIT_FLAGS_UTF8_ARGV and helps
+ * work around potential issues caused by a user's locale config not being
+ * UTF-8.  See @bugref{10153}.
+ *
+ * @note We don't need this on windows and it would be harmful to enable it
+ *       as the argc/argv vs __argc/__argv comparison would fail and we would
+ *       not use the unicode command line to create a UTF-8 argv.  Since the
+ *       original argv is ANSI, it may be missing codepoints not present in
+ *       the ANSI code page of the process. */
+# define VBOXSERVICE_ARG1_UTF8_ARGV      "--utf8-argv"
+#endif
+/** RTProcCreateEx flags corresponding to VBOXSERVICE_ARG1_UTF8_ARGV. */
+#ifdef VBOXSERVICE_ARG1_UTF8_ARGV
+# define VBOXSERVICE_PROC_F_UTF8_ARGV       0 /** @todo TBD */
+#else
+# define VBOXSERVICE_PROC_F_UTF8_ARGV       0
+#endif
+
 
 /**