You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Glenn <gs...@gluelogic.com> on 2003/10/13 23:30:24 UTC
Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28
I have privately received positive feedback from people using this patch
but have received very little feedback on this list, and none after my
repost on September 4th.
What must I do to get this into Apache 1.3?
If you won't accept it, please let me know what its shortcomings are
so that I may correct them.
Thank you.
Glenn
From gs-apache-dev@gluelogic.com Thu Sep 4 07:38:48 2003
Date: Thu, 4 Sep 2003 07:38:48 -0400
From: Glenn <gs...@gluelogic.com>
Subject: Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28
On Tue, Jul 08, 2003 at 12:41:09AM -0400, Glenn wrote:
> With the talk about a minor MMN bump, I put together this patch which
> adds a flag at the end of server_rec. This also changes ErrorLog to
> a TAKE12, with an optional style of "default" or "vhosts", where the
> "vhosts" includes the server name and port in the error log entries.
> The TAKE12 maintains backwards compatibility to existing config files.
>
> Comments appreciated on the method(s) that would most likely get this
> accepted into 1.3.28 or 1.3.29. (global flag, server_rec addition,
> other ...) Thanks!
I reworked this patch (from July!) for 1.3.28. Comments appreciated.
Cheers,
Glenn
diff -ru apache_1.3.28/src/include/httpd.h apache_1.3.28.new/src/include/httpd.h
--- apache_1.3.28/src/include/httpd.h 2003-07-16 16:20:26.000000000 -0400
+++ apache_1.3.28.new/src/include/httpd.h 2003-09-04 06:25:26.000000000 -0400
@@ -967,8 +967,13 @@
int limit_req_line; /* limit on size of the HTTP request line */
int limit_req_fieldsize; /* limit on size of any request header field */
int limit_req_fields; /* limit on number of request header fields */
+
+ int error_format_style; /* format style of error log entries */
};
+/* error format styles for log entry layout */
+enum { ERRORLOG_DEFAULT_STYLE = 0, ERRORLOG_WITH_VHOSTS = 1 };
+
/* These are more like real hosts than virtual hosts */
struct listen_rec {
listen_rec *next;
Only in apache_1.3.28.new/src/include: httpd.h.orig
diff -ru apache_1.3.28/src/main/http_core.c apache_1.3.28.new/src/main/http_core.c
--- apache_1.3.28/src/main/http_core.c 2003-07-07 09:02:28.000000000 -0400
+++ apache_1.3.28.new/src/main/http_core.c 2003-09-04 06:25:26.000000000 -0400
@@ -2076,6 +2076,35 @@
return NULL;
}
+static const char *set_error_log(cmd_parms *cmd, void *dummy,
+ char *arg1, char *arg2)
+{
+ const char *err = ap_check_cmd_context(cmd,
+ NOT_IN_DIR_LOC_FILE|NOT_IN_LIMIT);
+ if (err != NULL) {
+ return err;
+ }
+
+ if (arg2 == NULL) {
+ cmd->server->error_format_style = ERRORLOG_DEFAULT_STYLE;
+ cmd->server->error_fname = arg1;
+ }
+ else {
+ cmd->server->error_fname = arg2;
+ if (strcmp(arg1, "vhosts") == 0) {
+ cmd->server->error_format_style = ERRORLOG_WITH_VHOSTS;
+ }
+ else if (strcmp(arg1, "default") == 0) {
+ cmd->server->error_format_style = ERRORLOG_DEFAULT_STYLE;
+ }
+ else {
+ return "invalid ErrorLog syntax";
+ }
+ }
+
+ return NULL;
+}
+
static const char *set_send_buffer_size(cmd_parms *cmd, void *dummy, char *arg)
{
int s = atoi(arg);
@@ -3450,9 +3479,8 @@
"En-/disable server signature (on|off|email)" },
{ "ServerRoot", set_server_root, NULL, RSRC_CONF, TAKE1,
"Common directory of server-related files (logs, confs, etc.)" },
-{ "ErrorLog", set_server_string_slot,
- (void *)XtOffsetOf (server_rec, error_fname), RSRC_CONF, TAKE1,
- "The filename of the error log" },
+{ "ErrorLog", set_error_log, NULL, RSRC_CONF, TAKE12,
+ "The log style tag (default|vhosts) and filename of the error log" },
{ "PidFile", set_pidfile, NULL, RSRC_CONF, TAKE1,
"A file for logging the server process ID"},
{ "ScoreBoardFile", set_scoreboard, NULL, RSRC_CONF, TAKE1,
Only in apache_1.3.28.new/src/main: http_core.c.orig
diff -ru apache_1.3.28/src/main/http_log.c apache_1.3.28.new/src/main/http_log.c
--- apache_1.3.28/src/main/http_log.c 2003-02-03 12:13:21.000000000 -0500
+++ apache_1.3.28.new/src/main/http_log.c 2003-09-04 06:26:43.000000000 -0400
@@ -388,8 +388,15 @@
* quad is the most secure, which is why I'm implementing it
* first. -djg
*/
- len += ap_snprintf(errstr + len, sizeof(errstr) - len,
- "[client %s] ", r->connection->remote_ip);
+ if (r->server->error_format_style == ERRORLOG_WITH_VHOSTS) {
+ len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+ "[client %s] [%s:%d] ", r->connection->remote_ip,
+ r->server->server_hostname, r->server->port);
+ }
+ else { /* r->error_format_style == ERRORLOG_DEFAULT_STYLE */
+ len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+ "[client %s] ", r->connection->remote_ip);
+ }
}
if (!(level & APLOG_NOERRNO)
&& (save_errno != 0)
Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28
Posted by Glenn <gs...@gluelogic.com>.
On Mon, Oct 13, 2003 at 02:52:33PM -0700, Roy T. Fielding wrote:
> >On Tue, Jul 08, 2003 at 12:41:09AM -0400, Glenn wrote:
> >>Comments appreciated on the method(s) that would most likely get this
> >>accepted into 1.3.28 or 1.3.29. (global flag, server_rec addition,
> >>other ...) Thanks!
>
> In general, it would require a great deal of value added to justify
> a new feature in 1.3. I don't see that here. It certainly doesn't
> justify a change to server_rec (even an append is a risky change).
> A global flag is possible, but a compile-time ifdef would be sufficient
> for 1.3.
Thank you very much for the explanation.
My original patch used a global and can be found at:
http://www.mail-archive.com/dev@httpd.apache.org/msg16898.html
Here's one that uses a #define:
diff -ru apache_1.3.28/src/main/http_log.c apache_1.3.28.new/src/main/http_log.c
--- apache_1.3.28/src/main/http_log.c 2003-02-03 12:13:21.000000000 -0500
+++ apache_1.3.28.new/src/main/http_log.c 2003-10-13 19:27:47.000000000 -0400
@@ -388,8 +388,14 @@
* quad is the most secure, which is why I'm implementing it
* first. -djg
*/
+#ifdef ERRORLOGS_WITH_VHOST
+ len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+ "[client %s] [%s:%d] ", r->connection->remote_ip,
+ r->server->server_hostname, r->server->port);
+#else
len += ap_snprintf(errstr + len, sizeof(errstr) - len,
"[client %s] ", r->connection->remote_ip);
+#endif
}
if (!(level & APLOG_NOERRNO)
&& (save_errno != 0)
I prefer having the ability in a directive because I am not always
permitted to recompile Apache with the #define on systems that are
kept "distribution conformist".
Thank you for your consideration.
Glenn
Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28
Posted by "Roy T. Fielding" <fi...@apache.org>.
> On Tue, Jul 08, 2003 at 12:41:09AM -0400, Glenn wrote:
>> With the talk about a minor MMN bump, I put together this patch which
>> adds a flag at the end of server_rec. This also changes ErrorLog to
>> a TAKE12, with an optional style of "default" or "vhosts", where the
>> "vhosts" includes the server name and port in the error log entries.
>> The TAKE12 maintains backwards compatibility to existing config files.
>>
>> Comments appreciated on the method(s) that would most likely get this
>> accepted into 1.3.28 or 1.3.29. (global flag, server_rec addition,
>> other ...) Thanks!
In general, it would require a great deal of value added to justify
a new feature in 1.3. I don't see that here. It certainly doesn't
justify a change to server_rec (even an append is a risky change).
A global flag is possible, but a compile-time ifdef would be sufficient
for 1.3.
....Roy