httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Darroch <chr...@pearsoncmg.com>
Subject Re: FYI... tentatively planning to roll mod_fcgid again towards the end of the week...
Date Tue, 02 Feb 2010 04:54:22 GMT
Jeff Trawick wrote:

> ... to correct a regression when used with httpd 2.0.x on some
> Unix-ish platforms.

   OK, I have a first cut at what I'm hoping to commit shortly, unless
anyone sees problems.  Let me know if you want me to hold off until
you've cut another GA; otherwise I'll continue testing tomorrow and
look to commit ASAP.

   Way back in the mists of time I added a one-liner to my collection
of mod_fcgid patches which simply changed the default share_group_id to
-1 for for responders which weren't invoked through a wrapper.  This is
in handle_request(), like this:

   shareid = wrapper_conf ? wrapper_conf->share_group_id : -1;

The intent is to avoid spawning the same process twice if it's used in both
responder and authorizer roles; auth configs always have a -1 share_group_id.

   Anyway, looking into it more deeply this time around, I found that
there were a number of interconnected issues hidden in the wrapper code.

   For one, it appears to be the case that when using FcgidWrapper in
.htaccess, there can be pool thread safety issues with multiple threads
trying to write into wrapper_id_hash to add any wrapper configs which turn
at runtime.  And the wrapper_id_hash is per-process, so there's also
unnecessary duplication of FastCGI processes, because one httpd process's
list of wrapper IDs won't match another's for any IDs generated after
initial the config hooks finish.

   I also found that because of the way wrapper IDs are handled, with
a global cur_id that's incremented for each new command line, and
then a per-command-line one that's incremented further from whatever
is assigned from cur_id whenever a command line appears in multiple
FcgidWrappers, that one can construct (admittedly pathological) cases
where the wrong FastCGI process is run.  For example, this setup,
where wrapS is a symlink to wrap1:

FcgidWrapper "/path/to/fcgi/fcgi-bin/wrap1 1" .wrap11 virtual
FcgidWrapper "/path/to/fcgi/fcgi-bin/wrap1 1" .wrap12 virtual
FcgidWrapper "/path/to/fcgi/fcgi-bin/wrapS 2" .wrap21 virtual

In my testing, the .wrap12 and .wrap21 wrappers both ended up with
the same inode and deviceid (because of the symlink) and the same
share_group_id, even though they passed different arguments on their
command lines.  So one of them ended up doing the wrong thing.

   And there's also the original problem I set out to solve back
in 2007, plus various oddities about using AAA configs with wrappers
(which seems kinda supported, kinda not).

   Anyway, this patchset below aims to fix those wrapper problems
in one, primarily by ditching the share_group_id and associated IDs,
and using the actual command line to decide if two FastCGI processes
are the same.  In some quick late-evening testing that appeared to
work for me; no extra FastCGI processes were spawned even when using
multiple .htaccess configs.

   I'll continue testing tomorrow, but I wanted to get some initial
feedback on (a) potential bugs and (b) whether I should aim to commit
before the 2.3.6 is rolled, or wait till after.

Chris.


Index: fcgid_proc_unix.c
===================================================================
--- fcgid_proc_unix.c	(revision 905510)
+++ fcgid_proc_unix.c	(working copy)
@@ -168,7 +168,7 @@
 }
 
 apr_status_t
-proc_spawn_process(char *lpszwapper, fcgid_proc_info * procinfo,
+proc_spawn_process(const char *lpszwapper, fcgid_proc_info * procinfo,
                    fcgid_procnode * procnode)
 {
     server_rec *main_server = procinfo->main_server;
Index: fcgid_proc_win.c
===================================================================
--- fcgid_proc_win.c	(revision 905510)
+++ fcgid_proc_win.c	(working copy)
@@ -58,7 +58,8 @@
     return APR_SUCCESS;
 }
 
-apr_status_t proc_spawn_process(char *wrapper_cmdline, fcgid_proc_info *procinfo,
+apr_status_t proc_spawn_process(const char *wrapper_cmdline,
+                                fcgid_proc_info *procinfo,
                                 fcgid_procnode *procnode)
 {
     HANDLE *finish_event, listen_handle;
Index: fcgid_pm_main.c
===================================================================
--- fcgid_pm_main.c	(revision 905510)
+++ fcgid_pm_main.c	(working copy)
@@ -19,6 +19,7 @@
 #define CORE_PRIVATE
 #include "httpd.h"
 #include "http_config.h"
+#include "apr_strings.h"
 
 #include "fcgid_pm.h"
 #include "fcgid_pm_main.h"
@@ -492,7 +493,7 @@
     /* Prepare to spawn */
     procnode->deviceid = command->deviceid;
     procnode->inode = command->inode;
-    procnode->share_grp_id = command->share_grp_id;
+    apr_cpystrn(procnode->cmdline, command->cmdline, _POSIX_PATH_MAX);
     procnode->virtualhost = command->virtualhost;
     procnode->uid = command->uid;
     procnode->gid = command->gid;
@@ -537,7 +538,7 @@
     /* Spawn the process now */
     /* XXX Spawn uses wrapper_cmdline, but log uses cgipath ? */
     if ((rv =
-         proc_spawn_process(command->wrapper_cmdline, &procinfo,
+         proc_spawn_process(command->cmdline, &procinfo,
                             procnode)) != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_WARNING, rv, main_server,
                      "mod_fcgid: spawn process %s error", command->cgipath);
Index: fcgid_conf.c
===================================================================
--- fcgid_conf.c	(revision 905510)
+++ fcgid_conf.c	(working copy)
@@ -621,11 +621,12 @@
     dirconfig->authenticator_info =
         apr_pcalloc(cmd->server->process->pconf,
                     sizeof(*dirconfig->authenticator_info));
-    apr_cpystrn(dirconfig->authenticator_info->path, authenticator,
-                _POSIX_PATH_MAX);
+    dirconfig->authenticator_info->cgipath =
+        apr_pstrdup(cmd->pool, authenticator);
+    dirconfig->authenticator_info->cmdline =
+        dirconfig->authenticator_info->cgipath;
     dirconfig->authenticator_info->inode = finfo.inode;
     dirconfig->authenticator_info->deviceid = finfo.device;
-    dirconfig->authenticator_info->share_group_id = (apr_size_t) - 1;
     return NULL;
 }
 
@@ -639,7 +640,7 @@
     return NULL;
 }
 
-fcgid_auth_conf *get_authenticator_info(request_rec * r, int *authoritative)
+fcgid_cmd_conf *get_authenticator_info(request_rec * r, int *authoritative)
 {
     fcgid_dir_conf *config =
         ap_get_module_config(r->per_dir_config, &fcgid_module);
@@ -669,11 +670,12 @@
     dirconfig->authorizer_info =
         apr_pcalloc(cmd->server->process->pconf,
                     sizeof(*dirconfig->authorizer_info));
-    apr_cpystrn(dirconfig->authorizer_info->path, authorizer,
-                _POSIX_PATH_MAX);
+    dirconfig->authorizer_info->cgipath =
+        apr_pstrdup(cmd->pool, authorizer);
+    dirconfig->authorizer_info->cmdline =
+        dirconfig->authorizer_info->cgipath;
     dirconfig->authorizer_info->inode = finfo.inode;
     dirconfig->authorizer_info->deviceid = finfo.device;
-    dirconfig->authorizer_info->share_group_id = (apr_size_t) - 1;
     return NULL;
 }
 
@@ -687,7 +689,7 @@
     return NULL;
 }
 
-fcgid_auth_conf *get_authorizer_info(request_rec * r, int *authoritative)
+fcgid_cmd_conf *get_authorizer_info(request_rec * r, int *authoritative)
 {
     fcgid_dir_conf *config =
         ap_get_module_config(r->per_dir_config, &fcgid_module);
@@ -717,10 +719,12 @@
     dirconfig->access_info =
         apr_pcalloc(cmd->server->process->pconf,
                     sizeof(*dirconfig->access_info));
-    apr_cpystrn(dirconfig->access_info->path, access, _POSIX_PATH_MAX);
+    dirconfig->access_info->cgipath =
+        apr_pstrdup(cmd->pool, access);
+    dirconfig->access_info->cmdline = 
+        dirconfig->access_info->cgipath;
     dirconfig->access_info->inode = finfo.inode;
     dirconfig->access_info->deviceid = finfo.device;
-    dirconfig->access_info->share_group_id = (apr_size_t) - 1;
     return NULL;
 }
 
@@ -734,7 +738,7 @@
     return NULL;
 }
 
-fcgid_auth_conf *get_access_info(request_rec * r, int *authoritative)
+fcgid_cmd_conf *get_access_info(request_rec * r, int *authoritative)
 {
     fcgid_dir_conf *config =
         ap_get_module_config(r->per_dir_config, &fcgid_module);
@@ -747,14 +751,6 @@
     return NULL;
 }
 
-typedef struct {
-    apr_hash_t *wrapper_id_hash;
-    apr_size_t cur_id;
-} wrapper_id_info;
-
-/* FIXME thread safety issues when FcgidWrapper is used in .htaccess;
- * see use of pconf
- */
 const char *set_wrapper_config(cmd_parms * cmd, void *dirconfig,
                                const char *wrapper_cmdline,
                                const char *extension,
@@ -763,11 +759,7 @@
     const char *path, *tmp;
     apr_status_t rv;
     apr_finfo_t finfo;
-    const char *userdata_key = "fcgid_wrapper_id";
-    wrapper_id_info *id_info;
-    apr_size_t *wrapper_id;
-    fcgid_wrapper_conf *wrapper = NULL;
-    apr_pool_t *wrapper_conf_pool = cmd->server->process->pconf; /* bad */
+    fcgid_cmd_conf *wrapper = NULL;
     fcgid_dir_conf *config = (fcgid_dir_conf *) dirconfig;
 
     /* Sanity checks */
@@ -786,36 +778,6 @@
             || ap_strchr_c(extension, '/') || ap_strchr_c(extension, '\\')))
         return "Invalid wrapper file extension";
 
-    /* Get wrapper_id hash from user data */
-    {
-        void *id_info_vp;
-        apr_pool_userdata_get(&id_info_vp, userdata_key,
-                              cmd->server->process->pool);
-        id_info = id_info_vp;
-    }
-
-    if (!id_info) {
-        id_info =
-            apr_pcalloc(cmd->server->process->pool, sizeof(*id_info));
-        id_info->wrapper_id_hash =
-            apr_hash_make(cmd->server->process->pool);
-        apr_pool_userdata_set((const void *) id_info, userdata_key,
-                              apr_pool_cleanup_null,
-                              cmd->server->process->pool);
-    }
-    /* Get wrapper_id for wrapper_cmdline */
-    if ((wrapper_id =
-         apr_hash_get(id_info->wrapper_id_hash, wrapper_cmdline,
-                      strlen(wrapper_cmdline))) == NULL) {
-        wrapper_id =
-            apr_pcalloc(cmd->server->process->pool, sizeof(*wrapper_id));
-        *wrapper_id = id_info->cur_id++;
-        apr_hash_set(id_info->wrapper_id_hash, wrapper_cmdline,
-                     strlen(wrapper_cmdline), wrapper_id);
-    }
-
-    wrapper = apr_pcalloc(wrapper_conf_pool, sizeof(*wrapper));
-
     /* Get wrapper path */
     tmp = wrapper_cmdline;
     path = ap_getword_white(cmd->temp_pool, &tmp);
@@ -828,14 +790,13 @@
         return missing_file_msg(cmd->pool, "Wrapper", path, rv);
     }
 
-    wrapper->exe = apr_pstrdup(wrapper_conf_pool, path);
-    /* FIXME no need to embed in structure (subject to correct pool usage) */
-    apr_cpystrn(wrapper->args, wrapper_cmdline, _POSIX_PATH_MAX);
+    wrapper = apr_pcalloc(cmd->pool, sizeof(*wrapper));
+
+    wrapper->cgipath = apr_pstrdup(cmd->pool, path);
+    wrapper->cmdline = apr_pstrdup(cmd->pool, wrapper_cmdline);
     wrapper->inode = finfo.inode;
     wrapper->deviceid = finfo.device;
-    wrapper->share_group_id = *wrapper_id;
     wrapper->virtual = (virtual != NULL && !strcasecmp(virtual, WRAPPER_FLAG_VIRTUAL));
-    (*wrapper_id)++;
 
     if (extension == NULL)
         extension = DEFAULT_WRAPPER_KEY;
@@ -848,10 +809,10 @@
     return NULL;
 }
 
-fcgid_wrapper_conf *get_wrapper_info(const char *cgipath, request_rec * r)
+fcgid_cmd_conf *get_wrapper_info(const char *cgipath, request_rec * r)
 {
     const char *extension;
-    fcgid_wrapper_conf *wrapper;
+    fcgid_cmd_conf *wrapper;
     fcgid_dir_conf *config =
         ap_get_module_config(r->per_dir_config, &fcgid_module);
 
Index: fcgid_pm_win.c
===================================================================
--- fcgid_pm_win.c	(revision 905510)
+++ fcgid_pm_win.c	(working copy)
@@ -128,37 +128,20 @@
 }
 
 void procmgr_init_spawn_cmd(fcgid_command * command, request_rec * r,
-                            const char *argv0, dev_t deviceid,
-                            apr_ino_t inode, apr_size_t share_grp_id)
+                            fcgid_cmd_conf *cmd_conf)
 {
-    fcgid_wrapper_conf *wrapperconf;
-    const char *cmd_to_spawn;
-
     memset(command, 0, sizeof(*command));
 
-    apr_cpystrn(command->cgipath, argv0, _POSIX_PATH_MAX);
+    apr_cpystrn(command->cgipath, cmd_conf->cgipath, _POSIX_PATH_MAX);
+    apr_cpystrn(command->cmdline, cmd_conf->cmdline, _POSIX_PATH_MAX);
     command->deviceid = deviceid;
     command->inode = inode;
-    command->share_grp_id = share_grp_id;
     command->uid = (uid_t) - 1;
     command->gid = (gid_t) - 1;
     command->userdir = 0;
     command->virtualhost = r->server->server_hostname;
 
-    /* Update fcgid_command with wrapper info */
-    command->wrapper_cmdline[0] = '\0';
-    if ((wrapperconf = get_wrapper_info(argv0, r))) {
-        apr_cpystrn(command->wrapper_cmdline, wrapperconf->args, _POSIX_PATH_MAX);
-        command->deviceid = wrapperconf->deviceid;
-        command->inode = wrapperconf->inode;
-        command->share_grp_id = wrapperconf->share_group_id;
-        cmd_to_spawn = wrapperconf->exe;
-    }
-    else {
-        cmd_to_spawn = command->cgipath;
-    }
-
-    get_cmd_options(r, cmd_to_spawn, &command->cmdopts, &command->cmdenv);
+    get_cmd_options(r, command->cgipath, &command->cmdopts, &command->cmdenv);
 }
 
 apr_status_t procmgr_post_spawn_cmd(fcgid_command * command,
Index: fcgid_conf.h
===================================================================
--- fcgid_conf.h	(revision 905510)
+++ fcgid_conf.h	(working copy)
@@ -44,23 +44,16 @@
 #ifndef VERSION_ONLY
 
 #include "apr_user.h"
+
 #include "fcgid_global.h"
 
 typedef struct {
-    char path[_POSIX_PATH_MAX];
+    const char *cgipath;           /* executable file path */
+    const char *cmdline; /* entire command line */
     apr_ino_t inode;
     apr_dev_t deviceid;
-    apr_size_t share_group_id;
-} fcgid_auth_conf;
-
-typedef struct {
-    const char *exe;            /* executable file path */
-    char args[_POSIX_PATH_MAX]; /* entire command line */
-    apr_ino_t inode;
-    apr_dev_t deviceid;
-    apr_size_t share_group_id;
     int virtual;
-} fcgid_wrapper_conf;
+} fcgid_cmd_conf;
 
 typedef struct {
     /* global only */
@@ -113,17 +106,17 @@
     apr_hash_t *wrapper_info_hash;
 
     /* authenticator */
-    fcgid_auth_conf *authenticator_info;
+    fcgid_cmd_conf *authenticator_info;
     int authenticator_authoritative;
     int authenticator_authoritative_set;
 
     /* authorizer */
-    fcgid_auth_conf *authorizer_info;
+    fcgid_cmd_conf *authorizer_info;
     int authorizer_authoritative;
     int authorizer_authoritative_set;
 
     /* access check */
-    fcgid_auth_conf *access_info;
+    fcgid_cmd_conf *access_info;
     int access_authoritative;
     int access_authoritative_set;
 } fcgid_dir_conf;
@@ -229,25 +222,25 @@
 
 const char *set_wrapper_config(cmd_parms * cmd, void *dummy,
                                const char *wrapper, const char *extension, const char* virtual);
-fcgid_wrapper_conf *get_wrapper_info(const char *cgipath, request_rec * r);
+fcgid_cmd_conf *get_wrapper_info(const char *cgipath, request_rec * r);
 
 const char *set_authenticator_info(cmd_parms * cmd, void *config,
                                    const char *arg);
 const char *set_authenticator_authoritative(cmd_parms * cmd,
                                             void *config, int arg);
-fcgid_auth_conf *get_authenticator_info(request_rec * r, int *authoritative);
+fcgid_cmd_conf *get_authenticator_info(request_rec * r, int *authoritative);
 
 const char *set_authorizer_info(cmd_parms * cmd, void *config,
                                 const char *arg);
 const char *set_authorizer_authoritative(cmd_parms * cmd,
                                          void *config, int arg);
-fcgid_auth_conf *get_authorizer_info(request_rec * r, int *authoritative);
+fcgid_cmd_conf *get_authorizer_info(request_rec * r, int *authoritative);
 
 const char *set_access_info(cmd_parms * cmd, void *config,
                             const char *arg);
 const char *set_access_authoritative(cmd_parms * cmd,
                                      void *config, int arg);
-fcgid_auth_conf *get_access_info(request_rec * r, int *authoritative);
+fcgid_cmd_conf *get_access_info(request_rec * r, int *authoritative);
 
 const char *set_php_fix_pathinfo_enable(cmd_parms * cmd, void *dummy,
                                         const char *arg);
Index: fcgid_spawn_ctl.c
===================================================================
--- fcgid_spawn_ctl.c	(revision 905510)
+++ fcgid_spawn_ctl.c	(working copy)
@@ -17,6 +17,9 @@
 
 #include "fcgid_spawn_ctl.h"
 #include "fcgid_conf.h"
+
+#include "apr_strings.h"
+
 #define REGISTER_LIFE 1
 #define REGISTER_DEATH 2
 
@@ -25,7 +28,7 @@
     dev_t deviceid;
     uid_t uid;
     gid_t gid;
-    apr_size_t share_grp_id;
+    const char *cmdline;
     const char *virtualhost;
     int score;
     int process_counter;
@@ -50,13 +53,13 @@
     if (!g_stat_pool || !procnode)
         abort();
 
-    /* Can I find the node base on inode, device id and share group id? */
+    /* Can I find the node base on inode, device id and cmdline? */
     previous_node = g_stat_list_header;
     for (current_node = previous_node;
          current_node != NULL; current_node = current_node->next) {
         if (current_node->inode == procnode->inode
             && current_node->deviceid == procnode->deviceid
-            && current_node->share_grp_id == procnode->share_grp_id
+            && !strcmp(current_node->cmdline, procnode->cmdline)
             && current_node->virtualhost == procnode->virtualhost
             && current_node->uid == procnode->uid
             && current_node->gid == procnode->gid)
@@ -92,7 +95,7 @@
         current_node = apr_pcalloc(g_stat_pool, sizeof(*current_node));
         current_node->deviceid = procnode->deviceid;
         current_node->inode = procnode->inode;
-        current_node->share_grp_id = procnode->share_grp_id;
+        current_node->cmdline = apr_pstrdup(g_stat_pool, procnode->cmdline);
         current_node->virtualhost = procnode->virtualhost;
         current_node->uid = procnode->uid;
         current_node->gid = procnode->gid;
@@ -156,12 +159,12 @@
     if (!command || !g_stat_pool)
         return 1;
 
-    /* Can I find the node base on inode, device id and share group id? */
+    /* Can I find the node base on inode, device id and cmdline? */
     for (current_node = g_stat_list_header;
          current_node != NULL; current_node = current_node->next) {
         if (current_node->inode == command->inode
             && current_node->deviceid == command->deviceid
-            && current_node->share_grp_id == command->share_grp_id
+            && !strcmp(current_node->cmdline, command->cmdline)
             && current_node->virtualhost == command->virtualhost
             && current_node->uid == command->uid
             && current_node->gid == command->gid)
@@ -220,12 +223,12 @@
     if (!g_stat_pool || !procnode)
         return 0;
 
-    /* Can I find the node base on inode, device id and share group id? */
+    /* Can I find the node base on inode, device id and cmdline? */
     for (current_node = g_stat_list_header;
          current_node != NULL; current_node = current_node->next) {
         if (current_node->inode == procnode->inode
             && current_node->deviceid == procnode->deviceid
-            && current_node->share_grp_id == procnode->share_grp_id
+            && !strcmp(current_node->cmdline, procnode->cmdline)
             && current_node->virtualhost == procnode->virtualhost
             && current_node->uid == procnode->uid
             && current_node->gid == procnode->gid)
Index: fcgid_proc.h
===================================================================
--- fcgid_proc.h	(revision 905510)
+++ fcgid_proc.h	(working copy)
@@ -26,7 +26,7 @@
     apr_table_t *proc_environ;
     server_rec *main_server;
     apr_pool_t *configpool;
-    char *cgipath;
+    const char *cgipath;
     uid_t uid;                  /* For suEXEC */
     gid_t gid;                  /* For suEXEC */
     int userdir;                /* For suEXEC */
@@ -39,7 +39,7 @@
     request_rec *request;
 } fcgid_ipc;
 
-apr_status_t proc_spawn_process(char *lpszwapper,
+apr_status_t proc_spawn_process(const char *lpszwapper,
                                 fcgid_proc_info * procinfo,
                                 fcgid_procnode * procnode);
 
Index: fcgid_proctbl.h
===================================================================
--- fcgid_proctbl.h	(revision 905510)
+++ fcgid_proctbl.h	(working copy)
@@ -46,9 +46,9 @@
     char socket_path[_POSIX_PATH_MAX];  /* cgi application socket path */
     apr_ino_t inode;            /* cgi file inode */
     apr_dev_t deviceid;         /* cgi file device id */
+    char cmdline[_POSIX_PATH_MAX]; /* entire command line */
     gid_t gid;                  /* for suEXEC */
     uid_t uid;                  /* for suEXEC */
-    apr_size_t share_grp_id;    /* cgi wrapper share group id */
     const char *virtualhost;      /* the virtualhost this process belongs to */
     apr_time_t start_time;      /* the time of this process create */
     apr_time_t last_active_time;    /* the time this process last active */
Index: fcgid_pm_unix.c
===================================================================
--- fcgid_pm_unix.c	(revision 905510)
+++ fcgid_pm_unix.c	(working copy)
@@ -410,12 +410,9 @@
 }
 
 void procmgr_init_spawn_cmd(fcgid_command * command, request_rec * r,
-                            const char *argv0, dev_t deviceid,
-                            apr_ino_t inode, apr_size_t share_grp_id)
+                            fcgid_cmd_conf *cmd_conf)
 {
     ap_unix_identity_t *ugid;
-    fcgid_wrapper_conf *wrapperconf;
-    const char *cmd_to_spawn;
 
     memset(command, 0, sizeof(*command));
 
@@ -430,26 +427,13 @@
         command->userdir = 0;
     }
 
-    apr_cpystrn(command->cgipath, argv0, _POSIX_PATH_MAX);
-    command->deviceid = deviceid;
-    command->inode = inode;
-    command->share_grp_id = share_grp_id;
+    apr_cpystrn(command->cgipath, cmd_conf->cgipath, _POSIX_PATH_MAX);
+    apr_cpystrn(command->cmdline, cmd_conf->cmdline, _POSIX_PATH_MAX);
+    command->deviceid = cmd_conf->deviceid;
+    command->inode = cmd_conf->inode;
     command->virtualhost = r->server->server_hostname;
 
-    /* Update fcgid_command with wrapper info */
-    command->wrapper_cmdline[0] = '\0';
-    if ((wrapperconf = get_wrapper_info(argv0, r))) {
-        apr_cpystrn(command->wrapper_cmdline, wrapperconf->args, _POSIX_PATH_MAX);
-        command->deviceid = wrapperconf->deviceid;
-        command->inode = wrapperconf->inode;
-        command->share_grp_id = wrapperconf->share_group_id;
-        cmd_to_spawn = wrapperconf->exe;
-    }
-    else {
-        cmd_to_spawn = command->cgipath;
-    }
-
-    get_cmd_options(r, cmd_to_spawn, &command->cmdopts, &command->cmdenv);
+    get_cmd_options(r, command->cgipath, &command->cmdopts, &command->cmdenv);
 }
 
 apr_status_t procmgr_post_spawn_cmd(fcgid_command * command,
Index: mod_fcgid.c
===================================================================
--- mod_fcgid.c	(revision 905510)
+++ mod_fcgid.c	(working copy)
@@ -20,6 +20,7 @@
 #include "http_protocol.h"
 #include "ap_mmn.h"
 #include "apr_buckets.h"
+#include "apr_strings.h"
 #include "apr_thread_proc.h"
 #include "mod_cgi.h"
 #include "mod_status.h"
@@ -146,7 +147,7 @@
     apr_pool_t *p;
     apr_status_t rv;
     int http_retcode;
-    fcgid_wrapper_conf *wrapper_conf;
+    fcgid_cmd_conf *wrapper_conf;
 
     if (strcmp(r->handler, "fcgid-script"))
         return DECLINED;
@@ -199,17 +200,24 @@
                           r->filename);
             return HTTP_INTERNAL_SERVER_ERROR;
         }
-    }
 
-    /* Check request like "http://localhost/cgi-bin/a.exe/defghi" */
-    if (!wrapper_conf && r->finfo.inode == 0 && r->finfo.device ==
0) {
-        if ((rv =
-             apr_stat(&r->finfo, command, APR_FINFO_IDENT,
-                      r->pool)) != APR_SUCCESS) {
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING, rv, r,
-                          "mod_fcgid: can't get %s file info", command);
-            return HTTP_NOT_FOUND;
+        /* Check request like "http://localhost/cgi-bin/a.exe/defghi" */
+        if (r->finfo.inode == 0 && r->finfo.device == 0) {
+            if ((rv =
+                 apr_stat(&r->finfo, command, APR_FINFO_IDENT,
+                          r->pool)) != APR_SUCCESS) {
+                ap_log_rerror(APLOG_MARK, APLOG_WARNING, rv, r,
+                              "mod_fcgid: can't get %s file info", command);
+                return HTTP_NOT_FOUND;
+            }
         }
+
+        wrapper_conf = apr_pcalloc(r->pool, sizeof(*wrapper_conf));
+
+        wrapper_conf->cgipath = apr_pstrdup(r->pool, command);
+        wrapper_conf->cmdline = wrapper_conf->cgipath;
+        wrapper_conf->inode = r->finfo.inode;
+        wrapper_conf->deviceid = r->finfo.device;
     }
 
     ap_add_common_vars(r);
@@ -231,8 +239,7 @@
     ap_add_output_filter_handle(fcgid_filter_handle, NULL, r,
                                 r->connection);
 
-    http_retcode =
-        bridge_request(r, FCGI_RESPONDER, command, NULL, wrapper_conf);
+    http_retcode = bridge_request(r, FCGI_RESPONDER, wrapper_conf);
     return (http_retcode == HTTP_OK ? OK : http_retcode);
 }
 
@@ -246,8 +253,9 @@
         return (*e1)->gid > (*e2)->gid ? 1 : -1;
     if ((*e1)->uid != (*e2)->uid)
         return (*e1)->uid > (*e2)->uid ? 1 : -1;
-    if ((*e1)->share_grp_id != (*e2)->share_grp_id)
-        return (*e1)->share_grp_id > (*e2)->share_grp_id ? 1 : -1;
+    cmp = strcmp((*e1)->cmdline, (*e2)->cmdline);
+    if (cmp != 0)
+        return cmp;
     if ((*e1)->virtualhost != (*e2)->virtualhost)
         return (*e1)->virtualhost > (*e2)->virtualhost ? 1 : -1;
     if ((*e1)->diewhy != (*e2)->diewhy)
@@ -294,7 +302,7 @@
     apr_dev_t last_deviceid = 0;
     gid_t last_gid = 0;  
     uid_t last_uid = 0;
-    apr_size_t last_share_grp_id = 0;
+    const char *last_cmdline = "";
     apr_time_t now;
     const char *last_virtualhost = NULL;
     const char *basename, *tmpbasename;
@@ -368,7 +376,7 @@
     	current_node = ar[index];
         if (current_node->inode != last_inode || current_node->deviceid != last_deviceid
             || current_node->gid != last_gid || current_node->uid != last_uid
-            || current_node->share_grp_id != last_share_grp_id
+            || strcmp(current_node->cmdline, last_cmdline)
             || current_node->virtualhost != last_virtualhost) {
             if (index != 0)
                  ap_rputs("</table>\n\n", r);
@@ -394,7 +402,7 @@
             last_deviceid = current_node->deviceid;
             last_gid = current_node->gid;
             last_uid = current_node->uid;
-            last_share_grp_id = current_node->share_grp_id;
+            last_cmdline = current_node->cmdline;
             last_virtualhost = current_node->virtualhost;
         }
 
@@ -431,8 +439,7 @@
     int res = 0;
     const char *password = NULL;
     apr_table_t *saved_subprocess_env = NULL;
-    fcgid_wrapper_conf *wrapper_conf;
-    fcgid_auth_conf *authenticator_info;
+    fcgid_cmd_conf *authenticator_info;
     int authoritative;
 
     authenticator_info = get_authenticator_info(r, &authoritative);
@@ -441,9 +448,6 @@
     if (authenticator_info == NULL)
         return DECLINED;
 
-    /* Check wrapper */
-    wrapper_conf = get_wrapper_info(authenticator_info->path, r);
-
     /* Get the user password */
     if ((res = ap_get_basic_auth_pw(r, &password)) != OK)
         return res;
@@ -476,9 +480,7 @@
     apr_table_set(r->subprocess_env, "HTTP_CONNECTION", "close");
 
     /* Handle the request */
-    res =
-        bridge_request(r, FCGI_AUTHORIZER, NULL, authenticator_info,
-                       wrapper_conf);
+    res = bridge_request(r, FCGI_AUTHORIZER, authenticator_info);
 
     /* Restore r->subprocess_env */
     r->subprocess_env = saved_subprocess_env;
@@ -525,8 +527,7 @@
 {
     int res = 0;
     apr_table_t *saved_subprocess_env = NULL;
-    fcgid_wrapper_conf *wrapper_conf;
-    fcgid_auth_conf *authorizer_info;
+    fcgid_cmd_conf *authorizer_info;
     int authoritative;
 
     authorizer_info = get_authorizer_info(r, &authoritative);
@@ -535,9 +536,6 @@
     if (authorizer_info == NULL)
         return DECLINED;
 
-    /* Check wrapper */
-    wrapper_conf = get_wrapper_info(authorizer_info->path, r);
-
     /* Save old process environment */
     saved_subprocess_env = apr_table_copy(r->pool, r->subprocess_env);
 
@@ -565,9 +563,7 @@
     apr_table_set(r->subprocess_env, "HTTP_CONNECTION", "close");
 
     /* Handle the request */
-    res =
-        bridge_request(r, FCGI_AUTHORIZER, NULL, authorizer_info,
-                       wrapper_conf);
+    res = bridge_request(r, FCGI_AUTHORIZER, authorizer_info);
 
     /* Restore r->subprocess_env */
     r->subprocess_env = saved_subprocess_env;
@@ -614,8 +610,7 @@
 {
     int res = 0;
     apr_table_t *saved_subprocess_env = NULL;
-    fcgid_wrapper_conf *wrapper_conf;
-    fcgid_auth_conf *access_info;
+    fcgid_cmd_conf *access_info;
     int authoritative;
 
     access_info = get_access_info(r, &authoritative);
@@ -624,9 +619,6 @@
     if (access_info == NULL)
         return DECLINED;
 
-    /* Check wrapper */
-    wrapper_conf = get_wrapper_info(access_info->path, r);
-
     /* Save old process environment */
     saved_subprocess_env = apr_table_copy(r->pool, r->subprocess_env);
 
@@ -655,9 +647,7 @@
     apr_table_set(r->subprocess_env, "HTTP_CONNECTION", "close");
 
     /* Handle the request */
-    res =
-        bridge_request(r, FCGI_AUTHORIZER, NULL, access_info,
-                       wrapper_conf);
+    res = bridge_request(r, FCGI_AUTHORIZER, access_info);
 
     /* Restore r->subprocess_env */
     r->subprocess_env = saved_subprocess_env;
Index: fcgid_pm.h
===================================================================
--- fcgid_pm.h	(revision 905510)
+++ fcgid_pm.h	(working copy)
@@ -22,10 +22,9 @@
 
 typedef struct {
     char cgipath[_POSIX_PATH_MAX];
-    char wrapper_cmdline[_POSIX_PATH_MAX];
+    char cmdline[_POSIX_PATH_MAX];
     apr_ino_t inode;
     dev_t deviceid;
-    apr_size_t share_grp_id;
     const char *virtualhost;  /* Virtualhost granularity */
     uid_t uid;                  /* For suEXEC */
     gid_t gid;                  /* For suEXEC */
@@ -37,8 +36,7 @@
 } fcgid_command;
 
 void procmgr_init_spawn_cmd(fcgid_command * command, request_rec * r,
-                            const char *argv0, dev_t deviceid,
-                            apr_ino_t inode, apr_size_t share_grp_id);
+                            fcgid_cmd_conf *cmd_conf);
 apr_status_t procmgr_post_spawn_cmd(fcgid_command * command,
                                     request_rec * r);
 apr_status_t procmgr_peek_cmd(fcgid_command * command,
Index: fcgid_bridge.c
===================================================================
--- fcgid_bridge.c	(revision 905510)
+++ fcgid_bridge.c	(working copy)
@@ -44,7 +44,7 @@
     apr_dev_t deviceid = command->deviceid;
     uid_t uid = command->uid;
     gid_t gid = command->gid;
-    apr_size_t share_grp_id = command->share_grp_id;
+    const char *cmdline = command->cmdline;
     const char *virtualhost = command->virtualhost;
 
     proc_table = proctable_get_table_array();
@@ -58,7 +58,7 @@
 
         if (current_node->inode == inode
             && current_node->deviceid == deviceid
-            && current_node->share_grp_id == share_grp_id
+            && !strcmp(current_node->cmdline, cmdline)
             && current_node->virtualhost == virtualhost
             && current_node->uid == uid && current_node->gid == gid)
{
             /* Unlink from idle list */
@@ -138,7 +138,7 @@
     while (current_node != proc_table) {
         if (current_node->inode == command->inode
             && current_node->deviceid == command->deviceid
-            && current_node->share_grp_id == command->share_grp_id
+            && !strcmp(current_node->cmdline, command->cmdline)
             && current_node->virtualhost == command->virtualhost
             && current_node->uid == command->uid
             && current_node->gid == command->gid) {
@@ -282,17 +282,12 @@
 }
 
 static int
-handle_request(request_rec * r, int role, const char *argv0,
-               fcgid_auth_conf * auth_conf,
-               fcgid_wrapper_conf * wrapper_conf,
+handle_request(request_rec * r, int role, fcgid_cmd_conf *cmd_conf,
                apr_bucket_brigade * output_brigade)
 {
     apr_pool_t *request_pool = r->main ? r->main->pool : r->pool;
     fcgid_command fcgi_request;
     fcgid_bucket_ctx *bucket_ctx;
-    apr_ino_t inode;
-    apr_dev_t deviceid;
-    apr_size_t shareid;
     int i, j, cond_status;
     apr_status_t rv;
     apr_bucket_brigade *brigade_stdout;
@@ -305,26 +300,12 @@
     apr_pool_cleanup_register(request_pool, bucket_ctx,
                               bucket_ctx_cleanup, apr_pool_cleanup_null);
 
-    if (role == FCGI_AUTHORIZER) {
-        argv0 = auth_conf->path;
-        inode = wrapper_conf ? wrapper_conf->inode : auth_conf->inode;
-        deviceid = wrapper_conf ? wrapper_conf->deviceid : auth_conf->deviceid;
-        shareid = wrapper_conf ? wrapper_conf->share_group_id
-                               : auth_conf->share_group_id;
-    }
-    else {
-        inode = wrapper_conf ? wrapper_conf->inode : r->finfo.inode;
-        deviceid = wrapper_conf ? wrapper_conf->deviceid : r->finfo.device;
-        shareid = wrapper_conf ? wrapper_conf->share_group_id : 0;
-    }
-
     /* Try to get a connected ipc handle */
     for (i = 0; i < FCGID_REQUEST_COUNT; i++) {
         /* Apply a free process slot, send a spawn request if I can't get one */
         for (j = 0; j < FCGID_APPLY_TRY_COUNT; j++) {
             /* Init spawn request */
-            procmgr_init_spawn_cmd(&fcgi_request, r, argv0, deviceid,
-                                   inode, shareid);
+            procmgr_init_spawn_cmd(&fcgi_request, r, cmd_conf);
 
             bucket_ctx->ipc.connect_timeout =
                 fcgi_request.cmdopts.ipc_connect_timeout;
@@ -368,7 +349,8 @@
     /* Now I get a connected ipc handle */
     if (!bucket_ctx->procnode) {
         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
-                      "mod_fcgid: can't apply process slot for %s", argv0);
+                      "mod_fcgid: can't apply process slot for %s",
+                      cmd_conf->cmdline);
         return HTTP_SERVICE_UNAVAILABLE;
     }
     bucket_ctx->active_time = bucket_ctx->procnode->last_active_time =
@@ -626,9 +608,7 @@
     return 0;
 }
 
-int bridge_request(request_rec * r, int role, const char *argv0,
-                   fcgid_auth_conf * auth_conf,
-                   fcgid_wrapper_conf * wrapper_conf)
+int bridge_request(request_rec * r, int role, fcgid_cmd_conf *cmd_conf)
 {
     apr_pool_t *request_pool = r->main ? r->main->pool : r->pool;
     apr_bucket_brigade *output_brigade;
@@ -663,6 +643,5 @@
     APR_BRIGADE_INSERT_TAIL(output_brigade, bucket_eos);
 
     /* Bridge the request */
-    return handle_request(r, role, argv0, auth_conf, wrapper_conf,
-                          output_brigade);
+    return handle_request(r, role, cmd_conf, output_brigade);
 }
Index: fcgid_bridge.h
===================================================================
--- fcgid_bridge.h	(revision 905510)
+++ fcgid_bridge.h	(working copy)
@@ -24,8 +24,6 @@
 #include "fcgid_conf.h"
 
 apr_status_t bucket_ctx_cleanup(void *thectx);
-int bridge_request(request_rec * r, int role, const char *argv0,
-                   fcgid_auth_conf * auth_conf,
-                   fcgid_wrapper_conf * wrapper_conf);
+int bridge_request(request_rec * r, int role, fcgid_cmd_conf *cmd_conf);
 
 #endif

Mime
View raw message