[Messenger] Fix transporting non-UTF8 payloads by encoding them using base 64 #39970
Conversation
1b97648 to
6fc9e51
Compare
When you wrote this, did you mean it will increase the size of our messages in the storage, or did you refer to emails that are later sent out? From the code it looks like the former, but your comment almost implies the latter. |
yes, that's what I meant. |
| @@ -29,6 +29,10 @@ public function decode(array $encodedEnvelope): Envelope | ||
| throw new MessageDecodingFailedException('Encoded envelope should have at least a "body".'); | ||
| } | ||
|
|
||
| if (false === strpos($encodedEnvelope['body'], '}', -1)) { |
There was a problem hiding this comment.
Isn't this check a bit errorprone, what about wrapping the base64 encoded content or prefix it with a special key which we can check against later when retrieving the message?
There was a problem hiding this comment.
I don't think it's error-prone: the php serialization format is likely more stable than this very class.
|
Thank you @nicolas-grekas. |
|
Nice change @nicolas-grekas ! |
Replaces #33920
When using the Doctrine transport, sending emails with binary attachments currently requires a custom Messenger serializer because the "body" column is created for UTF-8 only.
In #33920, it is proposed to change the TEXT type to a BLOB. It leaves at least one problem unhandled: the conversion of existing messenger tables.
This PR takes a more conservative approach, by encoding messages to base 64, only if they are non-UTF8.
Compatibility with the existing format is preserved.
The drawback of this approach is that the size of eg email attachments is going to increase by 33% because of the extra encoding. I think this drawback is acceptable for 4.4, and that this PR is the most pragmatic way to make attachments just work.