metaclass-nl / tresholds-governor

Tresholds Governor, aims to facilitate the protection of authentication against brute force and dictionary attacks
2 stars 1 forks source link

crash DBAL < 2.5 on PHP7 #6

Closed metaclass-nl closed 8 years ago

metaclass-nl commented 8 years ago

Doctrine\DBAL\Query\QueryBuilder::andWhere assigns $this->getQueryPart('where') to its first argument variable. On PHP 7 this results in func_get_args(), which is called AFTERWARDS, to return the Doctrine\DBAL\Query\Expression\CompositeExpression from $this->getQueryPart('where') instead of the (string) argument supplied by \Metaclass\TresholdsGovernor\Gateway\DbalGateway::countWhereSpecifiedAfter.

Then the querybuilder adds the 'arguments' as extra AND expressions resulting in self-reference(s) from the CompositeExpression in $this->sqlParts['where'](which is returned by $this->getQueryPart%28'where'%29). When the actual SQL string is being generated $this->sqlParts['where'] is casted to string resulting in an endless recursion. I guess that crashes PHP 7.

This behavior is consistent with a note in the php manual:

If the arguments are passed by reference, any changes to the arguments 
will be reflected in the values returned by this function. 
As of PHP 7 the current values will also be returned if the arguments are passed by value."

The problem is solved in this commit in dbal v2.5.0-BETA2 with comment:

HHVM compatibility: func_get_args

All func_get_args() calls have been moved to the top of the methods
because HHVM doesn't keep a copy of the original args for performance
reasons.

Tested on PHP7.0.1 windows VC14 x64 Non Thread Safe (2015-Dec-17 00:17:18) with Symfony v2.7.7, doctrine/dbal v2.4.4., metaclass-nl/authentication-guard-bundle 5ca9e55f744ead919ec9e630745010187f037652, metaclass/tresholds-governor 9a74fae6f4e90c2aee18fcdc1cfa68d9dc6b1980

metaclass-nl commented 8 years ago

On 14-Jan-2016 17:43, Marco Pivetta wrote:

2.4.x is not going to get new releases, unless there are security issues. Please upgrade to 2.5.

gplanchat commented 8 years ago

Same issue here on PHP 7.0.4 from homebrew and doctrine/dbal 2.4.x-dev (f5b85d182799c539d1a56aa145977aa2c43dfe1a) :

PHP 7.0.4 (cli) (built: Mar 25 2016 00:02:51) ( ZTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies
    with Xdebug v2.4.0, Copyright (c) 2002-2016, by Derick Rethans

Symptoms are an infinite recursion in the Doctrine\DBAL\Query\Expression\CompositeExpression, the error isn't easy to detect.

For the report, everything was fixed after upgrading to version 2.5

metaclass-nl commented 8 years ago

To solve this by updating symfony one needs to update ones own composer.json to reflect the version numbers in the release of Syfony framework standard edition that is used before running composer update.

IMHO the http://symfony.com/doc/current/cookbook/upgrade/minor_version.html should instruct one to allways do this instead of only mentioning it under "Dependency Errors" after "If this still doesn't work": debugging a crash like this is hard and even if one succeeds, finding that all that effort was only to find a "dependency error" is kind of frustrating.. )

http://symfony.com/doc/current/cookbook/upgrade/patch_version.html does not mention it at all, but the crash may also occur with a patched version of the Symfony 2.3 (a long term support version - yes, bug fixes are no longer supported now, but they were in januari, when this issue was opened).

metaclass-nl commented 8 years ago

Release v0.3 no longer uses DBAL's QueryBuilder, so it should also work on versions of DBAL < 2.5 with PHP 7, but this was not tested.