Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP7 Uniform Variable Syntax (was export permissions don't work with PHP7) #4597

Closed
9 tasks done
rdoursenaud opened this issue Feb 10, 2016 · 4 comments
Closed
9 tasks done
Assignees
Labels
Bug This is a bug (something does not work as expected) Priority - High / Blocking This is a security hole or a bug that make a feature not possible to use or very expected feature.

Comments

@rdoursenaud
Copy link
Contributor

When using PHP7, export is always unavailable.
The interface always displays NotEnoughPermissions.

After a deep analysis, the defect is caused by a backward incompatible change introduced by the "Uniform Variable Syntax" RFC.

I've published a condensed example in the following gist to help understanding the behavior differences: https://gist.github.com/rdoursenaud/224ac8dff52518402cc5

In Dolibarr the offending code is in export.class.php around line 134.
Eg.:

$bool = $user->rights->$perm[0]->$perm[1]->$perm[2];

In PHP7, this is interpreted as

$bool = ((($user->rights->$perm)[0]->$perm)[1]->$perm)[2];

and triggers a "Notice: Array to string conversion" and "Notice: Undefined property".
$bool then becomes a null.

The workaround is to explicitly require the old behavior by adding braces to force precedence:

$bool = $user->rights->{$perm[0]}->{$perm[1]}->{$perm[2]};

I believe that this problem is widespread in Dolibarr codebase and can have very uncontrolled behavior.
IMHO, we should warn users that Dolibarr is incompatible with PHP7 for the moment and should not be used with it before this bug is ironed out.

Edit – List of affected files and number of occurrences:

  • htdocs/admin/oauth.php:2
  • htdocs/admin/delais.php:4
  • htdocs/admin/dict.php:51
  • htdocs/core/class/extrafields.class.php:16
  • htdocs/core/lib/functions2.lib.php:1
  • htdocs/core/lib/files.lib.php:12
  • htdocs/imports/class/import.class.php:5
  • htdocs/exports/class/export.class.php:5
  • htdocs/includes/phpoffice/phpexcel/Classes/PHPExcel/Worksheet/AutoFilter/Column.php:2
@rdoursenaud rdoursenaud added version develop Priority - High / Blocking This is a security hole or a bug that make a feature not possible to use or very expected feature. Bug This is a bug (something does not work as expected) labels Feb 10, 2016
@rdoursenaud rdoursenaud added this to the 3.9.0 milestone Feb 10, 2016
@jfefe
Copy link
Contributor

jfefe commented Feb 10, 2016

Following regex can be used to find related code :

^.*\$[a-z]+->rights->\$[a-z]+.*$

@rdoursenaud
Copy link
Contributor Author

@jfefe The best matching simple one is ->\$\w+\[

Using ag --php --ignore-dir=/htdocs/custom "\-\>\\\$\w+\[" -c, I got a list with the affected files and number of occurrences. I added this list to the original report for completeness.

@rdoursenaud rdoursenaud changed the title Export permissions don't work with PHP7 (Uniform variable syntax) PHP7 Uniform Variable Syntax (was export permissions don't work with PHP7) Feb 11, 2016
@rdoursenaud
Copy link
Contributor Author

OK, now that we have a proper search, I'm preparing a fix.

@rdoursenaud rdoursenaud self-assigned this Feb 11, 2016
@rdoursenaud
Copy link
Contributor Author

Reported PHPExcel bug upstream: PHPOffice/PHPExcel#818

eldy added a commit that referenced this issue Feb 14, 2016
FIX #4597 PHP 7 Uniform Variable Syntax
@eldy eldy closed this as completed in d038ca1 Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is a bug (something does not work as expected) Priority - High / Blocking This is a security hole or a bug that make a feature not possible to use or very expected feature.
Projects
None yet
Development

No branches or pull requests

2 participants