PHP8 compatibility #456

fideloper · 2020-12-21T15:55:56Z

Related to #453

TODO:

Updates phpunit version
Resolved use of libxml_disable_entity_loader()
Fixed tests The remaining are not passing but I'm not sure if that's specific to php 8
- Travis CI no longer in use it seems

Remaining Tests

Here is the confusing output remaining for phpunit tests:

Click to view

PHPUnit 9.5.0 by Sebastian Bergmann and contributors.
Warning:       No code coverage driver available

Warning:       Your XML configuration validates against a deprecated schema.

Suggestion:    Migrate your XML configuration using "--migrate-configuration"!
..............FF.........FFFFFFFFFFFFFFFFFF.............W.....W  63 / 279 ( 22%)

WWWW............WWWWWWWWW.............WWW..W.EWW............WWW 126 / 279 ( 45%)

W.E.......................................................F.... 189 / 279 ( 67%)

............................................................... 252 / 279 ( 90%)

...........................                                     279 / 279 (100%)
Time: 00:02.811, Memory: 14.00 MB
There were 2 errors:

OneLogin\Saml2\Tests\LogoutRequestTest::testGetIDException

OneLogin\Saml2\Error: LogoutRequest could not be processed

/Users/fideloper/Code/Userscape/php-saml/src/Saml2/LogoutRequest.php:201

/Users/fideloper/Code/Userscape/php-saml/tests/src/OneLogin/Saml2/LogoutRequestTest.php:916

OneLogin\Saml2\Tests\LogoutResponseTest::testGetIDException

OneLogin\Saml2\Error: LogoutResponse could not be processed

/Users/fideloper/Code/Userscape/php-saml/src/Saml2/LogoutResponse.php:93

/Users/fideloper/Code/Userscape/php-saml/tests/src/OneLogin/Saml2/LogoutResponseTest.php:560
--
There were 25 warnings:


OneLogin\Saml2\Tests\AuthnRequestTest::testCreateDeflatedSAMLRequestURLParameter

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\AuthnRequestTest::testCreateEncSAMLRequest

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\AuthnRequestTest::testWeCanChooseToCompressARequest

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\AuthnRequestTest::testWeCanChooseNotToCompressARequest

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\AuthnRequestTest::testWeCanChooseToDeflateARequestBody

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\AuthnRequestTest::testGetXML

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testConstructor

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testConstructorWithRequest

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testConstructorWithSessionIndex

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testConstructorWithNameIdFormatOnParameter

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testConstructorWithNameIdFormatOnSettings

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testConstructorWithoutNameIdFormat

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testConstructorWithNameIdNameQualifier

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testCreateDeflatedSAMLLogoutRequestURLParameter

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testConstructorEncryptIdUsingX509certMulti

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testWeCanChooseToCompressARequest

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testWeCanChooseNotToCompressARequest

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testWeCanChooseToDeflateARequestBody

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutRequestTest::testGetXML

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutResponseTest::testConstructor

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutResponseTest::testCreateDeflatedSAMLLogoutResponseURLParameter

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutResponseTest::testWeCanChooseToCompressAResponse

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutResponseTest::testWeCanChooseNotToCompressAResponse

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutResponseTest::testWeCanChooseToDeflateAResponseBody

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


OneLogin\Saml2\Tests\LogoutResponseTest::testGetXML

assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.


--
There were 21 failures:


OneLogin\Saml2\Tests\AuthTest::testRedirectTo

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testRedirectTowithUrl

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testProcessSLORequestDeletingSession

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testProcessSLORequestDeletingSessionCallback

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testProcessSLORequestRelayState

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testProcessSLORequestSignedResponse

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLogin

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLoginWithRelayState

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLoginWithRelayStateAndParameters

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLoginSigned

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLoginForceAuthN

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLoginIsPassive

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLoginNameIDPolicy

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLoginSubject

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLogout

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLogoutWithRelayState

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLogoutWithRelayStateAndParameters

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLogoutWithNameIdAndSessionIndex

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLogoutNameID

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\AuthTest::testLogoutSigned

Test was run in child process and ended unexpectedly


OneLogin\Saml2\Tests\ResponseTest::testIsInValidCert

Failed asserting that two strings are equal.

--- Expected

+++ Actual

@@ @@

-'openssl_x509_read(): supplied parameter cannot be coerced into an X509 certificate!'

+'openssl_x509_read(): X.509 Certificate cannot be retrieved'


/Users/fideloper/Code/Userscape/php-saml/tests/src/OneLogin/Saml2/ResponseTest.php:1537
ERRORS!

Tests: 279, Assertions: 1107, Errors: 2, Failures: 21, Warnings: 25.

fideloper · 2020-12-21T15:59:55Z

As this stands, I think this is OK, but I need some direction/help on fixing all the tests. I'm unsure about what these errors ean:

Test was run in child process and ended unexpectedly

And some failures seem to be related to the project logic instead of php8, but I'm not totally sure:

OneLogin\Saml2\Error: LogoutRequest could not be processed
OneLogin\Saml2\Error: LogoutResponse could not be processed

And finally this one:

21) OneLogin\Saml2\Tests\ResponseTest::testIsInValidCert
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'openssl_x509_read(): supplied parameter cannot be coerced into an X509 certificate!'
+'openssl_x509_read(): X.509 Certificate cannot be retrieved'

fideloper · 2020-12-28T13:39:45Z

Note that I need some help at this point to ensure tests are passing. Since travis is no longer runnings tests, I don't know if these tests used to work regardless of php8 or not, and don't have a good idea on if I've made backwards incompatible changes related to updates to phpunit.

In other words, I could use some extra help from from the maintainers!

pitbulk · 2020-12-28T16:48:34Z

The lastest code pushed past Travis on PHP 5.4, 5.5, 5.6 and 7.0 and had some issues with 7.1, 7.2 and 7.3

On my local environment them past so it should be some issue at the Travis, the 3.4.1 release past on Travis

Your code changes on the test made it incompatible with old versions of PHP
For example, the use of

 : void

or the method

assertStringContainsString

which was introduced in phpunit 7.5 which forces the use of PHP 7.1

pitbulk · 2020-12-28T16:57:53Z

I was able to reproduce the Travis fails on my local with recent versions of the dependencies, so I believe is related to some change in dependency behavior. But I don't get the "testIsInValidCert" test error you experienced.

fideloper · 2020-12-28T17:08:11Z

Thanks! We’ll have to decide what to do as php 8 requires a newer phpunit, which in turn requires the use the `:void` on the `setup()` method. Perhaps a new major version of the library to differentiate php 7.4+?

…

On Mon, Dec 28, 2020 at 10:58 Sixto Martin ***@***.***> wrote: I was able to reproduce the Travis fails on my local with recent versions of the dependencies, so I believe is related to some change in dependency behavior — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#456 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADSDUYFWKGM45VKYU2D3KDSXC2J7ANCNFSM4VEMUCVQ> .

pitbulk · 2020-12-28T17:10:20Z

The errors on Travis started after using phpunit 7.5.18, that introduced 2 bugfixes:

Fixed #3967: Cannot double interface that extends interface that extends \Throwable
Fixed #3968: Test class run in a separate PHP process are passing when exit called inside

I guess the php-saml test that start failing are related to #3968.
Meanwhile I fix them, you could force in your local to use phpunit 7.5.17

P.S With phpunit 7.5.17 Travis gives the OK: https://cdn.travis-ci.org/github/onelogin/php-saml/builds/751847145

pitbulk · 2020-12-28T19:28:23Z

I created #457. I think it has low priority since we can simply use phpunit 7.5.17 for now.

pitbulk · 2020-12-28T19:41:58Z

@fideloper

I already maintain the PHP5 (2.X branch) and the PHP7 (3.X branch).

I dislike introducing a new branch for PHP8 that will complicate more the maintenance of php-saml.
I prefer to try to make 3.X compatible with PHP 7 and PHP8 (maybe dropping the compatibility with PHP5 on the 3.X branch if there is no other choice)

fideloper · 2020-12-28T20:48:58Z

Sounds good!

I'm not sure if it was lost in all the changes, but the change here makes the library compatible with php8 (this change isn't a change to the tests): https://github.com/onelogin/php-saml/pull/456/files#diff-b61383b8af6d0832f4319fa27427cb2ab7c7932efc870be2a558145fbd828da0

In terms of priority, that will need to get included in order to work with php 8.

fideloper · 2020-12-31T13:33:09Z

Hi!

It looks like PHP8 requires a newer phpunit, so I can't pin version 7.5.17 of phpunit and still use PHP 8.

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires phpunit/phpunit 7.5.18 -> satisfiable by phpunit/phpunit[7.5.18].
    - phpunit/phpunit 7.5.18 requires php ^7.1 -> your php version (8.0.0) does not satisfy that requirement.

I've updated composer.json to require php 7.0 or greater, and install either phpunit 7.5.17 or ^9.5. In php 8, we'll still see these errors/warnings crop up in the tests.

fideloper · 2020-12-31T13:41:12Z

I may be getting a bit mixed up in our goals of this PR. Perhaps I (we?) can split this up into multiple PR's:

Get the library working in php 8
Get the tests working in php 8

Library Working in PHP 8

This requires an update to composer.json to change the PHP version constraint, and then ensuring that the function libxml_disable_entity_loader() isn't called in php 8.

Tests Working in PHP 8

This requires:

A different version of phpunit for php8
- Which then re-surfaces the errors Test was run in child process and ended unexpectedly
Updates to phpunit tests assertions to account for changes/depreciations

Proposal

What I think we can do then, is take care of item 1 so this library is working for those on PHP 8, and then do another PR to get the tests, etc, working for PHP8.

We can then work through the best strategy in terms of backwards compatibility, etc.

pitbulk · 2020-12-31T14:08:00Z

Sounds good

fideloper · 2020-12-31T14:19:46Z

For those looking back later, here's a diff of the original PR changes: fideloper/php-saml@3.6.0...3.6.0-original

fideloper closed this Dec 31, 2020

fideloper force-pushed the 3.6.0 branch from 52aefd9 to 9ffb241 Compare December 31, 2020 14:13

fideloper mentioned this pull request Dec 31, 2020

PHP 8 Support #458

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PHP8 compatibility #456

PHP8 compatibility #456

fideloper commented Dec 21, 2020 •

edited

fideloper commented Dec 21, 2020

fideloper commented Dec 28, 2020

pitbulk commented Dec 28, 2020

pitbulk commented Dec 28, 2020 •

edited

fideloper commented Dec 28, 2020 via email

pitbulk commented Dec 28, 2020 •

edited

pitbulk commented Dec 28, 2020

pitbulk commented Dec 28, 2020

fideloper commented Dec 28, 2020

fideloper commented Dec 31, 2020 •

edited

fideloper commented Dec 31, 2020 •

edited

pitbulk commented Dec 31, 2020

fideloper commented Dec 31, 2020

PHP8 compatibility #456

PHP8 compatibility #456

Conversation

fideloper commented Dec 21, 2020 • edited

TODO:

Remaining Tests

fideloper commented Dec 21, 2020

fideloper commented Dec 28, 2020

pitbulk commented Dec 28, 2020

pitbulk commented Dec 28, 2020 • edited

fideloper commented Dec 28, 2020 via email

pitbulk commented Dec 28, 2020 • edited

pitbulk commented Dec 28, 2020

pitbulk commented Dec 28, 2020

fideloper commented Dec 28, 2020

fideloper commented Dec 31, 2020 • edited

fideloper commented Dec 31, 2020 • edited

Library Working in PHP 8

Tests Working in PHP 8

Proposal

pitbulk commented Dec 31, 2020

fideloper commented Dec 31, 2020

fideloper commented Dec 21, 2020 •

edited

pitbulk commented Dec 28, 2020 •

edited

pitbulk commented Dec 28, 2020 •

edited

fideloper commented Dec 31, 2020 •

edited

fideloper commented Dec 31, 2020 •

edited