Fix for updater/application.php cli fatal errors #472 #482 owncloud/core#31425

[danieldjewell]

Description

As reported in several issues, running "php updater/application.php upgrade:detect/info" results in a fatal error.

Traced issue to an unnecessary/fatal "=" in the command line being provided to "php occ" by ConfigReader.php:109. I am unsure if the issue is with the original change of this code to its current revision (f78291e#diff-ad6d84de30f605974920c65454932957 ) or if the issue is one of a Symfony framework change.

The root issue appears to be Symfony\Component\Process\Process by way of occRunner->runJson interpreting the array as a key-value pair with an empty argument. This results in the argument to "occ" changing from '--private' to '--private=' ... resulting in a fatal error in "occ" and thus a fatal error in "updater/application.php".

Example cmd line: [commandline:Symfony\Component\Process\Process:private] => php /path/occ --no-warnings config:list --output=json '--private='

Nevertheless, changing ConfigReader.php:109 to simply remove the associative array assignment of an empty string (e.g. ** => '' **) appears to fix the issue.

Related Issue

• Fixes Updating from 10.0.7.2 to 10.0.8 failed  #472, php updater/application.php upgrade:detect fails #482, possibly Updater Button does not work #453 (100% untested by me on this particular issue), and Updating from 10.0.7.2 to 10.0.8 failed core#31425

Motivation and Context

I am unsure when this actually stopped working - presumably, when the original commit was made 2 years ago, it worked. My only guess is a regression and/or change in the Symfony framework process code -- or a commit/change to "occ" to make it perform more strict command line argument testing.

This proposed pull fixes the immediate issues with updater/application.php being 99.9% broken -- however, there may be other regressions/effects by the ultimate root cause that was the source of this.

How Has This Been Tested?

• test environment 1: Ubuntu 16.04/PHP 7.0/Owncloud 10.0.9 Stable
• test environment 2: Ubuntu 18.04/PHP 7.2/Owncloud 10.0.9 Stable
• test case 1: run "sudo -u www-data php updater/application.php upgrade:info"
• test result 1: it works! no more fatal error.
• other impacts: I do not believe this would impact other code since it only changes one function in the updater

Screenshots (if appropriate):

See issues for details.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests

Checklist:

• [#] Code changes
• [?] Unit tests added
• [?] Acceptance tests added
• [?] Documentation ticket raised:
  Do we really need the above for this?

Open tasks:

• Backport (if applicable set "backport-request" label and remove when the backport was done)

[danieldjewell]

More notes are in #482 - more analysis appears to be that this commit fixes an incorrect implementation of the parsing code as implemented in https://github.com/owncloud/updater/blob/master/src/Utils/OccRunner.php#L73-L80

[VicDeo]

@danieldjewell thanks a lot for taking this over!
Tested in CLI/Web UI and works

[VicDeo]

@danieldjewell May I ask you to submit another PR against stable10 branch?

[danieldjewell]

Hey @VicDeo no problem! Sorry, haven't been on in awhile and didn't see your reply until now. Glad to help! Just confirming -- did you still need me to submit another PR against stable10?

[VicDeo]

@danieldjewell yes, please

[danieldjewell]

@VicDeo done! see PR #500 - sorry it took a little while! :)

[VicDeo]

@danieldjewell thank you very much!