phing icon indicating copy to clipboard operation
phing copied to clipboard

Regression: empty argument in ExecTask cannot be escaped

Open lpd-au opened this issue 1 year ago • 3 comments

Describe the bug With Phing 2 it was possible to do this:

<target name="echo-old">
	<exec command="echo foo '' bar" passthru="true" />
</target>
<target name="php-old">
	<exec command="php -r 'var_dump($argv);' foo '' bar" passthru="true" />
</target>

This produced:

echo-old: foo  bar                                # Note: two spaces
php-old: array(4) {
  [0]=>
  string(19) "Standard input code"
  [1]=>
  string(3) "foo"
  [2]=>
  string(0) ""                                    # Note: length 0
  [3]=>
  string(3) "bar"
}

The documentation says it should still be possible with Phing 3:

To pass an empty argument, enclose two double quotes in single quotes ('""').

However, this produces a literal "" with any value of escape (including none); I cannot find a combination of attributes that produces the same behaviour as before.

Steps To Reproduce

<target name="echo">
	<exec executable="echo" passthru="true">
		<arg value="foo" />
		<arg value='""' />
		<arg value="bar" />
	</exec>
</target>
<target name="php">
	<exec executable="php" passthru="true">
		<arg value="-r" />
		<arg value="var_dump($argv);" escape="true" />
		<arg value="foo" />
		<arg value='""' />
		<arg value="bar" />
	</exec>
</target>

Expected behavior

echo: foo  bar
php: array(4) {
  [0]=>
  string(19) "Standard input code"
  [1]=>
  string(3) "foo"
  [2]=>
  string(0) ""
  [3]=>
  string(3) "bar"
}

Screenshots / terminal output

echo: foo "" bar
php: sh: 1: Syntax error: "(" unexpected
     [exec] Result: 2

Additional context Replacing the inline php with a file still outputs a non-zero length string:

php-file: array(4) {
  [0]=>
  string(8) "test.php"
  [1]=>
  string(3) "foo"
  [2]=>
  string(2) """"
  [3]=>
  string(3) "bar"
}

lpd-au avatar Jun 07 '24 12:06 lpd-au

I've had a look into this. Initially I tried changing exec and passthru to use proc_open, where we can pass in the arguments individually rather than have a shell interpret a string, however that change is quite large and affects multiple other classes. It makes things a lot more robust and the "output" and "error" features are much cleaner this way, but I'm afraid that the change is too large for a patch version number. For reference, I have uploaded my current snapshot to https://github.com/phingofficial/phing/compare/main...fredden:phing:feature/gh-issue-1839 - but this is far from ready as several tests are still failing.

@mrook I can see that you've tagged this as a good first issue. Please can you provide some guidance on what you had in mind to fix this?

fredden avatar Sep 07 '24 17:09 fredden

I have also been unsuccessfully trying to find a solution for this. We used to have several constructs such as the following:

<exec command="... -e '${exclude}'" />

It can be that the exclude property evaluates to an empty string, in which case -e '' would be the expected command line while simply omitting the empty string results in an error.

I have tried several approaches:

  • <arg value="-e"/><arg value="${exclude}"/> outputs -e
  • <arg line="-e '${exclude}'"/> outputs -e
  • <arg line='-e "${exclude}"'/> outputs -e
  • <arg value="-e"/><arg value="'${exclude}'"/> outputs -e ''\'''\'''
  • <arg value="-e"/><arg value='"${exclude}"'/> outputs -e "" but would output -e '"property"' if not empty

ralfstrobel avatar Oct 10 '24 10:10 ralfstrobel

Hi, there are a few things going on here.

First, the escape attribute on arg should not have been in the docs (as it is not used) - I'll mark that as deprecated.

@lpd-au your php target can be rewritten as:

        <exec executable="php" passthru="true" escape="true">
                <arg value="-r" />
                <arg value="var_dump($argv);" />
                <arg value="foo" />
                <arg value="" />
                <arg value="bar" />
        </exec>

That seems to be producing the output you're looking for.

@fredden proc_open could be a nice change in the future, but as you rightly point out, that's easier said than done. Labeling this issue as "good first issue" was not the right move (which is why I removed that label).

@ralfstrobel your issue can be written as:

        <exec executable="php" passthru="true" escape="true">
                <arg value="-e"/>
                <arg value="${exclude}"/>
                
        </exec>
        <exec executable="php" passthru="true" escape="true">
                <arg line="-e '${exclude}'"/>
        </exec>

This seems to be producing the output you're looking for.

mrook avatar Jun 09 '25 10:06 mrook