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.
>
>
