Skip to content

Commit

Permalink
feature #30957 [Messenger] Remove base64_encode & use addslashes (wea…
Browse files Browse the repository at this point in the history
…verryan)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Messenger] Remove base64_encode & use addslashes

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | already covered by existing issue

In #30814, we base64_encoded messages because some transports (specifically DoctrineTransport + Postgresql & SQS) do not allow binary data.

The downside is that the messages become unreadable, which makes it much less convenient to debug your messages with 3rd party monitoring tools, for example.

This PR replaces base64_encode with addslashes. Another alternative (that I first tried in this PR) was to use a blob type, which Drupal does in its code (https://www.drupal.org/project/drupal/issues/690746). But, it still meant that binary data could cause problems with other transports, like SQS.

I also put all the serializer config under a nice, neat `serializer` key under messenger.

Best seen with `?w=1`.

Cheers!

Commits
-------

70b448d Reorganizing messenger serializer config and replacing base64_encode with addslashes
  • Loading branch information
fabpot committed Apr 15, 2019
2 parents 9aedfeb + 70b448d commit e683dfa
Show file tree
Hide file tree
Showing 17 changed files with 73 additions and 54 deletions.
Expand Up @@ -1143,20 +1143,25 @@ function ($a) {
->end()
->end()
->end()
->scalarNode('default_serializer')
->defaultValue('messenger.transport.native_php_serializer')
->info('Service id to use as the default serializer for the transports.')
->end()
->arrayNode('symfony_serializer')
->arrayNode('serializer')
->addDefaultsIfNotSet()
->children()
->scalarNode('format')->defaultValue('json')->info('Serialization format for the messenger.transport.symfony_serializer service (which is not the serializer used by default).')->end()
->arrayNode('context')
->normalizeKeys(false)
->useAttributeAsKey('name')
->defaultValue([])
->info('Context array for the messenger.transport.symfony_serializer service (which is not the serializer used by default).')
->prototype('variable')->end()
->scalarNode('default_serializer')
->defaultValue('messenger.transport.native_php_serializer')
->info('Service id to use as the default serializer for the transports.')
->end()
->arrayNode('symfony_serializer')
->addDefaultsIfNotSet()
->children()
->scalarNode('format')->defaultValue('json')->info('Serialization format for the messenger.transport.symfony_serializer service (which is not the serializer used by default).')->end()
->arrayNode('context')
->normalizeKeys(false)
->useAttributeAsKey('name')
->defaultValue([])
->info('Context array for the messenger.transport.symfony_serializer service (which is not the serializer used by default).')
->prototype('variable')->end()
->end()
->end()
->end()
->end()
->end()
Expand Down
Expand Up @@ -1692,9 +1692,9 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
$container->removeDefinition('messenger.transport.amqp.factory');
} else {
$container->getDefinition('messenger.transport.symfony_serializer')
->replaceArgument(1, $config['symfony_serializer']['format'])
->replaceArgument(2, $config['symfony_serializer']['context']);
$container->setAlias('messenger.default_serializer', $config['default_serializer']);
->replaceArgument(1, $config['serializer']['symfony_serializer']['format'])
->replaceArgument(2, $config['serializer']['symfony_serializer']['context']);
$container->setAlias('messenger.default_serializer', $config['serializer']['default_serializer']);
}

$senderAliases = [];
Expand Down
Expand Up @@ -407,17 +407,21 @@

<xsd:complexType name="messenger">
<xsd:sequence>
<xsd:element name="default-serializer" type="xsd:string" minOccurs="0" />
<xsd:element name="symfony-serializer" type="messenger_symfony_serializer" minOccurs="0" />
<xsd:element name="encoder" type="xsd:string" minOccurs="0" />
<xsd:element name="decoder" type="xsd:string" minOccurs="0" />
<xsd:element name="serializer" type="messenger_serializer" minOccurs="0" />
<xsd:element name="routing" type="messenger_routing" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="transport" type="messenger_transport" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="bus" type="messenger_bus" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
<xsd:attribute name="default-bus" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="messenger_serializer">
<xsd:sequence>
<xsd:element name="symfony-serializer" type="messenger_symfony_serializer" minOccurs="0" />
</xsd:sequence>
<xsd:attribute name="default-serializer" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="messenger_symfony_serializer">
<xsd:sequence>
<xsd:element name="context" type="metadata" minOccurs="0" maxOccurs="unbounded" />
Expand Down
Expand Up @@ -328,10 +328,12 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor
'enabled' => !class_exists(FullStack::class) && interface_exists(MessageBusInterface::class),
'routing' => [],
'transports' => [],
'default_serializer' => 'messenger.transport.native_php_serializer',
'symfony_serializer' => [
'format' => 'json',
'context' => [],
'serializer' => [
'default_serializer' => 'messenger.transport.native_php_serializer',
'symfony_serializer' => [
'format' => 'json',
'context' => [],
],
],
'default_bus' => null,
'buses' => ['messenger.bus.default' => ['default_middleware' => true, 'middleware' => []]],
Expand Down
Expand Up @@ -5,7 +5,6 @@

$container->loadFromExtension('framework', [
'messenger' => [
'default_serializer' => false,
'routing' => [
FooMessage::class => ['sender.bar', 'sender.biz'],
BarMessage::class => 'sender.foo',
Expand Down
Expand Up @@ -3,7 +3,9 @@
$container->loadFromExtension('framework', [
'serializer' => true,
'messenger' => [
'default_serializer' => 'messenger.transport.symfony_serializer',
'serializer' => [
'default_serializer' => 'messenger.transport.symfony_serializer',
],
'routing' => [
'Symfony\Component\Messenger\Tests\Fixtures\DummyMessage' => ['amqp', 'audit'],
'Symfony\Component\Messenger\Tests\Fixtures\SecondMessage' => [
Expand Down
Expand Up @@ -3,10 +3,12 @@
$container->loadFromExtension('framework', [
'serializer' => true,
'messenger' => [
'default_serializer' => 'messenger.transport.symfony_serializer',
'symfony_serializer' => [
'format' => 'csv',
'context' => ['enable_max_depth' => true],
'serializer' => [
'default_serializer' => 'messenger.transport.symfony_serializer',
'symfony_serializer' => [
'format' => 'csv',
'context' => ['enable_max_depth' => true],
],
],
'transports' => [
'default' => 'amqp://localhost/%2f/messages',
Expand Down
Expand Up @@ -3,7 +3,9 @@
$container->loadFromExtension('framework', [
'serializer' => true,
'messenger' => [
'default_serializer' => 'messenger.transport.symfony_serializer',
'serializer' => [
'default_serializer' => 'messenger.transport.symfony_serializer',
],
'transports' => [
'default' => 'amqp://localhost/%2f/messages',
'customised' => [
Expand Down
Expand Up @@ -8,7 +8,7 @@
<framework:config>
<framework:serializer enabled="true" />
<framework:messenger>
<framework:default-serializer>messenger.transport.symfony_serializer</framework:default-serializer>
<framework:serializer default-serializer="messenger.transport.symfony_serializer" />
<framework:routing message-class="Symfony\Component\Messenger\Tests\Fixtures\DummyMessage">
<framework:sender service="amqp" />
<framework:sender service="audit" />
Expand Down
Expand Up @@ -8,12 +8,13 @@
<framework:config>
<framework:serializer enabled="true" />
<framework:messenger>
<framework:default-serializer>messenger.transport.symfony_serializer</framework:default-serializer>
<framework:symfony-serializer format="csv">
<framework:context>
<framework:enable_max_depth>true</framework:enable_max_depth>
</framework:context>
</framework:symfony-serializer>
<framework:serializer default-serializer="messenger.transport.symfony_serializer">
<framework:symfony-serializer format="csv">
<framework:context>
<framework:enable_max_depth>true</framework:enable_max_depth>
</framework:context>
</framework:symfony-serializer>
</framework:serializer>
<framework:transport name="default" dsn="amqp://localhost/%2f/messages" />
</framework:messenger>
</framework:config>
Expand Down
Expand Up @@ -8,7 +8,7 @@
<framework:config>
<framework:serializer enabled="true" />
<framework:messenger>
<framework:default-serializer>messenger.transport.symfony_serializer</framework:default-serializer>
<framework:serializer default-serializer="messenger.transport.symfony_serializer" />
<framework:transport name="default" dsn="amqp://localhost/%2f/messages" />
<framework:transport name="customised" dsn="amqp://localhost/%2f/messages?exchange_name=exchange_name" serializer="messenger.transport.native_php_serializer">
<framework:options>
Expand Down
@@ -1,6 +1,5 @@
framework:
messenger:
default_serializer: false
routing:
'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\FooMessage': ['sender.bar', 'sender.biz']
'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\BarMessage': 'sender.foo'
@@ -1,7 +1,8 @@
framework:
serializer: true
messenger:
default_serializer: messenger.transport.symfony_serializer
serializer:
default_serializer: messenger.transport.symfony_serializer
routing:
'Symfony\Component\Messenger\Tests\Fixtures\DummyMessage': [amqp, audit]
'Symfony\Component\Messenger\Tests\Fixtures\SecondMessage':
Expand Down
@@ -1,10 +1,11 @@
framework:
serializer: true
messenger:
default_serializer: messenger.transport.symfony_serializer
symfony_serializer:
format: csv
context:
enable_max_depth: true
serializer:
default_serializer: messenger.transport.symfony_serializer
symfony_serializer:
format: csv
context:
enable_max_depth: true
transports:
default: 'amqp://localhost/%2f/messages'
@@ -1,7 +1,8 @@
framework:
serializer: true
messenger:
default_serializer: messenger.transport.symfony_serializer
serializer:
default_serializer: messenger.transport.symfony_serializer
transports:
default: 'amqp://localhost/%2f/messages'
customised:
Expand Down
Expand Up @@ -25,7 +25,9 @@ public function testEncodedIsDecodable()

$envelope = new Envelope(new DummyMessage('Hello'));

$this->assertEquals($envelope, $serializer->decode($serializer->encode($envelope)));
$encoded = $serializer->encode($envelope);
$this->assertNotContains("\0", $encoded['body'], 'Does not contain the binary characters');
$this->assertEquals($envelope, $serializer->decode($encoded));
}

public function testDecodingFailsWithMissingBodyKey()
Expand Down Expand Up @@ -58,7 +60,7 @@ public function testDecodingFailsWithBadClass()
$serializer = new PhpSerializer();

$serializer->decode([
'body' => base64_encode('O:13:"ReceivedSt0mp":0:{}'),
'body' => 'O:13:"ReceivedSt0mp":0:{}',
]);
}
}
Expand Up @@ -30,11 +30,7 @@ public function decode(array $encodedEnvelope): Envelope
throw new MessageDecodingFailedException('Encoded envelope should have at least a "body".');
}

$serializeEnvelope = base64_decode($encodedEnvelope['body']);

if (false === $serializeEnvelope) {
throw new MessageDecodingFailedException('The "body" key could not be base64 decoded.');
}
$serializeEnvelope = stripslashes($encodedEnvelope['body']);

return $this->safelyUnserialize($serializeEnvelope);
}
Expand All @@ -44,8 +40,10 @@ public function decode(array $encodedEnvelope): Envelope
*/
public function encode(Envelope $envelope): array
{
$body = addslashes(serialize($envelope));

return [
'body' => base64_encode(serialize($envelope)),
'body' => $body,
];
}

Expand Down

0 comments on commit e683dfa

Please sign in to comment.