[Rt-commit] rt branch, 3.8/protect-fd0, created. rt-3.8.13rc1-1-g38093f9

[Alex Vandiver]

The branch, 3.8/protect-fd0 has been created
        at  38093f9fd10cbbd58f09c725b88d1c35bfa9cd51 (commit)

- Log -----------------------------------------------------------------
commit 38093f9fd10cbbd58f09c725b88d1c35bfa9cd51
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Fri May 25 18:15:58 2012 -0400

    Protect both FD 0 and FD 1 on mod_perl to avoid sendmailpipe errors
    
    Under the 'perl-script' handler on mod_perl, STDIN and STDOUT are
    effectively moved out of the way before the body of the request is run,
    such that they always appear to be closed in the handler.
    Unfortunately, the way this was done on mod_perl <= 2.0.4 was to
    actually close them, while storing away the handle in a higher FD for
    later restoration.  This means that, after the completion of the first
    request, FD 0 and FD 1 were left available for later system calls to
    obtain, as STDIN and STDOUT crept into higher-numbered FDs.
    
    While we explicitly opened FD 1 during the first request to protect
    against this sort of issue, no such protection was given to FD 0.  This
    allowed it to be claimed, in the second request, by the socket to the
    browser.  This causes failures in 'sendmailpipe' communication, as we
    are unable to dup STDIN onto FD 0 prior to calling IPC::Open2 (see
    b7a5a53).  The symptoms of this in Apache's error log varied, but
    included:
    
      Undefined value assigned to typeglob at RT/Interface/Email.pm line 459
    
      Apache2::RequestIO::rflush: (103) Software caused connection abort
    
      (9)Bad file descriptor: core_output_filter: writing data to the network
    
    Though these errors were only observed under the prefork MPM, there is
    no known reason that they could not occur under the worker MPM as well.
    
    Resolve this by ensuring that we always claim both FD 0 and FD 1 during
    the first request.  In doing so, we replace the possibly-brittle call to
    open(), where we can only hope we get the FD we want, with
    IO::Handle->new_from_fd(), which allows us to specify the FD we want.
    
    mod_perl 2.0.5 resolved this issue by never actually closing STDIN and
    STOUT, but rather local'ing them to be undef; this ensures that no later
    system calls will obtain them accidentally.  Forcing them open using
    new_from_fd, as this commit does, does not hurt matters, however.

diff --git a/bin/webmux.pl.in b/bin/webmux.pl.in
index 7aae041..296f649 100755
--- a/bin/webmux.pl.in
+++ b/bin/webmux.pl.in
@@ -60,24 +60,25 @@ package RT::Mason;
 
 our ($Nobody, $SystemUser, $Handler, $r);
 
-my $protect_fd;
+my $protect_fds;
 
 sub handler {
     ($r) = @_;
 
-    if ( !$protect_fd && $ENV{'MOD_PERL'} && exists $ENV{'MOD_PERL_API_VERSION'}
-        && $ENV{'MOD_PERL_API_VERSION'} >= 2 && fileno(STDOUT) != 1
+    if ( !$protect_fds && $ENV{'MOD_PERL'} && exists $ENV{'MOD_PERL_API_VERSION'}
+        && $ENV{'MOD_PERL_API_VERSION'} >= 2
     ) {
-        # under mod_perl2, STDOUT gets closed and re-opened, however new STDOUT
-        # is not on FD #1. In this case next IO operation will occupy this FD
-        # and make all system() and open "|-" dangerouse, for example DBI
-        # can get this FD for DB connection and system() call will close
-        # by putting grabage into the socket
-        open( $protect_fd, '>', '/dev/null' )
-          or die "Couldn't open /dev/null: $!";
-        unless ( fileno($protect_fd) == 1 ) {
-            warn "We opened /dev/null to protect FD #1, but descriptor #1 is already occupied";
-        }
+        # under mod_perl2, STDIN and STDOUT get closed and re-opened,
+        # however they are not on FD 0 and 1.  In this case, the next
+        # socket that gets opened will occupy one of these FDs, and make
+        # all system() and open "|-" calls dangerous; for example, the
+        # DBI handle can get this FD, which later system() calls will
+        # close by putting garbage into the socket.
+        $protect_fds = [];
+        push @{$protect_fds}, IO::Handle->new_from_fd(0, "r")
+            if fileno(STDIN) != 0;
+        push @{$protect_fds}, IO::Handle->new_from_fd(1, "w")
+            if fileno(STDOUT) != 1;
     }
 
     local $SIG{__WARN__};

-----------------------------------------------------------------------