httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r1525597 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/http_core.h include/httpd.h server/core.c server/log.c
Date Tue, 24 Sep 2013 00:32:26 GMT
On Mon, Sep 23, 2013 at 10:02 AM, <jkaluza@apache.org> wrote:

> Author: jkaluza
> Date: Mon Sep 23 14:02:27 2013
> New Revision: 1525597
>
> URL: http://svn.apache.org/r1525597
> Log:
> Add ap_errorlog_provider to make ErrorLog logging modular. Move
> syslog support from core to new mod_syslog.
>
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/docs/manual/mod/core.xml
>     httpd/httpd/trunk/include/http_core.h
>     httpd/httpd/trunk/include/httpd.h
>     httpd/httpd/trunk/server/core.c
>     httpd/httpd/trunk/server/log.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1525597&r1=1525596&r2=1525597&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Mon Sep 23 14:02:27 2013
> @@ -1,6 +1,9 @@
>                                                           -*- coding:
> utf-8 -*-
>  Changes with Apache 2.5.0
>
> +  *) core: Add ap_errorlog_provider to make ErrorLog logging modular. Move
> +     syslog support from core to new mod_syslog. [Jan Kaluza]
> +
>    *) mod_proxy_fcgi: Handle reading protocol data that is split between
>       packets.  [Jeff Trawick]
>
>
> Modified: httpd/httpd/trunk/docs/manual/mod/core.xml
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/core.xml?rev=1525597&r1=1525596&r2=1525597&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/docs/manual/mod/core.xml (original)
> +++ httpd/httpd/trunk/docs/manual/mod/core.xml Mon Sep 23 14:02:27 2013
> @@ -1298,16 +1298,19 @@ ErrorDocument 404 /cgi-bin/bad_urls.pl
>      more information.</p>
>
>      <p>Using <code>syslog</code> instead of a filename enables logging
> -    via syslogd(8) if the system supports it. The default is to use
> -    syslog facility <code>local7</code>, but you can override this by
> -    using the <code>syslog:<var>facility</var></code> syntax
where
> -    <var>facility</var> can be one of the names usually documented in
> +    via syslogd(8) if the system supports it and if
> <module>mod_syslog</module>
> +    is loaded. The default is to use syslog facility <code>local7</code>,
> +    but you can override this by using the
> <code>syslog:<var>facility</var></code>
> +    syntax where <var>facility</var> can be one of the names usually
> documented in
>      syslog(1).  The facility is effectively global, and if it is changed
>      in individual virtual hosts, the final facility specified affects the
>      entire server.</p>
>
>      <highlight language="config">ErrorLog syslog:user</highlight>
>
> +    <p>Additional modules can provide their own ErrorLog providers. The
> syntax
> +    is similar to <code>syslog</code> example above.</p>
> +
>      <p>SECURITY: See the <a
>      href="../misc/security_tips.html#serverroot">security tips</a>
>      document for details on why your security could be compromised
>
> Modified: httpd/httpd/trunk/include/http_core.h
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1525597&r1=1525596&r2=1525597&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/include/http_core.h (original)
> +++ httpd/httpd/trunk/include/http_core.h Mon Sep 23 14:02:27 2013
> @@ -833,8 +833,8 @@ typedef struct ap_errorlog_info {
>      /** apr error status related to the log message, 0 if no error */
>      apr_status_t status;
>
> -    /** 1 if logging to syslog, 0 otherwise */
> -    int using_syslog;
> +    /** 1 if logging using provider, 0 otherwise */
> +    int using_provider;
>      /** 1 if APLOG_STARTUP was set for the log message, 0 otherwise */
>      int startup;
>
> @@ -842,6 +842,30 @@ typedef struct ap_errorlog_info {
>      const char *format;
>  } ap_errorlog_info;
>
> +#define AP_ERRORLOG_PROVIDER_GROUP "error_log_writer"
> +#define AP_ERRORLOG_PROVIDER_VERSION "0"
> +#define AP_ERRORLOG_DEFAULT_PROVIDER "file"
> +
> +typedef struct ap_errorlog_provider ap_errorlog_provider;
> +
> +struct ap_errorlog_provider {
> +    /** Initializes the error log writer.
> +     * @param p The pool to create any storage from
> +     * @param s Server for which the logger is initialized
> +     * @return Pointer to handle passed later to writer() function
> +     */
> +    void * (*init)(apr_pool_t *p, server_rec *s);
> +
> +    /** Logs the error message to external error log.
> +     * @param info Context of the error message
> +     * @param handle Handle created by init() function
> +     * @param errstr Error message
> +     * @param len Length of the error message
> +     */
> +    apr_status_t (*writer)(const ap_errorlog_info *info, void *handle,
> +                           const char *errstr, int len);
>

Another thought here...

Some log writers may still need APR_EOL_STR, and it is awkward for the log
writer to add it in the efficient manner that core does.  Having a flags
field in the provider structure would solve that:

if (logf || (s->errorlog_provider->flags & PLEASE_ADD_APR_EOL_STR)) {
   logic-from-write_logline-moved-here
}



> +};
> +
>  /**
>   * callback function prototype for a external errorlog handler
>   * @note To avoid unbounded memory usage, these functions must not
> allocate
>
> Modified: httpd/httpd/trunk/include/httpd.h
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1525597&r1=1525596&r2=1525597&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/include/httpd.h (original)
> +++ httpd/httpd/trunk/include/httpd.h Mon Sep 23 14:02:27 2013
> @@ -1245,6 +1245,10 @@ struct server_rec {
>      apr_file_t *error_log;
>      /** The log level configuration */
>      struct ap_logconf log;
> +    /** External error log writer provider */
> +    struct ap_errorlog_provider *errorlog_provider;
> +    /** Handle to be passed to external log provider's logging method */
> +    void *errorlog_provider_handle;
>
>      /* Module-specific configuration for server, and defaults... */
>
>
> Modified: httpd/httpd/trunk/server/core.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1525597&r1=1525596&r2=1525597&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/server/core.c (original)
> +++ httpd/httpd/trunk/server/core.c Mon Sep 23 14:02:27 2013
> @@ -48,6 +48,7 @@
>  #include "mod_core.h"
>  #include "mod_proxy.h"
>  #include "ap_listen.h"
> +#include "ap_provider.h"
>
>  #include "mod_so.h" /* for ap_find_loaded_module_symbol */
>
> @@ -3955,6 +3956,49 @@ static apr_array_header_t *parse_errorlo
>      return a;
>  }
>
> +static const char *set_errorlog(cmd_parms *cmd, void *dummy, const char
> *arg1,
> +                                const char *arg2)
> +{
> +    ap_errorlog_provider *provider;
> +    cmd->server->errorlog_provider = NULL;
> +
> +    if (!arg2) {
> +        /* Stay backward compatible and check for "syslog" */
> +        if (strncmp("syslog", arg1, 6) == 0) {
> +            arg2 = arg1 + 7; /* skip the ':' if any */
> +            arg1 = "syslog";
> +        }
> +        else {
> +            /* Admin can define only "ErrorLog provider" and we should
> +             * still handle that using the defined provider, but with
> empty
> +             * error_fname. */
> +            provider = ap_lookup_provider(AP_ERRORLOG_PROVIDER_GROUP,
> arg1,
> +                                          AP_ERRORLOG_PROVIDER_VERSION);
> +            if (provider) {
> +                arg2 = "";
> +            }
> +            else {
> +                return set_server_string_slot(cmd, dummy, arg1);
> +            }
> +        }
> +    }
> +
> +    if (strcmp("file", arg1) == 0) {
> +        return set_server_string_slot(cmd, dummy, arg2);
> +    }
> +
> +    provider = ap_lookup_provider(AP_ERRORLOG_PROVIDER_GROUP, arg1,
> +                                    AP_ERRORLOG_PROVIDER_VERSION);
> +    if (!provider) {
> +        return apr_psprintf(cmd->pool,
> +                            "Unknown ErrorLog provider: %s",
> +                            arg1);
> +    }
> +
> +    cmd->server->errorlog_provider = provider;
> +    return set_server_string_slot(cmd, dummy, arg2);
> +}
> +
>  static const char *set_errorlog_format(cmd_parms *cmd, void *dummy,
>                                         const char *arg1, const char *arg2)
>  {
> @@ -4118,7 +4162,7 @@ AP_INIT_TAKE1("ServerRoot", set_server_r
>    "Common directory of server-related files (logs, confs, etc.)"),
>  AP_INIT_TAKE1("DefaultRuntimeDir", set_runtime_dir, NULL, RSRC_CONF |
> EXEC_ON_READ,
>    "Common directory for run-time files (shared memory, locks, etc.)"),
> -AP_INIT_TAKE1("ErrorLog", set_server_string_slot,
> +AP_INIT_TAKE12("ErrorLog", set_errorlog,
>    (void *)APR_OFFSETOF(server_rec, error_fname), RSRC_CONF,
>    "The filename of the error log"),
>  AP_INIT_TAKE12("ErrorLogFormat", set_errorlog_format, NULL, RSRC_CONF,
> @@ -4560,7 +4604,7 @@ AP_DECLARE(int) ap_sys_privileges_handle
>  static int check_errorlog_dir(apr_pool_t *p, server_rec *s)
>  {
>      if (!s->error_fname || s->error_fname[0] == '|'
> -        || strcmp(s->error_fname, "syslog") == 0) {
> +        || s->errorlog_provider != NULL) {
>          return APR_SUCCESS;
>      }
>      else {
> @@ -4986,7 +5030,7 @@ static void core_dump_config(apr_pool_t
>      apr_file_printf(out, "ServerRoot: \"%s\"\n", ap_server_root);
>      tmp = ap_server_root_relative(p, sconf->ap_document_root);
>      apr_file_printf(out, "Main DocumentRoot: \"%s\"\n", tmp);
> -    if (s->error_fname[0] != '|' && strcmp(s->error_fname, "syslog") !=
0)
> +    if (s->error_fname[0] != '|' && s->errorlog_provider == NULL)
>          tmp = ap_server_root_relative(p, s->error_fname);
>      else
>          tmp = s->error_fname;
>
> Modified: httpd/httpd/trunk/server/log.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=1525597&r1=1525596&r2=1525597&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/server/log.c (original)
> +++ httpd/httpd/trunk/server/log.c Mon Sep 23 14:02:27 2013
> @@ -53,6 +53,7 @@
>  #include "http_main.h"
>  #include "util_time.h"
>  #include "ap_mpm.h"
> +#include "ap_provider.h"
>
>  #if HAVE_GETTID
>  #include <sys/syscall.h>
> @@ -75,71 +76,6 @@ APR_HOOK_STRUCT(
>
>  int AP_DECLARE_DATA ap_default_loglevel = DEFAULT_LOGLEVEL;
>
> -#ifdef HAVE_SYSLOG
> -
> -static const TRANS facilities[] = {
> -    {"auth",    LOG_AUTH},
> -#ifdef LOG_AUTHPRIV
> -    {"authpriv",LOG_AUTHPRIV},
> -#endif
> -#ifdef LOG_CRON
> -    {"cron",    LOG_CRON},
> -#endif
> -#ifdef LOG_DAEMON
> -    {"daemon",  LOG_DAEMON},
> -#endif
> -#ifdef LOG_FTP
> -    {"ftp", LOG_FTP},
> -#endif
> -#ifdef LOG_KERN
> -    {"kern",    LOG_KERN},
> -#endif
> -#ifdef LOG_LPR
> -    {"lpr", LOG_LPR},
> -#endif
> -#ifdef LOG_MAIL
> -    {"mail",    LOG_MAIL},
> -#endif
> -#ifdef LOG_NEWS
> -    {"news",    LOG_NEWS},
> -#endif
> -#ifdef LOG_SYSLOG
> -    {"syslog",  LOG_SYSLOG},
> -#endif
> -#ifdef LOG_USER
> -    {"user",    LOG_USER},
> -#endif
> -#ifdef LOG_UUCP
> -    {"uucp",    LOG_UUCP},
> -#endif
> -#ifdef LOG_LOCAL0
> -    {"local0",  LOG_LOCAL0},
> -#endif
> -#ifdef LOG_LOCAL1
> -    {"local1",  LOG_LOCAL1},
> -#endif
> -#ifdef LOG_LOCAL2
> -    {"local2",  LOG_LOCAL2},
> -#endif
> -#ifdef LOG_LOCAL3
> -    {"local3",  LOG_LOCAL3},
> -#endif
> -#ifdef LOG_LOCAL4
> -    {"local4",  LOG_LOCAL4},
> -#endif
> -#ifdef LOG_LOCAL5
> -    {"local5",  LOG_LOCAL5},
> -#endif
> -#ifdef LOG_LOCAL6
> -    {"local6",  LOG_LOCAL6},
> -#endif
> -#ifdef LOG_LOCAL7
> -    {"local7",  LOG_LOCAL7},
> -#endif
> -    {NULL,      -1},
> -};
> -#endif
> -
>  static const TRANS priorities[] = {
>      {"emerg",   APLOG_EMERG},
>      {"alert",   APLOG_ALERT},
> @@ -395,29 +331,10 @@ static int open_error_log(server_rec *s,
>
>          s->error_log = dummy;
>      }
> -
> -#ifdef HAVE_SYSLOG
> -    else if (!strncasecmp(s->error_fname, "syslog", 6)) {
> -        if ((fname = strchr(s->error_fname, ':'))) {
> -            const TRANS *fac;
> -
> -            fname++;
> -            for (fac = facilities; fac->t_name; fac++) {
> -                if (!strcasecmp(fname, fac->t_name)) {
> -                    openlog(ap_server_argv0, LOG_NDELAY|LOG_CONS|LOG_PID,
> -                            fac->t_val);
> -                    s->error_log = NULL;
> -                    return OK;
> -                }
> -            }
> -        }
> -        else {
> -            openlog(ap_server_argv0, LOG_NDELAY|LOG_CONS|LOG_PID,
> LOG_LOCAL7);
> -        }
> -
> +    else if (s->errorlog_provider) {
> +        s->errorlog_provider_handle = s->errorlog_provider->init(p, s);
>          s->error_log = NULL;
>      }
> -#endif
>      else {
>          fname = ap_server_root_relative(p, s->error_fname);
>          if (!fname) {
> @@ -904,7 +821,7 @@ AP_DECLARE(void) ap_register_log_hooks(a
>
>  /*
>   * This is used if no error log format is defined and during startup.
> - * It automatically omits the timestamp if logging to syslog.
> + * It automatically omits the timestamp if logging using provider.
>   */
>  static int do_errorlog_default(const ap_errorlog_info *info, char *buf,
>                                 int buflen, int *errstr_start, int
> *errstr_end,
> @@ -917,7 +834,7 @@ static int do_errorlog_default(const ap_
>      char scratch[MAX_STRING_LEN];
>  #endif
>
> -    if (!info->using_syslog && !info->startup) {
> +    if (!info->using_provider && !info->startup) {
>          buf[len++] = '[';
>          len += log_ctime(info, "u", buf + len, buflen - len);
>          buf[len++] = ']';
> @@ -1076,22 +993,14 @@ static int do_errorlog_format(apr_array_
>  static void write_logline(char *errstr, apr_size_t len, apr_file_t *logf,
>                            int level)
>  {
> -    /* NULL if we are logging to syslog */
> -    if (logf) {
> -        /* Truncate for the terminator (as apr_snprintf does) */
> -        if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) {
> -            len = MAX_STRING_LEN - sizeof(APR_EOL_STR);
> -        }
> -        strcpy(errstr + len, APR_EOL_STR);
> -        apr_file_puts(errstr, logf);
> -        apr_file_flush(logf);
> -    }
> -#ifdef HAVE_SYSLOG
> -    else {
> -        syslog(level < LOG_PRIMASK ? level : APLOG_DEBUG, "%.*s",
> -               (int)len, errstr);
> -    }
> -#endif
> +
> +    /* Truncate for the terminator (as apr_snprintf does) */
> +    if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) {
> +        len = MAX_STRING_LEN - sizeof(APR_EOL_STR);
> +    }
> +    strcpy(errstr + len, APR_EOL_STR);
> +    apr_file_puts(errstr, logf);
> +    apr_file_flush(logf);
>  }
>
>  static void log_error_core(const char *file, int line, int module_index,
> @@ -1152,7 +1061,7 @@ static void log_error_core(const char *f
>          }
>          else {
>              /*
> -             * If we are doing syslog logging, don't log messages that are
> +             * If we are doing logging using provider, don't log messages
> that are
>               * above the module's log level (including a startup/shutdown
> notice)
>               */
>              if (level_and_mask > configured_level) {
> @@ -1194,7 +1103,7 @@ static void log_error_core(const char *f
>      info.file          = NULL;
>      info.line          = 0;
>      info.status        = 0;
> -    info.using_syslog  = (logf == NULL);
> +    info.using_provider= (logf == NULL);
>      info.startup       = ((level & APLOG_STARTUP) == APLOG_STARTUP);
>      info.format        = fmt;
>
> @@ -1272,7 +1181,14 @@ static void log_error_core(const char *f
>               */
>              continue;
>          }
> -        write_logline(errstr, len, logf, level_and_mask);
> +
> +        if (logf) {
> +            write_logline(errstr, len, logf, level_and_mask);
> +        }
> +        else {
> +            s->errorlog_provider->writer(&info,
> s->errorlog_provider_handle,
> +                                         errstr, len);
> +        }
>
>          if (done) {
>              /*
>
>
>


-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Mime
View raw message