Added support for PHP 8.1

[Koen1999]

Q	A
Documentation	no
Bugfix	no
BC Break	no
New Feature	yes
RFC	no
QA	no

Description

This PR adds support for PHP 8.1. No big changes had to be made luckily. I ran a code inspection indicating no compatibility issues. This fixes #23 .

I would like to use the Github actions to see if the tests pass. I'll fix any issues that come up.

[froschdesign]

Uff, there is a LDAP adapter. 🙈

Related to:

• https://github.com/laminas/laminas-ldap/pull/23
• https://github.com/laminas/laminas-ldap/pull/20

[Koen1999]

As a sidenote, locally the tests for PHP 8.1 passed with the exception of the LDAP online tests which I could not run. Moreover, this test did not contain any assertions which it warned about: https://github.com/laminas/laminas-authentication/blob/e1f27ac430aed5f6b53ac6f8712b5f1b570f00d0/test/Adapter/HttpTest.php#L31

[Koen1999]

I rebased on the 2.9.x branch for you.

[Koen1999]

How should I deal with internal deprecation warnings like these? https://github.com/laminas/laminas-authentication/runs/4379957729?check_suite_focus=true#step:3:611 They were not introduced by this PR as far as I can tell.

[Koen1999]

Should I also remove "laminas/laminas-zendframework-bridge": "^1.0" in this repository and mark it as conflicting with the zend package?

[froschdesign]

Should I also remove "laminas/laminas-zendframework-bridge": "^1.0" in this repository and mark it as conflicting with the zend package?

As a separate pull request and we will get an entry in the release notes for it. Thanks in advance! 👍

[Koen1999]

I rebased on top of #25 , so please merge after that one

[Koen1999]

I think we should bump laminas-http to a higher version to prevent an incompatible version being installed for PHP 8.1 https://github.com/laminas/laminas-authentication/runs/4396161380?check_suite_focus=true#step:3:147

According to the changelog, support for PHP 8.1 was only added in 2.15.0 https://github.com/laminas/laminas-http/releases

[snapshotpl]

ignore_php_platform_requirements helps and breaks things ;)

[Koen1999]

I am faithful that running the CI again after a new release for laminas-ldap supporting PHP 8.1 has come available will result in all tests passing. So, I'll call it a day for now.

[Ocramius]

laminas-ldap released: re-running all jobs

[Koen1999]

There were 3 failures:

1) LaminasTest\Authentication\Adapter\DbTable\CallbackCheckAdapterTest::testGetOmittedResultRow
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'O:8:"stdClass":3:{s:2:"id";s:1:"1";s:8:"username";s:11:"my_username";s:9:"real_name";s:12:"My Real Name";}'
+'O:8:"stdClass":3:{s:2:"id";i:1;s:8:"username";s:11:"my_username";s:9:"real_name";s:12:"My Real Name";}'

Error: /github/workspace/test/Adapter/DbTable/CallbackCheckAdapterTest.php:203

2) LaminasTest\Authentication\Adapter\DbTable\CredentialTreatmentAdapterTest::testGetOmittedResultRow
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'O:8:"stdClass":3:{s:2:"id";s:1:"1";s:8:"username";s:11:"my_username";s:9:"real_name";s:12:"My Real Name";}'
+'O:8:"stdClass":3:{s:2:"id";i:1;s:8:"username";s:11:"my_username";s:9:"real_name";s:12:"My Real Name";}'

Error: /github/workspace/test/Adapter/DbTable/CredentialTreatmentAdapterTest.php:188

3) LaminasTest\Authentication\Adapter\DbTableTest::testGetOmittedResultRow
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'O:8:"stdClass":3:{s:2:"id";s:1:"1";s:8:"username";s:11:"my_username";s:9:"real_name";s:12:"My Real Name";}'
+'O:8:"stdClass":3:{s:2:"id";i:1;s:8:"username";s:11:"my_username";s:9:"real_name";s:12:"My Real Name";}'

Error: /github/workspace/test/Adapter/DbTable/CredentialTreatmentAdapterTest.php:188

Technically speaking the testGetOmittedResultRow function works. The tests try to retrieve all rows, except the one containing the password. The password is not present in the result. But apparently, in the new PHP version additional rows have become available. I will therefore adjust the tests to check for the absence of the password field instead.

[froschdesign]

@Koen1999 Please check the types:

'O:8:"stdClass":3:{s:2:"id";s:1:"1";…

vs.

'O:8:"stdClass":3:{s:2:"id";i:1;…

String vs. Integer: '1' !== 1

[Koen1999]

Aah, that makes more sense on how to read the test results. I think the bug is somewhere in the authentication part of https://github.com/laminas/laminas-authentication/blob/3.0.x/src/Adapter/DbTable/AbstractAdapter.php But sadly, I don't have time to look into this right now and I wouldn't really know where to proceed honestly.

[Koen1999]

I should add that I went over the PHP 8.1 changelog to determine what caused this change in behaviour, but I couldn't find any clues there. So I'm feeling like it's a bug introduced in PHP 8.1. But then again, I'm not the expert here.

[tobias-trozowski]

@Koen1999 i made a PR against your branch previously this day: https://github.com/Koen1999/laminas-authentication/pull/1

[Ocramius]

Sounds like what doctrine/dbal had too: possibly related?

https://github.com/doctrine/dbal/pull/5017

[Ocramius]

In practice, we don't really care if it's 1 or "1" there, do we? Perhaps we could compare the unserialized values, rather than the serialized ones? :thinking:

[tobias-trozowski]

@Ocramius even the unserialized objects would differ, in php < 8.1 results were treated as strings by sqlite. with 8.1 they are returned as proper types. see https://bugs.php.net/bug.php?id=38334

[Ocramius]

Yeah, wondering if assertEquals() (instead of assertSame()) would help us there: PHP 8.1 introduced a nasty BC break there :shrug:

[tobias-trozowski]

just added https://github.com/laminas/laminas-authentication/pull/26 as a proposal to fix this issue

[Ocramius]

Handled in #26