Skip Menu |

This queue is for tickets about the Log-Log4perl CPAN distribution.

Report information
The Basics
Id: 83193
Status: resolved
Priority: 0/
Queue: Log-Log4perl
People
Owner: Nobody in particular
Requestors: g_ml2000-x [...] yahoo.de
Cc:
AdminCc:
Bug Information
Severity: Normal
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Thu Feb 07 08:02:51 2013 g_ml2000-x [...] yahoo.de - Ticket created
Subject: Appender::File should not insist on regular files
Currently, Log::Log4perl::Appender::File calls die if the specified log file is not a regular file which IMHO considerably reduces flexibility without any major advantage. The attached 1-char-patch fixes this. Regards, Peter
Subject: appender_file_allow_any.diff
--- Log/Log4perl/Appender/File.org 2013-02-07 13:53:48.920947195 +0100 +++ Log/Log4perl/Appender/File.pm 2013-02-07 13:53:56.485038428 +0100 @@ -94,7 +94,7 @@ sub file_open { umask($self->{umask}) if defined $self->{umask}; - my $didnt_exist = ! -f $self->{filename}; + my $didnt_exist = ! -e $self->{filename}; if($self->{syswrite}) { sysopen $fh, "$self->{filename}", $sysmode or
  Fri Feb 08 01:37:43 2013 MSCHILLI [...] cpan.org - Correspondence added
On Thu Feb 07 08:02:51 2013, g_ml200 wrote: Show quoted text
> Currently, Log::Log4perl::Appender::File calls die if the specified > log file is not a regular file which IMHO considerably reduces > flexibility without any major advantage. > The attached 1-char-patch fixes this.
Hi Peter, are you sure this is a problem? It works fine for me with non-files like symlinks in Log::Log4perl 1.40. Do you have a test case to help me reproduce the problem? Thanks -- Mike.
  Fri Feb 08 01:37:44 2013 The RT System itself - Status changed from 'new' to 'open'
  Fri Feb 08 10:34:36 2013 g_ml2000-x [...] yahoo.de - Correspondence added
From: g_ml2000-x [...] yahoo.de
On Fr 08. Feb 2013, 01:37:43, MSCHILLI wrote: Show quoted text
> are you sure this is a problem? It works fine for me with > non-files like symlinks in Log::Log4perl 1.40.
Symlinks are no problem, as long as the _point to_ a regular file. Any other file type will fail because of the "-f" test, even if it otherwise would work without problem. The case that I originally tripped over is that I use a default configuration file across a bunch of related programs because their logging behavior should be identical where possible. In some scenarios these would be called from a UID that would not allow write access. Because I didn't find any other obvious way to share the configuration file but deactivate the file appender in this case, I just used a perl hook to modify the log file path, setting it to "/dev/null" to deactivate the log file. For my particular problem there are certainly other possible solutions, but generally, I don't see a reason why it should be prohibited to use character device "files" (or whatever else can be used as a file) with Appender::File ... Regards, Peter
  Fri Feb 08 11:09:22 2013 MSCHILLI [...] cpan.org - Correspondence added
On Fri Feb 08 10:34:36 2013, g_ml200 wrote: Show quoted text
> Any other file type will fail because of the "-f" test, even if it > otherwise would work without problem.
I do agree that writing to non-files is useful (Unix domain sockets come to mind), but I don't see why you think the current code won't allow for these cases. For example, your /dev/null case: use Log::Log4perl qw(get_logger); my $conf = q( log4perl.category.Bar.Twix = WARN, Logfile log4perl.appender.Logfile = Log::Log4perl::Appender::File log4perl.appender.Logfile.filename = /dev/null log4perl.appender.Logfile.owner = mschilli log4perl.appender.Logfile.layout = Log::Log4perl::Layout::SimpleLayout ); Log::Log4perl::init(\$conf); my $logger = get_logger("Bar::Twix"); $logger->error("Waaah!"); works just fine. The only case where I can see this causes a different route to be taken in the code is if you explicitly set an owner of the file in the Log4perl configuration. Can you send me an excerpt of your configuration that reproduces the problem with /dev/null? I'm all game for allowing that case, just would like to know the exact cause of the problem. Thanks -- Mike
  Sat Feb 09 03:22:28 2013 g_ml2000-x [...] yahoo.de - Correspondence added
From: g_ml2000-x [...] yahoo.de
Am Fr 08. Feb 2013, 11:09:22, MSCHILLI schrieb: Show quoted text
> The only case where I can see this causes a different route > to be taken in the code is if you explicitly set an owner of > the file in the Log4perl configuration.
... you're right - the code only fails, if the logfile is some special file _and_ an owner or group is specified. Because this also is the case for your example configuration, it will fail: $ ~> /tmp/test.pl chown('500', '0') on '/dev/null' failed: Operation not permitted at /usr/share/perl5/Log/Log4perl/Appender/File.pm line 202, <DATA> line 522. In Summary: - regular files can be set up in advance and will be just used as they are - for other (pre-existing) files Appender::Files calls "chown", which will usually fail if run from an unprivileged account (unless the file is already owned by the same UID the perl code runs from, which might not be desirable in many cases) Is this asymmetry really intended? Regards, Peter
  Sat Feb 09 15:23:31 2013 MSCHILLI [...] cpan.org - Correspondence added
Show quoted text
> Is this asymmetry really intended?
It certainly isn't as intuitive as I'd like it to be. It's an interesting problem. If someone puts an "owner" field into the L4p configuration, and a file gets created by L4p, the expectation is that the process is either running under the intended UID already (in which case the owner setting is superfluous) or is privileged and the file needs to be created with the given UID, not the privileged UID (the more common case). So far so good. If the log file already exists, no owner changes are attempted. This is somewhat counter-intuitive, but it's not easy to change that now without breaking systems in production. If we accept that for now, it does make sense to extend it to *other* existing file-like entries, like devices, sockets, named pipes, etc. I've applied your patch with some explanatory text in the owner/group option documentation sections: https://github.com/mschilli/log4perl/commit/245f98f1bae8da6c4fd72974270 45e84d9da086f Sorry this took so long, just wanted to make sure I understand the implications before I apply the patch, thanks for your contribution! -- Mike
  Sat Feb 09 15:23:41 2013 MSCHILLI [...] cpan.org - Status changed from 'open' to 'resolved'