search

squigg / azure-queue-laravel

PHP Laravel Queue Driver package for Microsoft Azure Storage Queues
43 stars 24 forks source link

Added base64 encription to messages #13

Closed FixingTheBug closed 4 years ago

FixingTheBug commented 4 years ago

This PR fixes the problem with Azure trying to read messages in the queue that are not base64 encoded.

The error is: The input is not a balid Base-64 string as it contains a non-base 64 character, more than two padding characters, or an illegal character among the padding characters.

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-1.7%) to 98.276% when pulling 6755010226e9e3c317af26529a99a1bceabd9d46 on FixingTheBug:features/fix_base64_encode_when_push_message into c70c57ba756577391a217ce334a65157d2de3b90 on squigg:master.

squigg commented 4 years ago

Hi, thanks for the PR. A few comments:

  1. We will need to add tests that cover both code paths to validate the encode/decode functionality and that it correctly acts upon the configuration option.
  2. This being a configurable option is great, however this would be better set as part of the Laravel configuration that is passed into the AzureConnector and then set as a property on the AzureQueue where we can use it in this method.
  3. It looks like the message is never base64 decoded when the message is retrieved. I'm not sure if this something Azure will automatically do as I have not investigated it, but this would need to be checked.

If you want to tackle any of the above then let me know, otherwise I'll get around to it hopefully sometime soon.

Thanks!