Hello Bill,

Sorry to respond to this so late, but I see that this is up for a vote against httpd 2.4.
I noticed a possible problem in the code.
I have a question inserted below.

Thanks,

Mike Rumph

On 7/16/2014 1:15 PM, wr...@apache.org wrote:
Author: wrowe
Date: Wed Jul 16 20:15:49 2014
New Revision: 1611169

URL: http://svn.apache.org/r1611169
Log:
mpm_winnt: Accept utf-8 (Unicode) service names and descriptions for
internationalization.

Modified:
     httpd/httpd/trunk/CHANGES
     httpd/httpd/trunk/server/mpm/winnt/service.c

Modified: httpd/httpd/trunk/CHANGES
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1611169&r1=1611168&r2=1611169&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Jul 16 20:15:49 2014
@@ -1,6 +1,12 @@
                                                           -*- coding: utf-8 -*-
  Changes with Apache 2.5.0
+ *) mpm_winnt: Accept utf-8 (Unicode) service names and descriptions for
+     internationalization.  [William Rowe]
+
+  *) mpm_winnt: Normalize the error and status messages emitted by service.c,
+     the service control interface for Windows.  [William Rowe]
+
    *) SECURITY: CVE-2013-5704 (cve.mitre.org)
       core: HTTP trailers could be used to replace HTTP headers
       late during request processing, potentially undoing or

Modified: httpd/httpd/trunk/server/mpm/winnt/service.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/winnt/service.c?rev=1611169&r1=1611168&r2=1611169&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/winnt/service.c (original)
+++ httpd/httpd/trunk/server/mpm/winnt/service.c Wed Jul 16 20:15:49 2014
@@ -21,11 +21,18 @@
#define _WINUSER_ +#include "apr.h"
+#include "apr_strings.h"
+#include "apr_lib.h"
+#if APR_HAS_UNICODE_FS
+#include "arch/win32/apr_arch_utf8.h"
+#include "arch/win32/apr_arch_misc.h"
+#include <wchar.h>
+#endif
+
  #include "httpd.h"
  #include "http_log.h"
  #include "mpm_winnt.h"
-#include "apr_strings.h"
-#include "apr_lib.h"
  #include "ap_regkey.h"
#ifdef NOUSER
@@ -41,6 +48,10 @@ APLOG_USE_MODULE(mpm_winnt);
  static char *mpm_service_name = NULL;
  static char *mpm_display_name = NULL;
+#if APR_HAS_UNICODE_FS
+static apr_wchar_t *mpm_service_name_w;
+#endif
+
  typedef struct nt_service_ctx_t
  {
      HANDLE mpm_thread;       /* primary thread handle of the apache server */
@@ -57,6 +68,32 @@ static nt_service_ctx_t globdat;
  static int ReportStatusToSCMgr(int currentState, int waitHint,
                                 nt_service_ctx_t *ctx);
+/* Rather than repeat this logic throughout, create an either-or wide or narrow
+ * implementation because we don't actually pass strings to OpenSCManager.
+ * This election is based on build time defines and runtime os version test.
+ */
+#undef OpenSCManager
+typedef SC_HANDLE (WINAPI *fpt_OpenSCManager)(const void *lpMachine,
+                                              const void *lpDatabase,
+                                              DWORD dwAccess);
+static fpt_OpenSCManager pfn_OpenSCManager = NULL;
+static APR_INLINE SC_HANDLE OpenSCManager(const void *lpMachine,
+                                          const void *lpDatabase,
+                                          DWORD dwAccess)
+{
+    if (!pfn_OpenSCManager) {
+#if APR_HAS_UNICODE_FS
+        IF_WIN_OS_IS_UNICODE
+            pfn_OpenSCManager = (fpt_OpenSCManager)OpenSCManagerW;
+#endif
+#if APR_HAS_ANSI_FS
+        ELSE_WIN_OS_IS_ANSI
+            pfn_OpenSCManager = (fpt_OpenSCManager)OpenSCManagerA;
+#endif
+    }
+    return (*(pfn_OpenSCManager))(lpMachine, lpDatabase, dwAccess);
+}
+
  /* exit() for Win32 is macro mapped (horrible, we agree) that allows us
   * to catch the non-zero conditions and inform the console process that
   * the application died, and hang on to the console a bit longer.
@@ -245,16 +282,54 @@ static void set_service_description(void
      if ((ChangeServiceConfig2) &&
          (schSCManager = OpenSCManager(NULL, NULL, SC_MANAGER_CONNECT)))
      {
-        SC_HANDLE schService = OpenService(schSCManager, mpm_service_name,
-                                           SERVICE_CHANGE_CONFIG);
+        SC_HANDLE schService;
+
+#if APR_HAS_UNICODE_FS
+        IF_WIN_OS_IS_UNICODE
+        {
+            schService = OpenServiceW(schSCManager,
+                                      (LPCWSTR)mpm_service_name_w,
+                                      SERVICE_CHANGE_CONFIG);
+        }
+#endif /* APR_HAS_UNICODE_FS */
+#if APR_HAS_ANSI_FS
+        ELSE_WIN_OS_IS_ANSI
+        {
+            schService = OpenService(schSCManager, mpm_service_name,
+                                     SERVICE_CHANGE_CONFIG);
+        }
+#endif
          if (schService) {
If neither of these defines are set, then schService appears to be uninitialized.
Is this a problem?
              /* Cast is necessary, ChangeServiceConfig2 handles multiple
               * object types, some volatile, some not.
               */
-            /* ###: utf-ize */
-            ChangeServiceConfig2(schService,
-                                 1 /* SERVICE_CONFIG_DESCRIPTION */,
-                                 (LPVOID) &full_description);
+#if APR_HAS_UNICODE_FS
+            IF_WIN_OS_IS_UNICODE
+            {
+                apr_size_t slen = strlen(full_description) + 1;
+                apr_size_t wslen = slen;
+                apr_wchar_t *full_description_w =
+                    (apr_wchar_t*)apr_palloc(pconf,
+                                             wslen * sizeof(apr_wchar_t));
+                apr_status_t rv = apr_conv_utf8_to_ucs2(full_description, 
&slen,
+                                                        full_description_w,
+                                                        &wslen);
+                if ((rv != APR_SUCCESS) || slen
+                        || ChangeServiceConfig2W(schService, 1
+                                                 
/*SERVICE_CONFIG_DESCRIPTION*/,
+                                                 (LPVOID) &full_description_w))
+                    full_description = NULL;
+            }
+#endif /* APR_HAS_UNICODE_FS */
+#if APR_HAS_ANSI_FS
+            ELSE_WIN_OS_IS_ANSI
+            {
+                if (ChangeServiceConfig2(schService,
+                                         1 /* SERVICE_CONFIG_DESCRIPTION */,
+                                         (LPVOID) &full_description))
+                    full_description = NULL;
+            }
+#endif
              CloseServiceHandle(schService);
          }
          CloseServiceHandle(schSCManager);

(The remaining code changes are omitted in this email.)

Reply via email to