Symfony upgrade - #1467
Open
Dooij wants to merge 1 commit into
Open
Symfony upgrade#1467Dooij wants to merge 1 commit into
Dooij wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow Symfony 6.4, 7.4 and 8.1 for the bundled Symfony components
Summary
The Symfony constraints in
composer.jsonare out of date and point at versions that are no longer supported.<=6.1.0@stable^6.4|^7.4|^8.1@stable~6.4.18@stable^6.4|^7.4|^8.1@stable^6.4.13@stable^6.4|^7.4|^8.1@stable^6.4.13@stable^6.4|^7.4|^8.1@stableThe event dispatcher cap has not changed since the first commit (
bed5d67e1, 2020-09-28). 7.4 is the current LTS.symfony/mimealready resolves to 7.4 through mailer, so components from different majors run side by side today.6.4 stays in the range so existing installations do not move version until someone runs
composer update. 7.4 stays in the range because Symfony 8.1 requiresphp >= 8.4.1whilecomposer.jsonallows>= 8.4. On a host running exactly 8.4.0, Composer picks 7.4 instead of failing to install.Backwards compatibility
Event dispatcher
dispatch(object $event, ?string $eventName)and array callables inaddListener()have been stable since 5.0.RegisterListenersPassdeprecation, which AtroCore does not use.Mailer and SMTP
EsmtpTransportFactory::create()now rejects any DSN scheme other thansmtpandsmtps.ConnectionSmtp::connect()passed an empty scheme whensmtpSecuritywas notSSLorTLS, so every plain SMTP connection would throwUnsupportedSchemeException. Fixed by always passing a supported scheme. On 6.4 an empty scheme andsmtpboth produced$tls = null, so the resulting transport is unchanged there. Verified identical host, port and tls state across ports 0, 25, 465 and 587.HTML sanitizer
6.4 to 7.4: the component uses the native PHP HTML5 parser instead of
masterminds/html5on PHP 8.4 and up. 14 of 15 test inputs sanitize identically. Tables get atbodyadded:To keep existing output we added the setting
htmlSanitizerLegacyParser(Admin > Settings > Security, default on), which keeps the sanitizer on the Masterminds parser until an admin opts out.7.4 to 8.1:
MastermindsParseris removed upstream, so there is nothing left to switch between.Listeners\SettingsLayouttakes the field out of the settings layout and the native parser is used.YAML
6.4 to 7.4: parsing a duplicate mapping key whose first value is null became deprecated. That deprecation never reaches the log, for two independent reasons. Symfony raises it through
@trigger_error(..., E_USER_DEPRECATED), and the Monolog error handler registered inCore/Factories/Log.php:78skips suppressed errors. Even unsuppressed it maps toNOTICE, which is below the defaultlogger.levelofWARNING. Both checked on 7.4.15 with monolog 3.8.1.7.4 to 8.1: the same input now throws
ParseException. This matters becauseHTMLSanitizer::sanitize()returns the content untouched when the configuration cannot be parsed, so a sanitizer configuration that worked on 6.4 stops sanitizing without any visible error:On save the severity follows the installed version, without a version check anywhere in the code. The new
Services\HtmlSanitizerwarns when a configuration still parses but repeats a key with a null value. It reuses the existing 409 confirm flow, so the admin sees the message and can still save. When the configuration does not parse at all,Repositories\HtmlSanitizer::beforeSave()rejects it outright as it already did.That replaces the invisible deprecation with something the person editing the field actually sees. Detection is shared:
Core\Utils\YamlDuplicateKeysbacks both the service check and the migration. Configurations that are already stored are handled by the migration below.Migration V2Dot3Dot16
htmlSanitizerLegacyParserdefault into config, guarded so a rerun cannot overwrite a choice an admin has made.data/reference-data/HtmlSanitizer.jsonso stored configurations keep parsing on Yaml 8. Dropping the first occurrence reproduces the result from before 8.0, where the last value won.key:lines is left alone.warningorerror, so nothing changes silently.Verification
Verification ran against real installs of 6.4.43, 7.4.15 and 8.1.2 in throwaway
php:8.4-cliandcomposercontainers:--prefer-lowest, and onphp 8.4.0.HTMLSanitizer,SettingsLayout,YamlDuplicateKeys,Services\HtmlSanitizerandV2Dot3Dot16classes executed against each version, including the layout listener against the reallayouts/Settings/settings.jsonand the service check across all its branches.composer validatepasses.php -lclean on every changed PHP file, all changed JSON parses.*-contracts(event-dispatcher, service, cache, deprecation)Across the whole vendor tree (accessible to me, and from what I could see from my atro-pim installation) only five files use these four components, all of them in
atrocore/core, and all five are covered by the checks above. Specifically:devizzent/cebe-php-openapiusessymfony/yamlonly in its YAML reader and writer, which AtroCore never calls.Core\Http\Validatorfeeds the generated schema throughValidatorBuilder::fromJson().SymfonyMailerHandleris never instantiated.Core\Factories\Loguses the stream, rotating file and AtroCore reporting handlers.Notes for reviewers
.gitlab/review-prompt.mdsaid PHP 8.1+ whilecomposer.jsonrequires>=8.4 <8.6. Corrected in passing.composer.jsonhadmicrosoft-connectorinconflictwithout a vendor prefix, socomposer validaterejected the whole file and the rule could never match a real package. I have removed it. Let me know if it needs to be restored with a proper prefix.