Skip to content

Commit

Permalink
[WIP] Fixed whitespace issue on argument escaping. (#735)
Browse files Browse the repository at this point in the history 
* Fixed whitespace issue on argument escaping.

* Fixed test execution under linux.

* Fixed test execution under linux 2.

* ExecTaskTests working again

* Reduced complexity by extending from ExecTask

* Removed trailing whitespace

* Added additional stuff

* Fixed executCommand

* Fixed test

* Some minor fixes

* Fixed last on linux

* ApplyTask additions
  • Loading branch information
@siad007
siad007 authored Jul 2, 2018
1 parent 75d5e1f commit 3d1c41e
Show file tree
Hide file tree
Showing 10 changed files with 648 additions and 625 deletions.
752 changes: 349 additions & 403 deletions classes/phing/tasks/system/ApplyTask.php
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion classes/phing/tasks/system/AttribTask.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ protected function checkConfiguration()
* @param mixed $e
* @throws BuildException
*/
public function setExecutable($e)
public function setExecutable($e): void
{
throw new BuildException(
$this->getTaskType() . ' doesn\'t support the executable attribute',
Expand Down
139 changes: 71 additions & 68 deletions classes/phing/tasks/system/ExecTask.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@ class ExecTask extends Task
*/
protected $realCommand;

/**
* Given command
* @var string
*/
protected $command;

/**
* Commandline managing object
*
Expand Down Expand Up @@ -180,11 +174,22 @@ protected function prepare()
$this->dir = $this->getProject()->getBasedir();
}

if ($this->commandline->getExecutable() === null) {
throw new BuildException(
'ExecTask: Please provide "executable"'
);
}

// expand any symbolic links first
try {
if (!$this->dir->getCanonicalFile()->exists()) {
throw new BuildException(
"The directory '" . (string) $this->dir . "' does not exist"
);
}
if (!$this->dir->getCanonicalFile()->isDirectory()) {
throw new BuildException(
"'" . (string) $this->dir . "' is not a valid directory"
"'" . (string) $this->dir . "' is not a directory"
);
}
} catch (IOException $e) {
Expand All @@ -194,6 +199,8 @@ protected function prepare()
}
$this->currdir = getcwd();
@chdir($this->dir->getPath());

$this->commandline->setEscape($this->escape);
}

/**
Expand All @@ -205,47 +212,23 @@ protected function prepare()
*/
protected function buildCommand()
{
if ($this->command === null && $this->commandline->getExecutable() === null) {
throw new BuildException(
'ExecTask: Please provide "command" OR "executable"'
);
} else {
if ($this->command === null) {
$this->realCommand = Commandline::toString($this->commandline->getCommandline(), $this->escape);
} else {
if ($this->commandline->getExecutable() === null) {
$this->realCommand = $this->command;

//we need to escape the command only if it's specified directly
// commandline takes care of "executable" already
if ($this->escape == true) {
$this->realCommand = escapeshellcmd($this->realCommand);
}
} else {
throw new BuildException(
'ExecTask: Either use "command" OR "executable"'
);
}
}
}

if ($this->error !== null) {
$this->realCommand .= ' 2> ' . escapeshellarg($this->error->getPath());
$this->log(
"Writing error output to: " . $this->error->getPath(),
'Writing error output to: ' . $this->error->getPath(),
$this->logLevel
);
}

if ($this->output !== null) {
$this->realCommand .= ' 1> ' . escapeshellarg($this->output->getPath());
$this->log(
"Writing standard output to: " . $this->output->getPath(),
'Writing standard output to: ' . $this->output->getPath(),
$this->logLevel
);
} elseif ($this->spawn) {
$this->realCommand .= ' 1>/dev/null';
$this->log("Sending output to /dev/null", $this->logLevel);
$this->log('Sending output to /dev/null', $this->logLevel);
}

// If neither output nor error are being written to file
Expand All @@ -260,24 +243,29 @@ protected function buildCommand()
if ($this->spawn) {
$this->realCommand .= ' &';
}

$this->realCommand = (string) $this->commandline . $this->realCommand;
}

/**
* Executes the command and returns return code and output.
*
* @return array array(return code, array with output)
* @throws \BuildException
*/
protected function executeCommand()
{
$this->log("Executing command: " . $this->realCommand, $this->logLevel);
$cmdl = $this->realCommand;

$this->log('Executing command: ' . $cmdl, $this->logLevel);

$output = [];
$return = null;

if ($this->passthru) {
passthru($this->realCommand, $return);
passthru($cmdl, $return);
} else {
exec($this->realCommand, $output, $return);
exec($cmdl, $output, $return);
}

return [$return, $output];
Expand All @@ -295,7 +283,7 @@ protected function executeCommand()
* @throws BuildException
* @return void
*/
protected function cleanup($return, $output)
protected function cleanup($return, $output): void
{
if ($this->dir !== null) {
@chdir($this->currdir);
Expand All @@ -306,9 +294,7 @@ protected function cleanup($return, $output)
$this->log($line, $outloglevel);
}

if ($this->returnProperty) {
$this->project->setProperty($this->returnProperty, $return);
}
$this->maybeSetReturnPropertyValue($return);

if ($this->outputProperty) {
$this->project->setProperty(
Expand All @@ -319,8 +305,11 @@ protected function cleanup($return, $output)

$this->setExitValue($return);

if ($return != 0 && $this->checkreturn) {
throw new BuildException("Task exited with code $return");
if ($return !== 0) {
if ($this->checkreturn) {
throw new BuildException($this->getTaskType() . ' returned: ' . $return, $this->getLocation());
}
$this->log('Result: ' . $return, Project::MSG_ERR);
}
}

Expand All @@ -329,7 +318,7 @@ protected function cleanup($return, $output)
*
* @param int $value exit value of the process.
*/
protected function setExitValue($value)
protected function setExitValue($value): void
{
$this->exitValue = $value;
}
Expand All @@ -340,34 +329,41 @@ protected function setExitValue($value)
* @return int the exit value or self::INVALID if no exit value has
* been received.
*/
public function getExitValue()
public function getExitValue(): int
{
return $this->exitValue;
}

/**
* The command to use.
*
* @param mixed $command String or string-compatible (e.g. w/ __toString()).
* @param string $command String or string-compatible (e.g. w/ __toString()).
*
* @return void
* @throws \BuildException
*/
public function setCommand($command)
public function setCommand($command): void
{
$this->command = "" . $command;
$this->log("The command attribute is deprecated.\nPlease use the executable attribute and nested arg elements.",
Project::MSG_WARN);
$this->commandline = new Commandline($command);
$this->executable = $this->commandline->getExecutable();
}

/**
* The executable to use.
*
* @param mixed $executable String or string-compatible (e.g. w/ __toString()).
* @param string|bool $value String or string-compatible (e.g. w/ __toString()).
*
* @return void
*/
public function setExecutable($executable)
public function setExecutable($value): void
{
$this->executable = $executable;
$this->commandline->setExecutable((string) $executable);
if (is_bool($value)) {
$value = $value === true ? 'true' : 'false';
}
$this->executable = $value;
$this->commandline->setExecutable($value);
}

/**
Expand All @@ -377,7 +373,7 @@ public function setExecutable($executable)
*
* @return void
*/
public function setEscape($escape)
public function setEscape($escape): void
{
$this->escape = (bool) $escape;
}
Expand All @@ -389,7 +385,7 @@ public function setEscape($escape)
*
* @return void
*/
public function setDir(PhingFile $dir)
public function setDir(PhingFile $dir): void
{
$this->dir = $dir;
}
Expand All @@ -401,15 +397,15 @@ public function setDir(PhingFile $dir)
*
* @return void
*/
public function setOs($os)
public function setOs($os): void
{
$this->os = (string) $os;
}

/**
* List of operating systems on which the command may be executed.
*/
public function getOs()
public function getOs(): string
{
return $this->os;
}
Expand All @@ -418,7 +414,7 @@ public function getOs()
* Restrict this execution to a single OS Family
* @param string $osFamily the family to restrict to.
*/
public function setOsFamily($osFamily)
public function setOsFamily($osFamily): void
{
$this->osFamily = strtolower($osFamily);
}
Expand All @@ -438,7 +434,7 @@ public function getOsFamily()
*
* @return void
*/
public function setOutput(PhingFile $f)
public function setOutput(PhingFile $f): void
{
$this->output = $f;
}
Expand All @@ -450,7 +446,7 @@ public function setOutput(PhingFile $f)
*
* @return void
*/
public function setError(PhingFile $f)
public function setError(PhingFile $f): void
{
$this->error = $f;
}
Expand All @@ -462,7 +458,7 @@ public function setError(PhingFile $f)
*
* @return void
*/
public function setPassthru($passthru)
public function setPassthru($passthru): void
{
$this->passthru = $passthru;
}
Expand All @@ -474,7 +470,7 @@ public function setPassthru($passthru)
*
* @return void
*/
public function setLogoutput($logOutput)
public function setLogoutput($logOutput): void
{
$this->logOutput = $logOutput;
}
Expand All @@ -486,7 +482,7 @@ public function setLogoutput($logOutput)
*
* @return void
*/
public function setSpawn($spawn)
public function setSpawn($spawn): void
{
$this->spawn = $spawn;
}
Expand All @@ -498,7 +494,7 @@ public function setSpawn($spawn)
*
* @return void
*/
public function setCheckreturn($checkreturn)
public function setCheckreturn($checkreturn): void
{
$this->checkreturn = $checkreturn;
}
Expand All @@ -510,19 +506,26 @@ public function setCheckreturn($checkreturn)
*
* @return void
*/
public function setReturnProperty($prop)
public function setReturnProperty($prop): void
{
$this->returnProperty = $prop;
}

protected function maybeSetReturnPropertyValue(int $return)
{
if ($this->returnProperty) {
$this->getProject()->setNewProperty($this->returnProperty, $return);
}
}

/**
* The name of property to set to output value from exec() call.
*
* @param string $prop Property name
*
* @return void
*/
public function setOutputProperty($prop)
public function setOutputProperty($prop): void
{
$this->outputProperty = $prop;
}
Expand All @@ -535,7 +538,7 @@ public function setOutputProperty($prop)
* @throws BuildException
* @return void
*/
public function setLevel($level)
public function setLevel($level): void
{
switch ($level) {
case 'error':
Expand Down Expand Up @@ -597,9 +600,9 @@ public function createArg()
* <li><code>false</code> otherwise.</li>
* </ul>
*/
protected function isValidOs()
protected function isValidOs(): bool
{
//hand osfamily off to Os class, if set
//hand osfamily off to OsCondition class, if set
if ($this->osFamily !== null && !OsCondition::isFamily($this->osFamily)) {
return false;
}
Expand Down
Loading

0 comments on commit 3d1c41e

Please sign in to comment.