DownloadTicketService / dl

Download Ticket Service
https://www.thregr.org/~wavexx/software/dl/
GNU General Public License v2.0
84 stars 30 forks source link

Support PHP 5.3 #50

Closed fanis closed 7 years ago

fanis commented 7 years ago

The requirements state that PHP 5.3 and higher is supported, but the function not_empty() stumbles on an issue with isset() prior to PHP 5.5 that brings out a fatal error:

Fatal error: Can't use function return value in write context in C:\work\root\dl\htdocs\include\funcs.php on line 293

The issue lies with the following code:

function not_empty(&$v)
{
  return isset($v) && !empty(trim($v));
}

From the documentation on empty():

Note: Prior to PHP 5.5, empty() only supports variables; anything else will result in a parse error. In other words, the following will not work: empty(trim($name)). Instead, use trim($name) == false.

wavexx commented 7 years ago

On Tue, Aug 08 2017, Fanis Hatzidakis wrote:

The requirements state that PHP 5.3 and higher is supported, but the function not_empty() stumbles on an issue with isset() prior to PHP 5.5 that brings out a fatal error:

Fatal error: Can't use function return value in write context in C:\work\root\dl\htdocs\include\funcs.php on line 293

Do you actually still use PHP 5.3?

The oldest version I'm currently having access to is 5.6, and I would be tempted to raise the requirement to 5.5 (moving isset outside of other functions for testing is a major PITA, not just for this specific context).

fanis commented 7 years ago

I happened to test it on an old PHP 5.4 development instance I had lying around, that's why I noticed it.

By all means update the requirements :)

wavexx commented 7 years ago

On Tue, Aug 08 2017, Fanis Hatzidakis wrote:

I happened to test it on an old PHP 5.4 development instance I had lying around, that's why I noticed it.

Out of curiosity, what distribution is that?

Just to make sure we support at least the last stable release or major distributions.

fanis commented 7 years ago

Just an older manual installation. I ended up running this on a Debian 8.1, which is PHP 5.6.30,

wavexx commented 7 years ago

On Tue, Aug 08 2017, Fanis Hatzidakis wrote:

Just an older manual installation. I ended up running this on a Debian 8.1, which is PHP 5.6.30,

Then I'll raise the minimum compat to 5.5. This allows me to drop the bundled PasswordHash.

wavexx commented 7 years ago

Minimum requirement is now 5.5 to take advantage of password API.