[PATCH] chowning /dev/null should be guarded against

Eric Wong normalperson at yhbt.net
Tue Oct 2 01:27:31 UTC 2012


Joel Meador <joel at expectedbehavior.com> wrote:
> We've run into this problem several times and it's not really
> expected that someone is going to monkey with /dev/null. So here's a
> simple patch.
> I am not on the mailing list, so please Cc: me.

Interesting nobody found this bug earlier (I think pointing logs
to /dev/null is bad :)

> diff --git a/lib/unicorn/util.rb b/lib/unicorn/util.rb
> index cde2563..6b6cca2 100644
> --- a/lib/unicorn/util.rb
> +++ b/lib/unicorn/util.rb
> @@ -15,7  +15,7  @@ module Unicorn::Util
> 
>    def self.chown_logs(uid, gid)
>      ObjectSpace.each_object(File) do |fp|
> -      fp.chown(uid, gid) if is_log?(fp)
> +      fp.chown(uid, gid) if is_log?(fp)&&  fp.path != "/dev/null"
>      end
>    end
>  # :startdoc:

How about this, instead?  This will work in case some weirdo setups
use a non-standard path for /dev/null, and also allows logging to
FIFOs (e.g. cronolog users).

diff --git a/lib/unicorn/util.rb b/lib/unicorn/util.rb
index cde2563..f84241c 100644
--- a/lib/unicorn/util.rb
+++ b/lib/unicorn/util.rb
@@ -7,6 +7,7 @@ def self.is_log?(fp)
     append_flags = File::WRONLY | File::APPEND
 
     ! fp.closed? &&
+      fp.stat.file? &&
       fp.sync &&
       (fp.fcntl(Fcntl::F_GETFL) & append_flags) == append_flags
     rescue IOError, Errno::EBADF


More information about the mongrel-unicorn mailing list