I didn't get the change to review the code in a PR propertly, so I'll just write down what I noticed in this issue.
|
$payload = unserialize($message->body); |
Unserialise is not the safest function
https://www.php.net/manual/en/function.unserialize.php
We could either replace it with json or (probably easier) only serialize/deserialize classes that implement certain interface so it's at least somewhat protected. Though I do realise mostly it's not going to be a problem because rabbits port is not going to be exposed outside the container network. Just something to think about.
SafeSalesforceSaverServer and AsyncSfSaveConsumer uses the same code basically, we could figure out a way not to duplicate logic I guess?
MIssing parent __construct in exceptions.
Apart from these looks decent 👍 :)
I didn't get the change to review the code in a PR propertly, so I'll just write down what I noticed in this issue.
safe-salesforce-saver-bundle/src/Comsave/SafeSalesforceSaverBundle/Consumers/SafeSalesforceSaverServer.php
Line 46 in 9cbccbc
Unserialise is not the safest function https://www.php.net/manual/en/function.unserialize.php
We could either replace it with json or (probably easier) only serialize/deserialize classes that implement certain interface so it's at least somewhat protected. Though I do realise mostly it's not going to be a problem because rabbits port is not going to be exposed outside the container network. Just something to think about.
SafeSalesforceSaverServerandAsyncSfSaveConsumeruses the same code basically, we could figure out a way not to duplicate logic I guess?MIssing parent __construct in exceptions.
Apart from these looks decent 👍 :)