Luca -
Here's the same thing standardizing on strn?cmp(). Not that you couldn't have
done it yourself, but since I had it up, maybe this will save you 30 seconds.
;-)
Index: server/core.c
===================================================================
--- server/core.c (revision 1829439)
+++ server/core.c (working copy)
@@ -4867,7 +4867,8 @@
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) {
+ || strcmp(s->error_fname, "syslog") == 0
+ || strncmp(s->error_fname, "syslog:", 7) == 0) {
return APR_SUCCESS;
}
else {
@@ -5281,7 +5282,9 @@
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] != '|'
+ && strcmp(s->error_fname, "syslog") != 0
+ && strncmp(s->error_fname, "syslog:", 7) != 0)
tmp = ap_server_root_relative(p, s->error_fname);
else
tmp = s->error_fname;
@@ -5456,4 +5459,3 @@
core_cmds, /* command apr_table_t */
register_hooks /* register hooks */
};
-
Index: server/log.c
===================================================================
--- server/log.c (revision 1829439)
+++ server/log.c (working copy)
@@ -396,7 +396,8 @@
}
#ifdef HAVE_SYSLOG
- else if (!strncasecmp(s->error_fname, "syslog", 6)) {
+ else if (strcmp(s->error_fname, "syslog") == 0
+ || strncmp(s->error_fname, "syslog:", 7) == 0) {
if ((fname = strchr(s->error_fname, ':'))) {
/* s->error_fname could be [level]:[tag] (see #60525) */
const char *tag;
> On 18 Apr 2018, at 13:32, Luca Toscano <[email protected]> wrote:
>
> Thanks a lot Jim! I like your code change and the extra checks, but I'd
> prefer to use strncmp if possible, also in log.c.
> Feel free to amend the patch, or I'll do it tomorrow (I forgot the entry in
> CHANGES too, your name should be on it :).
>
> Luca
>
> 2018-04-18 18:36 GMT+02:00 Jim Riggs <[email protected]>:
> Fair enough. I'm fine standardizing either way. strn?cmp() is probably more
> "correct". As it stands, though, the check in core is not actually checking
> everything that log.c will handle.
>
>
> > On 18 Apr 2018, at 10:15, William A Rowe Jr <[email protected]> wrote:
> >
> > On Wed, Apr 18, 2018 at 7:17 AM, Jim Riggs <[email protected]> wrote:
> >> I didn't think of this before, but there is one edge case this would miss:
> >> if someone (for whatever reason) wants a relative ErrorLog *file* named
> >> `syslog*', for example `ErrorLog "syslog-httpd.log"' or `ErrorLog
> >> "syslog.log"'. It appears that this is already broken in server/log.c,
> >> though. Also, log.c is using str(n)casecmp. The following would make
> >> things consistent and handle this weird edge case. Thoughts?
> >>
> >> Index: server/core.c
> >> ===================================================================
> >> --- server/core.c (revision 1829431)
> >> +++ server/core.c (working copy)
> >> @@ -4867,7 +4867,8 @@
> >> 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) {
> >> + || strcasecmp(s->error_fname, "syslog") == 0
> >> + || strncasecmp(s->error_fname, "syslog:", 7) == 0) {
> >> return APR_SUCCESS;
> >> }
> >> else {
> >> @@ -5281,7 +5282,9 @@
> >> 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] != '|'
> >> + && strcasecmp(s->error_fname, "syslog") != 0
> >> + && strncasecmp(s->error_fname, "syslog:", 7) != 0)
> >> tmp = ap_server_root_relative(p, s->error_fname);
> >> else
> >> tmp = s->error_fname;
> >> @@ -5456,4 +5459,3 @@
> >> core_cmds, /* command apr_table_t */
> >> register_hooks /* register hooks */
> >> };
> >> -
> >> Index: server/log.c
> >> ===================================================================
> >> --- server/log.c (revision 1829431)
> >> +++ server/log.c (working copy)
> >> @@ -396,7 +396,8 @@
> >> }
> >>
> >> #ifdef HAVE_SYSLOG
> >> - else if (!strncasecmp(s->error_fname, "syslog", 6)) {
> >> + else if (strcasecmp(s->error_fname, "syslog") == 0
> >> + || strncasecmp(s->error_fname, "syslog:", 7) == 0) {
> >> if ((fname = strchr(s->error_fname, ':'))) {
> >> /* s->error_fname could be [level]:[tag] (see #60525) */
> >> const char *tag;
> >
> > Shouldn't we normalize the use of strcmp instead of strcasecmp?
> > In any case it must be entirely normalized to one or the other.
> >
> > Linux is a case-sensitive OS in the first place, and if configured
> > with SYSLOG: today it is broken when you hit core, which ignores
> > that code path. Since the behavior has always been syslog: I'm
> > not seeing a benefit to case insensitivity.
>
>
