[Console] ArgvInput - Short command input using incorrect environment name

[codereviewvideos]

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	no
RFC?	no
Symfony version	3.4.5-DEV

There appears to be a bug here, but I haven't been able to figure out a solution.

The phpunit tests pass.

If I run bin/console cac:cl -e=dev I'm seeing:

PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.dev/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 291
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.dev/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.dev/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.dev/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:291

The bigger issue is:

In FileLocator.php line 44:

  The file "/var/www/api.codereviewvideos.dev/app/config/config_=dev.yml" does not exist.

The =dev bit being the problem.

I tracked this down to the change below in getParameterOption.

return substr($token, strlen($leading));

Looking at the old code:

return substr($token, $pos + 1);

If I change to:

return substr($token, strlen($leading) + 1);

Then the command completes - but the strpos warnings remain.

I added the following just before the new return on line 321:

\Doctrine\Common\Util\Debug::dump([
    'value' => $value,
    'token' => $token,
    '0 === strpos($value, "--")' => 0 === strpos($value, '--'),
    'leading' => $leading,
    'strlen($leading)' => strlen($leading),
    'substr($token, strlen($leading))' => substr($token, strlen($leading) + 1),
]);

If I e.g. run the cache:clear command again I see:

array (size=6)
  'value' => string '-e' (length=2)
  'token' => string '-e=dev' (length=6)
  '0 === strpos($value, "--")' => boolean false
  'leading' => string '-e' (length=2)
  'strlen($leading)' => int 2
  'substr($token, strlen($leading))' => string 'dev' (length=3)

Looks good.

Now if I run the tests I see 8 failures (provideGetParameterOptionValues) e.g:

8) Symfony\Component\Console\Tests\Input\ArgvInputTest::testGetParameterOptionEqualSign with data set #13 (array('app/console', 'foo:bar', '--', '--env=dev'), '--env', false, 'dev')
->getParameterOption() returns the expected value
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'dev'
+'ev'

One extra thing, this only affects the short options (-e=dev), using the longer form (--env=dev) does not trigger this issue. However, the strpos(): Empty needle issue occurs for both long and short form.

Sorry I haven't been able to figure this out, but I hope this helps in some way in tracking down the root cause. Thank you for your time.

[chalasr]

ping @greg-1-anderson this seems to be related to the changes made in #25893, could you please have a look?

[greg-1-anderson]

When using short options, -e=dev is incorrect. -edev is the correct form to use with short options that have values.

Explanation is in the summary of #25825; see also #25825 (comment).

[greg-1-anderson]

Is the "empty needle" error happening on current (dev) Symfony code? That shouldn't happen with input of -e=dev; you should just get a value of =dev in that instance. I'll try to reproduce in a bit when I have some time.

[codereviewvideos]

Hi @greg-1-anderson

Regarding the short options - I did not know that e.g. -e=dev was incorrect. Thank you for making me aware of this.

Having checked, there is no issue when correctly using the short option syntax.

This said, in my opinion it would be confusing to suddenly stop "supporting" the incorrect short hand syntax in a patch release.

On the empty needle issue, I put a print_r($values); on ArgvInput:279

(3.4.4)

php bin/console cac:cl -e acceptance
Array
(
    [0] => --no-debug
    [1] => 
)
Array
(
    [0] => --ansi
)
Array
(
    [0] => --no-ansi
)
Array
(
    [0] => --no-interaction
    [1] => -n
)
Array
(
    [0] => --quiet
    [1] => -q
)
Array
(
    [0] => -vvv
)
Array
(
    [0] => --verbose=3
)
Array
(
    [0] => -vv
)
Array
(
    [0] => --verbose=2
)
Array
(
    [0] => -v
)
Array
(
    [0] => --verbose=1
)
Array
(
    [0] => --verbose
)
Array
(
    [0] => --version
    [1] => -V
)
Array
(
    [0] => --help
    [1] => -h
)

 // Clearing the cache for the acceptance environment with debug true  

[OK] Cache for the "acceptance" environment (debug=true) was successfully cleared.

(3.4.x-dev 01ccae8)

bin/console cac:cl -e acceptance
Array
(
    [0] => --no-debug
    [1] => 
)
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 290
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:290
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 290
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:290
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 290
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:290
Array
(
    [0] => --ansi
)
Array
(
    [0] => --no-ansi
)
Array
(
    [0] => --no-interaction
    [1] => -n
)
Array
(
    [0] => --quiet
    [1] => -q
)
Array
(
    [0] => -vvv
)
Array
(
    [0] => --verbose=3
)
Array
(
    [0] => -vv
)
Array
(
    [0] => --verbose=2
)
Array
(
    [0] => -v
)
Array
(
    [0] => --verbose=1
)
Array
(
    [0] => --verbose
)
Array
(
    [0] => --version
    [1] => -V
)
Array
(
    [0] => --help
    [1] => -h
)

 // Clearing the cache for the acceptance environment with debug true 

[OK] Cache for the "acceptance" environment (debug=true) was successfully cleared.

To answer your questions specifically:

Is the "empty needle" error happening on current (dev) Symfony code?

yes (I'm using commit hash 01ccae8)

That shouldn't happen with input of -e=dev; you should just get a value of =dev in that instance.

This is what I see:

bin/console cac:cl -e=acceptance
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 289
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:289
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 289
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:289

In FileLocator.php line 44:
                                                                                                         
  The file "/var/www/api.codereviewvideos.acceptance/app/config/config_=acceptance.yml" does not exist.



---

bin/console cac:cl -e=dev       
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 289
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:289
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 289
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:289

In FileLocator.php line 44:
                                                                                                  
  The file "/var/www/api.codereviewvideos.acceptance/app/config/config_=dev.yml" does not exist.

Again, sorry I haven't been able to find a reason as to why this is happening.

Thanks for your time and help,

Chris

[greg-1-anderson]

This said, in my opinion it would be confusing to suddenly stop "supporting" the incorrect short hand syntax in a patch release.

The "support" for -e=dev was inconsistent. getParameterOption() expected the =, but getOption() did not allow it. My opinion is that it is better to eliminate this inconsistency as soon as possible, but I'm not a maintainer here.

PHP Warning: strpos(): Empty needle in

This should definitely be fixed.

[greg-1-anderson]

@codereviewvideos I think that the root bug is here:

https://github.com/codereviewvideos/lets-explore-symfony-4/blob/9cd23e3387a563c03504a8fe8addcd71e10b5b68/bin/console#L27

$debug = ($_SERVER['APP_DEBUG'] ?? ('prod' !== $env)) && !$input->hasParameterOption(['--no-debug', '']);

Is there any reason that you need to pass an empty string as the last element to the parameter to hasParameterOption()? You should not get the warning above if you remove this entry, and test only for --no-debug.

That said, I think it would be reasonable to fix up Symfony so that it does not emit a warning when an empty string is passed to the hasParameterOption() argument array, as it did not warn in this way in previous releases.

[greg-1-anderson]

@codereviewvideos See #26156

[Simperfit]

I think we need to reword this one for a minor (by deprecating things we don't want to support) and a major release to avoid all this bugs and to be able to support all the syntax we need to support. WDYT @greg-1-anderson @chalasr ?

[chalasr]

@Simperfit We need to fix that bug where it has been introduced (2.7), new deprecations cannot be added there. If you think something is buggy and should go away in the next major, please consider opening a dedicated discussion.

[Simperfit]

@codereviewvideos does it fix the bug for you ?

[codereviewvideos]

Not to the best of my knowledge, apologies not looked into this at all today. Here's my output from a quick test:

composer update --prefer-dist
    
Package operations: 0 installs, 2 updates, 0 removals
  - Updating symfony/symfony (v3.4.4 => 3.4.x-dev 7bcccef):  Checking out 7bcccefb60


bin/console cac:cl -e dev
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 289
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:289
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 289
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:289
PHP Warning:  strpos(): Empty needle in /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php on line 289
PHP Stack trace:
PHP   1. {main}() /var/www/api.codereviewvideos.acceptance/bin/console:0
PHP   2. Symfony\Component\Console\Input\ArgvInput->hasParameterOption() /var/www/api.codereviewvideos.acceptance/bin/console:21
PHP   3. strpos() /var/www/api.codereviewvideos.acceptance/vendor/symfony/symfony/src/Symfony/Component/Console/Input/ArgvInput.php:289

 // Clearing the cache for the dev environment with debug true                                                          

                                                                                                                        
 [OK] Cache for the "dev" environment (debug=true) was successfully cleared.                                            
                                                                                                                       

Do you see the same?

[greg-1-anderson]

Try 9d3d237 on the 2.7 branch; I'm not sure this commit has made it to 3.4 or other branches.

[codereviewvideos]

Ok - really sorry, as I say, not looked into this at all as of yet. I have a real life issue that's taking precedence currently. I have some 2.x projects around which I will try this on and update. Sorry again for the delay.

Chris

[benbor]

Hi guys. @greg-1-anderson I agree with your

The "support" for -e=dev was inconsistent.

but, this case had worked before this fix was merged. At now all commands witch called with -e=prod are broken (fyi: with very unusual error Parse error: syntax error, unexpected '=', expecting ',' or ')' in /symfony/var/cache/=prod/src=prodDebugProjectContainer.php:5), so it hard to understand what happens.
Console Input documentation doesn't have any notes about that.
Also release changelog doesn't have any information about BC break.

I propose the following:

• add note to release changelog
• add note to Console Input documentation

cc @nicolas-grekas

[greg-1-anderson]

Good suggestions, @benbor. See symfony/symfony-docs#9387 and #26378.

The changelog has already gone out; perhaps someone can still update that retroactively, though.

[AlexanderMatveev]

+1