Skip to content

Commit 3036bcb

Browse files
bug #65541 [Serializer] Fix denormalizing mime messages typed as RawMessage (nicolas-grekas)
This PR was merged into the 6.4 branch. Discussion ---------- [Serializer] Fix denormalizing mime messages typed as RawMessage | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #33394 | License | MIT Sending emails asynchronously does not work when the messenger transport uses the Symfony serializer instead of the PHP one. `SendEmailMessage` declares its message as `Symfony\Component\Mime\RawMessage`, so the serializer is asked to denormalize the payload into that base class. `MimeMessageNormalizer` did not support it: `supportsDenormalization()` only accepted `Message` and its subclasses, so the generic object normalizer took over. It built a `RawMessage` with a `null` body, because `RawMessage::__construct()` accepts `mixed`. The email was therefore silently replaced by an empty raw message, and the worker later failed with `foreach() argument must be of type array|object, null given` when the transport tried to read it. On older versions, where that constructor had no type declaration, the same payload produced the `MissingConstructorArgumentsException` reported in the issue. The normalizer already records the concrete class of mime parts under a `class` key, so that they can be restored from the abstract `AbstractPart` type. This applies the same treatment to messages: `normalize()` now adds the message class, and `denormalize()` uses it when the requested type is `RawMessage` itself. As for parts, the value is rejected when it does not name a subclass of `RawMessage`. Backward compatibility: * payloads written before this change carry no `class` key, and denormalizing them into a concrete type such as `Email` behaves exactly as before; * payloads written after this change carry one extra key, which older consumers ignore as an unknown attribute; * when a concrete type is requested, that type still wins, so `denormalize($data, Email::class)` keeps returning an `Email`; * `denormalize(['message' => 'raw'], RawMessage::class)` keeps returning a plain `RawMessage`; * malformed payloads keep reporting serializer exceptions rather than PHP errors, so `COLLECT_DENORMALIZATION_ERRORS` still works on them. `supportsNormalization()` widens to `RawMessage` as well, so a plain raw message now goes through this normalizer instead of the generic object normalizer. Its content used to be dropped: `normalize(new RawMessage('raw body'))` returned an empty array. It now returns the body and the class. Two edge cases come with that change. A raw message built on a resource now throws a `NotNormalizableValueException`, which matches the docblock of the class saying that such messages are not serializable. A raw message built on a generator now has that generator consumed by `normalize()`, so the object itself cannot be sent afterwards. Both used to normalize to an empty array, which lost the message anyway. What to watch on CI, none of which is expected to be fixed here: * the two `^` groups of the high-deps job are red and stay red. That job runs the 5.4 tests of the patched components against this branch, and the 5.4 serialization fixtures of Mime and of the Twig bridge do not expect the new `class` key. Those fixtures sit on a frozen branch, so merging this up does not clear them. There is no runtime incompatibility behind it: a 5.4 shaped consumer denormalizing a new payload into `Email` still gets a correct `Email`. Whether that is acceptable is the one call to make before merging, since the leg would then be red for any later 6.4 pull request touching Mime, the Twig bridge or the Serializer. * the plain `Mime` and `Twig` groups of the same job are red for a different and temporary reason. The dev requirement is raised to `^6.4.44|^7.4.17` and nothing released matches yet. Low deps then falls back to the local package and is green, while high deps picks the upstream `7.4.x-dev`, which satisfies `^7.4.17` but does not carry the fix. This clears once the change is merged up into 7.4. * the four failures of the `Serializer^` group are pre-existing on any 6.4 serializer patch and are unrelated to mime. The `conflict` entry of Mime still reads `symfony/serializer: <6.4.3|>7.0,<7.0.3`. It is left alone on purpose. Mime itself is not modified here, and old and new payloads are read correctly by both the old and the new serializer, so there is no version pair to forbid. Checks that were run: * `MimeMessageNormalizerTest` against the unpatched normalizer: 2 errors and 2 failures, the central one being `Failed asserting that Symfony\Component\Mime\RawMessage Object (...) is an instance of class "Symfony\Component\Mime\Email"`; * `src/Symfony/Component/Serializer/Tests`: 1157 tests, 2329 assertions, 11 skipped, no failure; * `src/Symfony/Component/Mime/Tests`: 401 tests, 1617 assertions, 1 skipped, no failure; * `src/Symfony/Bridge/Twig/Tests`: 1756 tests, 2042 assertions, 14 skipped, no failure; * `src/Symfony/Component/Mailer/Tests` and `src/Symfony/Component/Messenger/Tests`: 166 and 410 tests, no failure; * a script that encodes a `SendEmailMessage` with `Symfony\Component\Messenger\Transport\Serialization\Serializer` and decodes it back: it returned a `RawMessage` with a null body before the change, and returns the original `Email` with its headers and text body after it; * the same script fed with a malformed body such as `{"message":"raw text","envelope":null}`: it returns a `RawMessage` both before and after the change. Commits ------- 0a709501887 [Serializer] Fix denormalizing mime messages typed as RawMessage
2 parents 3b84455 + 8e436c7 commit 3036bcb

2 files changed

Lines changed: 3 additions & 2 deletions

File tree

Tests/Mime/TemplatedEmailTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ public function testSymfonySerialize()
111111
}
112112
]
113113
},
114-
"body": null
114+
"body": null,
115+
"class": "Symfony\\\\Bridge\\\\Twig\\\\Mime\\\\TemplatedEmail"
115116
}
116117
EOF;
117118

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"symfony/security-core": "^5.4|^6.0|^7.0",
4545
"symfony/security-csrf": "^5.4|^6.0|^7.0",
4646
"symfony/security-http": "^5.4|^6.0|^7.0",
47-
"symfony/serializer": "^6.4.3|^7.0.3",
47+
"symfony/serializer": "^6.4.44|^7.4.17",
4848
"symfony/stopwatch": "^5.4|^6.0|^7.0",
4949
"symfony/console": "^5.4|^6.0|^7.0",
5050
"symfony/expression-language": "^5.4|^6.0|^7.0",

0 commit comments

Comments
 (0)