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.
---
lib/unicorn/util.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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:
-- 1.7.12.1
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
Eric Wong wrote:> Interesting nobody found this bug earlier (I think pointing logs > to /dev/null is bad :)Generally agree. Sometimes it happens. :|> 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::EBADFThis looks much better as a general solution to the problem. +1 Thanks, Eric!
Joel Meador <joel at expectedbehavior.com> wrote:> Eric Wong wrote: > >--- 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 > This looks much better as a general solution to the problem. +1 > Thanks, Eric!Thanks for reporting! Pushed to "master" of git://bogomips.org/unicorn as commit 032791b9a367f67febbe7534f6ea4cac127e7897 Will probably tag + release a new version in day or two.