Symfony 6 Support

[esetnik]

Is your feature request related to a problem? Please describe.
Symfony 6 has been stable since November 2021 and simplesamlphp only supports v5 currently. This blocks installation of other packages, e.g. illuminate v9 which require symfony 6.

Describe the solution you'd like
Add support for Symfony 6.

Describe alternatives you've considered
None

[tvdijen]

I've made a start by adding symfony/phpunit-bridge as a dev-dependency.. The following issues have to be resolved before we can start testing with Symfony 6.x;

  1x: SimpleSAML\Session implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary)
    1x in SimpleTest::setUp from SimpleSAML\Test\Auth

Remaining direct deprecation notices (3)

  1x: Return type of Gettext\Translations::offsetSet($index, $value) should either be compatible with ArrayObject::offsetSet(mixed $key, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
    1x in TemplateTest::testSyntax from SimpleSAML\TestUtils

  1x: SAML2\XML\Chunk implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary)
    1x in MetaDataStorageSourceTest::testStaticXMLSource from SimpleSAML\Test\Metadata

  1x: SAML2\XML\saml\AttributeValue implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary)
    1x in SAMLParserTest::testHiddenFromDiscovery from SimpleSAML\Test\Metadata

Other deprecation notices (1)

  1x: Mock_Session_1ce1c398 implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary)
    1x in ConfigTest::setUp from SimpleSAML\Test\Module\admin\Controller

I think, looking at this, we may have to finish upgrading gettext as well.. I'm not sure this is feasible for our next major release..

[thijskh]

It’s not required that it works without any deprecation? Deprecated == it still works

[tvdijen]

You are correct Thijs.. Apparently the phpunit-bridge also detects PHP deprecations.. I must add that this text comes from the PHPUnit test for PHP 8.1..
You have to read this as; if you want to support PHP 8.1, you have to fix these deprecations.. The serialization has changed significally, as someone else already noticed here..
Also note that PHP 8.1 doesn't gracefully deal with deprecations anymore.. An exception is thrown, unless you specifically mute them in your PHP-config..

We could decide to have SSP 2.0 only support up to PHP 8.0, but that's probably gonna disappoint some people.. Anyway, I think it's safe to add support for symfony 6.0 and start doing some tests with it..

[tvdijen]

Ok, so I've added Symfony 6 support to the composer-files.. It could work on PHP 8.0, but we probably have to resolve the above issues for PHP 8.1..
@esetnik If you are able to test our master branch and share your experiences, that would be really helpful, thanks!

[esetnik]

Ok, so I've added Symfony 6 support to the composer-files.. It could work on PHP 8.0, but we probably have to resolve the above issues for PHP 8.1.. @esetnik If you are able to test our master branch and share your experiences, that would be really helpful, thanks!

I am using php 8.1 so should I wait until we have a known compatible version on master before I begin testing?

[tvdijen]

Yeah, I don't think it will work on 8.1.. The gettext-error is out of our control and has to be fixed first.
You should be able to make this work with tagged release v2.0.0-beta.6 if you set PHPs error_reporting setting to E_ALL & ~E_DEPRECATED..

[esetnik]

The first error i'm seeing is

 NOTICE: PHP message: PHP Fatal error:  Declaration of SimpleSAML\XHTML\Template::send() must be compatible with Symfony\Component\HttpFoundation\Response::send(): static in /usr/share/nginx/html/vendor/simplesamlphp/simplesamlphp/lib/SimpleSAML/XHTML/Template.php on line 532

[tvdijen]

Hmm, the only way around this would be to bump the minimum PHP-version to 8.0 and drop Symfony 5.4 support..
I think that's a bit too soon for us.. @thijskh what do you reckon?

[tvdijen]

We've discussed this internally and we feel that it's too soon to drop support for Symfony 5.4 and we don't see a way to support 5.4 and 6.x simultaneously due to the changes in method signatures in Symfony.
Support for Symfony6 will therefore not be included in SimpleSAMLphp 2.0 and has to wait for a future major release where we can drop support for Symfony 5.4 and raise the minimum PHP version to 8.x.

[tvdijen]

I've been giving this a little bit more thought and we could work our way around this if we duplicate the following classes;
SimpleSAML\XHTML\Template
SimpleSAML\Kernel
SimpleSAML\Command\RouterDebugCommand
and make Symfony6-compatible versions of them.. We can trick PHP into using the right one by adding a class-alias based on the installed version of Symfony.. It's kinda dirty and I'm not sure if we're willing to step into this snake-pit. Just dropping the suggestion..

[janklan]

Adding to the discussion, I'd say there is nothing wrong with releasing a two major versions simultaneously. 2.x and 3.x with the same features, but different baseline dependencies. Why not?

It adds a bit to the maintenance complexity, but if the real changes are only in a few classes' signatures, the complexity involves merging changes to two major-version-specific branches, instead of one. Symfony project itself does it all the time.

[tvdijen]

Yeah, with current capacity, that's just not going to happen..

[Veraxus]

Isn't a major version jump (e.g. 1.x → 2.x) the perfect opportunity to make important breaking changes like moving to Symfony 6?

PHP 7 is fully EOL in November, which is only 6 months away... and this library is already facing compatibility problems with environments and packages that are, as of now, already out-of-date themselves. Not moving on to Symfony 6 and PHP 8 before November 28 (PHP 7.4's EOL date), simplesamlphp itself stands to be considered inherently insecure itself, does it not?

[tvdijen]

@Veraxus It does not, because vendors like RedHat keep maintaining PHP...
RHEL7 ships with PHP 5.6 and is not end-of-life until June 30, 2024.

We have been postponing our 2.0 release for too long, so our current goal is to release this ASAP and from then up the release-cycle..

[tvdijen]

I've spent the night checking into this to see if there's an easy way.. There's just so much stuff breaking with Symfony 6 that there is absolutely no way we're adding this to SSP 2.0... Feel free to file a PR if this is important to you.

[nathanjrobertson]

@Veraxus It does not, because vendors like RedHat keep maintaining PHP... RHEL7 ships with PHP 5.6 and is not end-of-life until June 30, 2024.

For years I'd made the assumption "supported" meant "secure" in LTS linux distros. About 3 years ago we did our own internal audit of the LTS distros from the mainstream vendors and their patching of the major packages as the distro aged. We found that the backporting was more "best effort", and as things age (and packages like PHP get older and further from the upstream supported version), it takes more effort to do the backport. Something like Apache httpd (which doesn't move much) would be fairly easy to backport for. Backporting to PHP 5.6 patches when 7.4 is only months away from being EOL? The outcome of our audit actually showed the short term supported distributions were actually the best at patching, because the amount of effort to patch recently released software was trivial compared to attempting to backport from something several major versions into the future. The LTS distros (paid or free) were pretty hit and miss, and didn't really map to the sales pitch.

Short version: Ignore what RHEL 7 ships with on a product which is designed to run on the public internet. In the case of RHEL, they use short term supported software collections to provide more up to date packages than in the base distribution, which currently includes PHP 7.3 (also not supported upstream anymore).

[nathanjrobertson]

Isn't a major version jump (e.g. 1.x → 2.x) the perfect opportunity to make important breaking changes like moving to Symfony 6?

PHP 7 is fully EOL in November, which is only 6 months away... and this library is already facing compatibility problems with environments and packages that are, as of now, already out-of-date themselves. Not moving on to Symfony 6 and PHP 8 before November 28 (PHP 7.4's EOL date), simplesamlphp itself stands to be considered inherently insecure itself, does it not?

No. Symfony 5.x does support PHP 8. If you take a look through the Symfony 5.4 changelog, there's PRs being merged for issues with PHP 8.1 and 8.2, so it should work.

To be clear, I'd like Symfony 6 support too. We use simpleSAMLphp as a library within our Symfony apps, and so it holds us back from upgrading. When it does finally support Symfony 6, we'll need to upgrade all of our apps to Symfony 6 at that time. Hopefully they can figure out a way to support Symfony 5.x and 6.x in parallel for a short period of time to allow people to upgrade.

[tvdijen]

Hopefully they can figure out a way to support Symfony 5.x and 6.x in parallel for a short period of time to allow people to upgrade.

Again, PR's are welcome. I wasn't able to get 6.x to work at all and there is zero Symfony-expertise here.

[Berdir]

PR for Symfony 6 compatiblity: #1675

No major breaking changes, all the required changes are compatible with Symfony 5.4 as well. Please test and give feedback.

[tvdijen]

Was backported to 2.0 release branch. A new release candidate for 2.0 will be released soon

[github-actions]

\n This issue has been automatically locked since there has \n not been any recent activity after it was closed.\n Please open a new issue for related bugs.