httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dietz, Phil E." <PEDi...@west.com>
Subject RE: [RFC] [PATCH] mod_cgi adding ap_hook_cgi_fault()
Date Wed, 05 Feb 2003 17:18:58 GMT
This is incorrect.  

Only 1 module would able to read the stderr bucket brigade.  All other modules would not see
the data in the brigade.

Instead stderr needs to be put into its own special memory structure, and passed via a pointer.

I'll post a patch in a few weeks.

> -----Original Message-----
> From:	Dietz, Phil E. 
> Sent:	Friday, January 10, 2003 4:10 PM
> To:	'dev@httpd.apache.org'
> Subject:	[RFC] [PATCH] mod_cgi adding ap_hook_cgi_fault()
> 
> With this patch, modules can hook into the CGI fault path and:
>   - escalate cgi errors  (ie email, page, self-heal, etc.)
>   - write reports to places other than ScriptLog  (ie back to the user)
>   - tie into monitoring tools (ie a mod_unicenter.c)
> 
> 
> The following patch to mod_cgi.c (off apache2.0.43):
>  - all of stderr from a cgi_app is stored into it's own bucket brigade.
>  - a hook called ap_hook_cgi_fault() allows modules to access the cgi parameters when
ap_scan_script_header_err_brigade() returns error (including the stderr brigade).
>  - log_script() was converted to use this hook.  log_script() is set as the default handler
for cgi_fault.
>  - log_script_err() was changed to use bucket brigades.
> 
> 
> Parameters for the cgi_fault hook are the same as log_script() with exception that stderr
is a bucket brigade instead of a file_t handle.
> 
> I've lightly tested on linux with mod_so.  A "modules/test" application is available.
> If approved, I'll convert mod_cgid.
> 
> 
> --- httpd-2.0.43/modules/generators/mod_cgi.h  Wed Jun  5 20:17:50 2002
> +++ mod_cgi.h   Fri Jan 10 16:57:03 2003
> @@ -59,6 +59,7 @@
>  #ifndef _MOD_CGI_H
>  #define _MOD_CGI_H 1
>  
> +#include "ap_config.h"
>  #include "mod_include.h"
>  
>  typedef enum {RUN_AS_SSI, RUN_AS_CGI} prog_types;
> @@ -76,6 +77,27 @@
>      ap_filter_t         *next;
>  } cgi_exec_info_t;
>  
> +typedef struct {
> +    const char *logname;
> +    long        logbytes;
> +    apr_size_t  bufbytes;
> +} cgi_server_conf;
> +
> +/**
> + * A Hook to tie into the path when cgi-apps fail.
> + * @param r Pointer to the request
> + * @param conf Pointer to the cgi config structure
> + * @param ret The reason why the cgi app failed
> + * @param dbuf Pointer to the any data posted to the app
> + * @param sbuf Pointer to the any output received from the cgi app
> + * @param bb The bucket with the stdout from the cgi app
> + * @param ebb The bucket with the stderr from the cgi app
> + * Note the bucket brigades are created and destroyed by mod_cgi.c
> + */ 
> +AP_DECLARE_HOOK(void, cgi_fault,(request_rec *r, cgi_server_conf *conf, int ret,
> +                        char *dbuf, const char *sbuf, apr_bucket_brigade *bb,
> +                        apr_bucket_brigade *ebb))
> +
>  /**
>   * Registerable optional function to override CGI behavior;
>   * Reprocess the command and arguments to execute the given CGI script.
> @@ -97,4 +119,7 @@
>                           request_rec *r, apr_pool_t *p, 
>                           cgi_exec_info_t *e_info));
>  
> +const void *ap_hack_ap_hook_cgi_fault = (const void *)ap_hook_cgi_fault;
> +const void *ap_hack_ap_run_cgi_fault  = (const void *)ap_run_cgi_fault;
> +
>  #endif /* _MOD_CGI_H */
> 
> --- httpd-2.0.43/modules/generators/mod_cgi.c  Tue Jul 30 14:18:03 2002
> +++ mod_cgi.c   Fri Jan 10 16:57:32 2003
> @@ -102,6 +102,11 @@
>  static APR_OPTIONAL_FN_TYPE(ap_ssi_parse_string) *cgi_pfn_ps;
>  static APR_OPTIONAL_FN_TYPE(ap_cgi_build_command) *cgi_build_command;
>  
> +/* Allow other modules to hook into the ScriptLog routine */
> +APR_HOOK_STRUCT(
> +    APR_HOOK_LINK(cgi_fault)
> +)
> +
>  /* Read and discard the data in the brigade produced by a CGI script */
>  static void discard_script_output(apr_bucket_brigade *bb);
>  
> @@ -122,12 +127,6 @@
>  #define DEFAULT_LOGBYTES 10385760
>  #define DEFAULT_BUFBYTES 1024
>  
> -typedef struct {
> -    const char *logname;
> -    long        logbytes;
> -    apr_size_t  bufbytes;
> -} cgi_server_conf;
> -
>  static void *create_cgi_config(apr_pool_t *p, server_rec *s)
>  {
>      cgi_server_conf *c =
> @@ -234,25 +233,34 @@
>  
>  /* Soak up stderr from a script and redirect it to the error log. 
>   */
> -static void log_script_err(request_rec *r, apr_file_t *script_err)
> +static void log_script_err(request_rec *r, apr_bucket_brigade *ebb)
>  {
> -    char argsbuffer[HUGE_STRING_LEN];
> +    const char *buf;
> +    apr_size_t len;
> +    apr_bucket *e;
> +    apr_status_t rv;
>      char *newline;
>  
> -    while (apr_file_gets(argsbuffer, HUGE_STRING_LEN,
> -                         script_err) == APR_SUCCESS) {
> -        newline = strchr(argsbuffer, '\n');
> +    APR_BRIGADE_FOREACH(e, ebb) {
> +        if (APR_BUCKET_IS_EOS(e)) {
> +            break;
> +        }
> +        rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ);
> +        if (rv != APR_SUCCESS || (len == 0)) {
> +            break;
> +        }
> +        newline = strchr(buf, '\n');
>          if (newline) {
>              *newline = '\0';
>          }
>          ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 
> -                      "%s", argsbuffer);            
> +                      "%s", buf);            
>      }
>  }
>  
> -static int log_script(request_rec *r, cgi_server_conf * conf, int ret,
> +static void log_script(request_rec *r, cgi_server_conf * conf, int ret,
>                        char *dbuf, const char *sbuf, apr_bucket_brigade *bb, 
> -                      apr_file_t *script_err)
> +                      apr_bucket_brigade *ebb)
>  {
>      const apr_array_header_t *hdrs_arr = apr_table_elts(r->headers_in);
>      const apr_table_entry_t *hdrs = (const apr_table_entry_t *) hdrs_arr->elts;
> @@ -276,9 +284,8 @@
>                         APR_APPEND|APR_WRITE|APR_CREATE, APR_OS_DEFAULT,
>                         r->pool) != APR_SUCCESS)) {
>          /* Soak up script output */
> -        discard_script_output(bb);
> -        log_script_err(r, script_err);
> -        return ret;
> +        log_script_err(r, ebb);
> +        return;
>      }
>  
>      /* "%% [Wed Jun 19 10:53:21 1996] GET /cgi-bin/printenv HTTP/1.0" */
> @@ -329,21 +336,24 @@
>          apr_file_puts("\n", f);
>      }
>  
> -    if (apr_file_gets(argsbuffer, HUGE_STRING_LEN, script_err) == APR_SUCCESS) {
> -        apr_file_puts("%stderr\n", f);
> -        apr_file_puts(argsbuffer, f);
> -        while (apr_file_gets(argsbuffer, HUGE_STRING_LEN,
> -                             script_err) == APR_SUCCESS) {
> -            apr_file_puts(argsbuffer, f);
> +    first = 1;
> +    APR_BRIGADE_FOREACH(e, ebb) {
> +        if (APR_BUCKET_IS_EOS(e)) {
> +            break;
>          }
> +        rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ);
> +        if (rv != APR_SUCCESS || (len == 0)) {
> +            break;
> +        }
> +        if (first) {
> +            apr_file_puts("%stderr\n", f);
> +            first = 0;
> +        }
> +        apr_file_write(f, buf, &len);
>          apr_file_puts("\n", f);
>      }
> -
> -    apr_brigade_destroy(bb);
> -    apr_file_close(script_err);
> -
>      apr_file_close(f);
> -    return ret;
> +    return;
>  }
>  
>  
> @@ -578,6 +588,7 @@
>      char *dbuf = NULL;
>      apr_file_t *script_out = NULL, *script_in = NULL, *script_err = NULL;
>      apr_bucket_brigade *bb;
> +    apr_bucket_brigade *ebb;
>      apr_bucket *b;
>      int is_included;
>      int seen_eos, child_stopped_reading;
> @@ -585,6 +596,7 @@
>      cgi_server_conf *conf;
>      apr_status_t rv;
>      cgi_exec_info_t e_info;
> +    conn_rec *c = r->connection;
>  
>      if(strcmp(r->handler, CGI_MAGIC_TYPE) && strcmp(r->handler, "cgi-script"))
>          return DECLINED;
> @@ -666,7 +678,7 @@
>      /* Transfer any put/post args, CERN style...
>       * Note that we already ignore SIGPIPE in the core server.
>       */
> -    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);> 
> +    bb = apr_brigade_create(r->pool, c->bucket_alloc);
>      seen_eos = 0;
>      child_stopped_reading = 0;
>      if (conf->logname) {
> @@ -739,9 +751,17 @@
>      apr_file_flush(script_out);
>      apr_file_close(script_out);
>  
> +    /* Handle cgi-errors and cgi-control messages... */
> +    if (script_err) {
> +        ebb = apr_brigade_create(r->pool, c->bucket_alloc);
> +        b = apr_bucket_pipe_create(script_err, c->bucket_alloc);
> +        APR_BRIGADE_INSERT_TAIL(ebb, b);
> +        b = apr_bucket_eos_create(c->bucket_alloc);
> +        APR_BRIGADE_INSERT_TAIL(ebb, b);
> +    }
> +
>      /* Handle script return... */
>      if (script_in && !nph) {
> -        conn_rec *c = r->connection;
>          const char *location;
>          char sbuf[MAX_STRING_LEN];
>          int ret;
> @@ -751,16 +771,39 @@
>          b = apr_bucket_eos_create(c->bucket_alloc);
>          APR_BRIGADE_INSERT_TAIL(bb, b);
>  
> +        /* Run all stderr hooks */
> +
>          if ((ret = ap_scan_script_header_err_brigade(r, bb, sbuf))) {
> -            return log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
> +
> +            /* cgi-error or cgi-control message found.  Run all hooks */
> +            ap_run_cgi_fault(r, conf, ret, dbuf, sbuf, bb, ebb);
> +            /* return log_script(r, conf, ret, dbuf, sbuf, bb, script_err); */
> +
> +            /* Soak up script output */
> +            discard_script_output(bb);
> +            apr_brigade_destroy(bb);
> +
> +            /* Soak up script cgi-error output */
> +            discard_script_output(ebb);
> +            apr_brigade_destroy(ebb);
> +
> +            apr_file_close(script_err);
> +            return ret;
>          }
>  
>          location = apr_table_get(r->headers_out, "Location");
>  
>          if (location && location[0] == '/' && r->status == 200) {
> +            log_script_err(r, ebb);
> +            /* Soak up script output */
>              discard_script_output(bb);
>              apr_brigade_destroy(bb);
> -            log_script_err(r, script_err);
> +
> +            /* Soak up script cgi-error output */
> +            discard_script_output(ebb);
> +            apr_brigade_destroy(ebb);
> +
> +            apr_file_close(script_err);
>              /* This redirect needs to be a GET no matter what the original
>               * method was.
>               */
> @@ -782,19 +825,27 @@
>               */
>              discard_script_output(bb);
>              apr_brigade_destroy(bb);
> +            discard_script_output(ebb);
> +            apr_brigade_destroy(ebb);
> +            apr_file_close(script_err);
>              return HTTP_MOVED_TEMPORARILY;
>          }
>  
>          ap_pass_brigade(r->output_filters, bb);
>  
> -        log_script_err(r, script_err);
> +        log_script_err(r, ebb);
> +        discard_script_output(ebb);
> +        apr_brigade_destroy(ebb);
>          apr_file_close(script_err);
> +        return OK;
>      }
>  
>      if (script_in && nph) {
> -        conn_rec *c = r->connection;
>          struct ap_filter_t *cur;
>          
> +        discard_script_output(ebb);
> +        apr_brigade_destroy(ebb);
> +
>          /* get rid of all filters up through protocol...  since we
>           * haven't parsed off the headers, there is no way they can
>           * work
> @@ -1060,6 +1111,7 @@
>  {
>      static const char * const aszPre[] = { "mod_include.c", NULL };
>      ap_hook_handler(cgi_handler, NULL, NULL, APR_HOOK_MIDDLE);
> +    ap_hook_cgi_fault(log_script, NULL, NULL, APR_HOOK_MIDDLE);
>      ap_hook_post_config(cgi_post_config, aszPre, NULL, APR_HOOK_REALLY_FIRST);
>  }
>  
> @@ -1073,3 +1125,10 @@
>      cgi_cmds,                    /* command apr_table_t */
>      register_hooks               /* register hooks */
>  };
> +
> +
> +AP_IMPLEMENT_HOOK_VOID(cgi_fault,
> +                        (request_rec *r, cgi_server_conf *conf, int ret,
> +                        char *dbuf, const char *sbuf, apr_bucket_brigade *bb, > 
> +                        apr_bucket_brigade *ebb), (r, conf, ret, dbuf, sbuf, bb, ebb))
> +

Mime
View raw message