Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:44849
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 212.55.154.21 as permitted sender)
Message-ID: <33C67F4EB9E9469F8822AEA16C5A9818@PC3EE1F19287>
To: "Gwynne Raskind" <gwynne@darkrainfall.org>
Cc: "PHP Internals" <internals@lists.php.net>
References: <E2310D92-1B79-428A-ABB0-918E9488C21E@darkrainfall.org>
Date: Wed, 8 Jul 2009 21:16:13 +0100
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=response
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] A patch for HEAD
From: nlopess@php.net ("Nuno Lopes")

The idea is great. In fact this was in my todo list for php 5.3..
Please give me a few more days to review the patch.

Nuno

P.S.: you can add on more point to your list: you get to know the PID of the 
exec'ed process instead of the PID of the shell.

----- Original Message -----
> I've just finished making this patch for my own use (diffed against  5.3 
> CVS):
>
> http://darkrainfall.org/php-5.3-shellbypass.patch
>
> In short, what it does is make proc_open()'s shell_bypass option 
> available to UNIX systems. This is accomplished by allowing the  "command" 
> parameter to proc_open() to be an array of arguments to pass  to 
> execv[e](). I've included a few tests to check the functionality.
>
> (A few more tests could be devised to, for example, check that the 
> correct warning is issued if you pass an array without bypass_shell  set, 
> or a string with it set, etc.)
>
> The exact behavior of the argument array is:
> 1) The array must contain at least one element, at index 0.
> 2) The element at index 0 is always the exact command path passed to 
> execv[e]() (after being filtered through any safe_mode restrictions,  as 
> with the normal behavior of proc_open()).
> 3) Any other elements form the argv array passed to execv[e](). By 
> convention the first of these arguments (argv[0] in the child process)  is 
> the same as the command path, however my patch does NOT enforce or  assume 
> this; it simply calls execv[e]($argument_array[0], 
> array_slice($argument_array, 1)).
>
> This patch currently provides the only useful way to fork a process 
> without running a shell (pcntl_fork() + pcntl_exec() are useless since 
> there's no pcntl_dup2() to control the descriptors of the child).
>
> Why would you want to avoid the shell?
>
> - Efficiency. The shell is an extra, often unnecessary process, which 
> must parse the commandline given to it into individual arguments 
> according to all its various rules. Not to mention the overhead of 
> setting up another entire process just to run a third process.
>
> - Resource control. The shell is an extra process. If you don't need  it, 
> and your system is tight on process space, best to avoid it.
>
> - Sanity. Correctly quoting arguments to a shell command ranges from 
> mildly annoying (escapeshellarg() in simple cases) to nightmarish  (manual 
> parsing of a string in some edge cases). Passing arguments  directly 
> completely bypasses this, quite possibly saving you quite a  bit of string 
> parsing time if you were doing something like  "$shell_args = implode(' ', 
> array_map('escapeshellarg', $raw_args));".
>
> - Oddly enough, security. Since there's no shell, it's more difficult  to 
> subvert the child process to do other things than the coder  intended 
> (unless of course, said coder executes a shell this way).
>
> This patch does nothing on Windows, since the option was already 
> implemented there. It also does nothing on Netware, since from what I 
> could see in the code, Netware doesn't have a shell in the first place.
>
> I'm proposing the inclusion of this patch in HEAD (which I'll port it  to 
> if I get a thumbs-up here), and possibly 5.3.2. Criticism and angry 
> flames welcome. Constructive critcism and good-natured comments will  be 
> ignored ;) (just kidding... or am I?).
>
> -- Gwynne