Skip to content

Commit

Permalink
bug #10929 [2.3][Process] Add validation on Process input (romainneut…
Browse files Browse the repository at this point in the history
…ron)

This PR was merged into the 2.3 branch.

Discussion
----------

[2.3][Process] Add validation on Process input

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT

This adds validation on Process input. For the moment, passing a stream would result in a PHP error.
I propose to deprecate values that are not strictly string in 2.6 (see upcoming PR)

Commits
-------

583092b [Process] Add validation on Process input
  • Loading branch information
fabpot committed May 22, 2014
2 parents c505a63 + 583092b commit c8476ee
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 4 deletions.
5 changes: 3 additions & 2 deletions src/Symfony/Component/Process/Process.php
Expand Up @@ -880,15 +880,16 @@ public function getStdin()
*
* @return self The current Process instance
*
* @throws LogicException In case the process is running
* @throws LogicException In case the process is running
* @throws InvalidArgumentException In case the argument is invalid
*/
public function setStdin($stdin)
{
if ($this->isRunning()) {
throw new LogicException('STDIN can not be set while the process is running.');
}

$this->stdin = $stdin;
$this->stdin = ProcessUtils::validateInput(sprintf('%s::%s', __CLASS__, __FUNCTION__), $stdin);

return $this;
}
Expand Down
6 changes: 4 additions & 2 deletions src/Symfony/Component/Process/ProcessBuilder.php
Expand Up @@ -148,13 +148,15 @@ public function setEnv($name, $value)
/**
* Sets the input of the process.
*
* @param string $stdin The input as a string
* @param string|null $stdin The input as a string
*
* @return ProcessBuilder
*
* @throws InvalidArgumentException In case the argument is invalid
*/
public function setInput($stdin)
{
$this->stdin = $stdin;
$this->stdin = ProcessUtils::validateInput(sprintf('%s::%s', __CLASS__, __FUNCTION__), $stdin);

return $this;
}
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/Process/ProcessUtils.php
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Process;

use Symfony\Component\Process\Exception\InvalidArgumentException;

/**
* ProcessUtils is a bunch of utility methods.
*
Expand Down Expand Up @@ -72,6 +74,29 @@ public static function escapeArgument($argument)
return escapeshellarg($argument);
}

/**
* Validates and normalized a Process input
*
* @param string $caller The name of method call that validates the input
* @param mixed $input The input to validate
*
* @return string The validated input
*
* @throws InvalidArgumentException In case the input is not valid
*/
public static function validateInput($caller, $input)
{
if (null !== $input) {
if (is_scalar($input) || (is_object($input) && method_exists($input, '__toString'))) {
return (string) $input;
}

throw new InvalidArgumentException(sprintf('%s only accepts strings.', $caller));
}

return $input;
}

private static function isSurroundedBy($arg, $char)
{
return 2 < strlen($arg) && $char === $arg[0] && $char === $arg[strlen($arg) - 1];
Expand Down
53 changes: 53 additions & 0 deletions src/Symfony/Component/Process/Tests/AbstractProcessTest.php
Expand Up @@ -171,6 +171,47 @@ public function testSetStdinWhileRunningThrowsAnException()
$process->stop();
}

/**
* @dataProvider provideInvalidStdinValues
* @expectedException \Symfony\Component\Process\Exception\InvalidArgumentException
* @expectedExceptionMessage Symfony\Component\Process\Process::setStdin only accepts strings.
*/
public function testInvalidStdin($value)
{
$process = $this->getProcess('php -v');
$process->setStdin($value);
}

public function provideInvalidStdinValues()
{
return array(
array(array()),
array(new NonStringifiable()),
array(fopen('php://temporary', 'w')),
);
}

/**
* @dataProvider provideStdinValues
*/
public function testValidStdin($expected, $value)
{
$process = $this->getProcess('php -v');
$process->setStdin($value);
$this->assertSame($expected, $process->getStdin());
}

public function provideStdinValues()
{
return array(
array(null, null),
array('24.5', 24.5),
array('input data', 'input data'),
// to maintain BC, supposed to be removed in 3.0
array('stringifiable', new Stringifiable()),
);
}

public function chainedCommandsOutputProvider()
{
if (defined('PHP_WINDOWS_VERSION_BUILD')) {
Expand Down Expand Up @@ -813,3 +854,15 @@ public function methodProvider()
*/
abstract protected function getProcess($commandline, $cwd = null, array $env = null, $stdin = null, $timeout = 60, array $options = array());
}

class Stringifiable
{
public function __toString()
{
return 'stringifiable';
}
}

class NonStringifiable
{
}
10 changes: 10 additions & 0 deletions src/Symfony/Component/Process/Tests/ProcessBuilderTest.php
Expand Up @@ -197,4 +197,14 @@ public function testShouldNotThrowALogicExceptionIfNoPrefix()
$this->assertEquals("'/usr/bin/php'", $process->getCommandLine());
}
}

/**
* @expectedException \Symfony\Component\Process\Exception\InvalidArgumentException
* @expectedExceptionMessage Symfony\Component\Process\ProcessBuilder::setInput only accepts strings.
*/
public function testInvalidInput()
{
$builder = ProcessBuilder::create();
$builder->setInput(array());
}
}

0 comments on commit c8476ee

Please sign in to comment.