Skip to content

Symfony upgrade - #1467

Open
Dooij wants to merge 1 commit into
atrocore:masterfrom
Dooij:symfony-dependency-upgrade
Open

Dooij wants to merge 1 commit into
atrocore:masterfrom
Dooij:symfony-dependency-upgrade

Conversation

@Dooij

@Dooij Dooij commented Aug 21, 2026

Copy link
Copy Markdown

Allow Symfony 6.4, 7.4 and 8.1 for the bundled Symfony components

Summary

The Symfony constraints in composer.json are out of date and point at versions that are no longer supported.

package before resolves today supported until after
symfony/event-dispatcher <=6.1.0@stable 6.1.0 ended January 2023 ^6.4|^7.4|^8.1@stable
symfony/mailer ~6.4.18@stable 6.4.31 November 2026 ^6.4|^7.4|^8.1@stable
symfony/yaml ^6.4.13@stable 6.4.30 November 2026 ^6.4|^7.4|^8.1@stable
symfony/html-sanitizer ^6.4.13@stable 6.4.28 November 2026 ^6.4|^7.4|^8.1@stable

The event dispatcher cap has not changed since the first commit (bed5d67e1, 2020-09-28). 7.4 is the current LTS. symfony/mime already 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 requires php >= 8.4.1 while composer.json allows >= 8.4. On a host running exactly 8.4.0, Composer picks 7.4 instead of failing to install.

Backwards compatibility

Event dispatcher

  • 6.1 to 6.4: nothing changed in what AtroCore uses. dispatch(object $event, ?string $eventName) and array callables in addListener() have been stable since 5.0.
  • 6.4 to 7.4: no change for us.
  • 7.4 to 8.1: no change for us. The only 8.1 entry is a RegisterListenersPass deprecation, which AtroCore does not use.

Mailer and SMTP

  • 6.4 to 7.4: EsmtpTransportFactory::create() now rejects any DSN scheme other than smtp and smtps. ConnectionSmtp::connect() passed an empty scheme when smtpSecurity was not SSL or TLS, so every plain SMTP connection would throw UnsupportedSchemeException. Fixed by always passing a supported scheme. On 6.4 an empty scheme and smtp both produced $tls = null, so the resulting transport is unchanged there. Verified identical host, port and tls state across ports 0, 25, 465 and 587.
  • 7.4 to 8.1: no change for us.

HTML sanitizer

  • 6.4 to 7.4: the component uses the native PHP HTML5 parser instead of masterminds/html5 on PHP 8.4 and up. 14 of 15 test inputs sanitize identically. Tables get a tbody added:

    in : <table><tr><td>cell</td></tr></table>
    6.4: <table><tr><td>cell</td></tr></table>
    7.4: <table><tbody><tr><td>cell</td></tr></tbody></table>
    

    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: MastermindsParser is removed upstream, so there is nothing left to switch between. Listeners\SettingsLayout takes the field out of the settings layout and the native parser is used.

html-sanitizer setting shown parser used
6.4 no component default (Masterminds)
7.4 yes on: Masterminds, off: Native
8.1 no Native

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 in Core/Factories/Log.php:78 skips suppressed errors. Even unsuppressed it maps to NOTICE, which is below the default logger.level of WARNING. Both checked on 7.4.15 with monolog 3.8.1.

  • 7.4 to 8.1: the same input now throws ParseException. This matters because HTMLSanitizer::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:

    7.4  out: <p>text</p>heading
    8.1  out: <p>text</p><script>alert(1)</script><h1>heading</h1>     <-- unsanitized
    

On save the severity follows the installed version, without a version check anywhere in the code. The new Services\HtmlSanitizer warns 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.

stored YAML 7.4 8.1
duplicate key, first value null 409, message shown, save possible 400, rejected
genuinely malformed 400, rejected 400, rejected

That replaces the invisible deprecation with something the person editing the field actually sees. Detection is shared: Core\Utils\YamlDuplicateKeys backs both the service check and the migration. Configurations that are already stored are handled by the migration below.

Migration V2Dot3Dot16

  • Writes the htmlSanitizerLegacyParser default into config, guarded so a rerun cannot overwrite a choice an admin has made.
  • Removes duplicate YAML keys with a null first value from data/reference-data/HtmlSanitizer.json so stored configurations keep parsing on Yaml 8. Dropping the first occurrence reproduces the result from before 8.0, where the last value won.
  • Every repair is checked by parsing the result. If the configuration parses today, the repair is kept only when the parsed structure is identical to the original. If it does not parse, lines are dropped one at a time until it does, so removal stays minimal. A block scalar containing key: lines is left alone.
  • Every outcome is logged at warning or error, 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-cli and composer containers:

  • Dependency resolution with the final constraints, at the top of the range, with --prefer-lowest, and on php 8.4.0.
  • The real HTMLSanitizer, SettingsLayout, YamlDuplicateKeys, Services\HtmlSanitizer and V2Dot3Dot16 classes executed against each version, including the layout listener against the real layouts/Settings/settings.json and the service check across all its branches.
  • composer validate passes. php -l clean on every changed PHP file, all changed JSON parses.
package 6.4 install 7.4 install 8.1 install
symfony/event-dispatcher 6.4.43 7.4.15 8.1.2
symfony/mailer 6.4.43 7.4.15 8.1.2
symfony/yaml 6.4.43 7.4.15 8.1.2
symfony/html-sanitizer 6.4.41 7.4.14 8.1.1
symfony/mime 7.4.16 8.1.4 8.1.4
symfony/cache 8.1.4 8.1.4 8.1.4
symfony/var-exporter 8.1.4 8.1.4 8.1.4
symfony/expression-language 8.1.1 8.1.1 8.1.1
*-contracts (event-dispatcher, service, cache, deprecation) 3.7.1 3.7.1 3.7.1

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-openapi uses symfony/yaml only in its YAML reader and writer, which AtroCore never calls. Core\Http\Validator feeds the generated schema through ValidatorBuilder::fromJson().
  • Monolog's SymfonyMailerHandler is never instantiated. Core\Factories\Log uses the stream, rotating file and AtroCore reporting handlers.

Notes for reviewers

  • .gitlab/review-prompt.md said PHP 8.1+ while composer.json requires >=8.4 <8.6. Corrected in passing.
  • composer.json had microsoft-connector in conflict without a vendor prefix, so composer validate rejected 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant