httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Rumph <mike.ru...@oracle.com>
Subject Re: svn commit: r1611169 - in /httpd/httpd/trunk: CHANGES server/mpm/winnt/service.c
Date Wed, 14 Jan 2015 18:24:13 GMT
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, wrowe@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.)

Mime
View raw message