Helper function `WP_CLI\Entity\Utils::has_stdin()` misfires depending on shell environment

[xego]

Given a command like:

wp option patch update gglcptch_options login_form 0

run on a non interactive shell (like through atd/crod/puppet exec) we receive an Error:

Error: No data exists for key "0"

if we run the same program through the command line everything works fine.

Taking a look at the code here

https://github.com/wp-cli/entity-command/blob/v1.2.0/src/Option_Command.php#L525

and trying the command:

echo 1 | wp option patch update gglcptch_options login_form

gave us the expected results even in atd/crod/puppet exec shells.

[schlessera]

At first glance, this looks like a bug in the argument parsing code.

I could reproduce it by adding < /dev/null to it:

$ wp option patch update wp_user_roles administrator capabilities unfiltered_html false
Success: Updated 'wp_user_roles' option.

$ wp option patch update wp_user_roles administrator capabilities unfiltered_html false < /dev/null
Error: No data exists for key "false"

I suppose it detects the < /dev/null as a valid STDIN input. This makes it pick STDIN as value, so the actual value we had provided gets interpreted as a key as well.

[njam]

Just stumbled opon the same issue.

Option_Command::patch() has:

} elseif ( \WP_CLI\Entity\Utils::has_stdin() ||true) {
	$stdin_value = WP_CLI::get_value_from_arg_or_stdin( $args, -1 );
	$patch_value = WP_CLI::read_value( trim( $stdin_value ), $assoc_args );
} else {
	$patch_value = WP_CLI::read_value( array_pop( $key_path ), $assoc_args );
}

It looks to me like the first branch here is missing that array_pop() which the second one has.
Also I'm not sure if get_value_from_arg_or_stdin() will ever return the last element of $args, because I think there's no array element access using negative indices in PHP?

[odyniec]

I came across this issue when running wp option patch update inside a shell script executed through Vagrant. As a workaround, I replaced:

wp option patch update key subkey value

with:

echo "value" | wp option patch update key subkey

[schlessera]

Some more details on this bug after trying to resolve it without success yesterday...

The root cause lies in WP_CLI\Entity\Utils::has_stdin(). Under some circumstances (due to the way the surrounding shell has called the process), the stream on STDIN is present, but returns an empty value.

This in turn will cause the above logic to miscount the arguments.

I fixed the symptom in the above code in the following way:

$stdin_value = WP_CLI\Entity\Utils::has_stdin()
	? trim( WP_CLI::get_value_from_arg_or_stdin( $args, -1 ) )
	: null;
$patch_value = ! empty( $stdin_value )
	? WP_CLI::read_value( $stdin_value, $assoc_args )
	: WP_CLI::read_value( array_pop( $key_path ), $assoc_args );

So, for the option patch call, the problem is fixed with v2. However, I didn't yet solve the root cause in WP_CLI\Entity\Utils::has_stdin(), so places where we cannot just trick with the values are not yet fixed.

The one case I know of so far is wp user import-csv -.

[schlessera]

Related #194

[soderlind]

I've been testing https://stackoverflow.com/a/11327451/1434155, and it works on all the shells I've tested (on my Mac). Maybe rewrite has_stdin and use the code from stackoverflow ?

[schlessera]

@soderlind I already experimented with a similar approach, reading the mode directly, but the main issue is not detecting what type of pipe we have, but rather, whether the pipe has content or not. Did you find a way to do that through the above (mode-based) method, or did you only test for the type of pipe?

[soderlind]

Initially I only tested detecting what type of pipe I had, but (so far) this works for reading stdin:

private function read( $fp ) {
	   $fstats = fstat( $fp );
	   return fread( $fp, $fstats['size'] );
}
public function __construct() {
	   echo $this->read( STDIN );
}

tested cat file.txt | io.php and io.php < file.txt

Reason I'm looking into this is that I want to do wp export --stdout | wp wxr2pdf

wxr2pdf is at https://github.com/soderlind/wxr2pdf

[schlessera]

@soderlind The above code for reading from STDIN would be a blocking request, though. That works fine if you can rely on STDIN content being available. But if the content is not available, WP-CLI will just hang, without the user knowing what is going on...

I'll experiment some more with this, maybe it is just about finding the right combination of checking mode and reading/skipping.

[timkite]

I may have found a related issue. The WordPress plugin Authorizer uses an option called auth_settings which contains numerous sub-options. Boolean values for these sub-options must be strings, so if I want to change the value of 'ldap' from off to on, I have to set it to '1', not 1.

The following does not work, as it sets the value to a numeric 1 rather than a string '1':
wp option patch update auth_settings ldap '1'
Note: single vs. double quotes don't matter.

If I do the following, on non-interactive shells (like being called from a script), this works to set the value correctly:
echo '1' | wp option patch update auth_settings ldap

However, if I do the same in an interactive shell, it actually replaces the entirety of the auth_settings option with the string 'ldap', as if I'd run wp option update auth_settings ldap (without the patch verb) instead.

[brendon]

So glad this issue was here. Thought I was going mad.