On Mon, Sep 23, 2013 at 10:02 AM, <[email protected]> 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/
