-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Description
Symfony version(s) affected: 4.4.7 / 5.0.7
Description
#33897 caused a regression, which I would call a BC break.
In general I agree with the changes made with the intention to allow
providing input to the application using STDIN.
There are however some edge cases demonstrated here that need some attention.
Tests with current state (symfony/console 5.0.7/4.4.7)
Test 1
Forcing detached terminal (no tty) and not providing any input via STDIN
echo '' | php console.php hidden:input
Expectation
Error message saying that two required arguments are missing
Output:
Not enough arguments (missing: "password, action").
Actual
Infinite loop.
This definitely needs a fix in AskHidden
handling to avoid the infinite loop.
Additionally it would be helpful to inform users that action argument is missing as well.
Password:
> stty: stdin isn't a terminal
stty: stdin isn't a terminal
[ERROR] Password must not be empty.
Password:
> stty: stdin isn't a terminal
stty: stdin isn't a terminal
[ERROR] Password must not be empty.
...
Test 2
Forcing detached terminal (no tty) and not providing any input via STDIN,
but providing password argument
echo '' | php console.php hidden:input 123456
Expectation
Password argument is set to "123456" and error message that action argument is missing
Output:
Not enough arguments (missing: "action").
Actual
Works in general, but would be helpful to inform users that action argument is missing
What do you want to do:
>
[ERROR] Action must not be empty.
What do you want to do:
>
Aborted.
hidden:input <password> <action>
Test 3
Forcing detached terminal (no tty) and providing password argument via STDIN
echo '123456' | php console.php hidden:input
Expectation
Password argument is set to "123456" and error message that action argument is missing
Output:
Not enough arguments (missing: "action").
Actual
Works in general, but would be helpful to inform users that action argument is missing.
Password:
> stty: stdin isn't a terminal
stty: stdin isn't a terminal
What do you want to do:
>
Aborted.
hidden:input <password> <action>
How to reproduce
Here is an example application
to test out the issues I discovered.
Possible Solution
- The infinite loop is triggered, because the hidden response is empty, but it is checked for false
Probably checking whether the return value is an empty string and setting $ret
to false in this case will fix it.
-
Calling
interact
even when no tty is available.
That is a tough one, as doing so is major part of the initial fix.
It could be mitigated, by wrapping the call in a try/catch, catching MissingInputException, setting to non iteractive in the catch and letting the application continue. This would in my case at least output the error message about the missing argument in the end. -
Cluttered output
Since the output for the questions is already sent, the output will look much more cluttered than before. Depending on the type of the cli application, this might be acceptable, but as well might not.
This could be mitigated by an additional Application option/property whether to evaluate the existance of a tty and set interactive to false in this case or not.
If this would be accepted, it would be up for decision which the default behaviour should be.