Skip to content

[Console] Remaining regressions due to #33897 #36565

@helhum

Description

@helhum

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

  1. 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.

  1. 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.

  2. 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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions