From d33674846c2647e2944cd9cff3b53ed4b26cee0e Mon Sep 17 00:00:00 2001 From: Dominik Skerhak Date: Tue, 24 Sep 2024 12:39:38 +0200 Subject: [PATCH] Fix parsing of attachment's filename in MailgunMailer Filename is parsed from header `Content-Disposition`. It is put there by Nette Mailer. Previous implementation incorrectly parsed filenames with dash. Filename of attached file "invoice-2024-09-24.pdf" would be only last part "24.pdf". Regex responsible for parsing header was moved into trait MailHeaderTrait so it can be tested. remp/remp#1386 Co-authored-by: Peter Dulacka --- CHANGELOG.md | 3 + .../src/Models/Mailer/MailHeaderTrait.php | 22 ++++++ .../src/Models/Mailer/MailgunMailer.php | 7 +- .../Feature/Mails/MailHeaderTraitTest.php | 73 +++++++++++++++++++ 4 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 Mailer/extensions/mailer-module/src/Models/Mailer/MailHeaderTrait.php create mode 100644 Mailer/extensions/mailer-module/src/Tests/Feature/Mails/MailHeaderTraitTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 508e05bc3..ea0aed61b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ### [Mailer] - Added ability to set custom health check TTL for `ProcessJobCommand`. remp/remp#1376 +- Fixed parsing of attachment's filename from header within `MailgunMailer`. remp/remp#1386 + - Previous implementation incorrectly parsed filenames with dash. Filename of attached file "invoice-2024-09-24.pdf" would be only last part "24.pdf". + - Added `MailHeaderTrait` with method `getHeaderParameter()` and tests to validate it. ## Archive diff --git a/Mailer/extensions/mailer-module/src/Models/Mailer/MailHeaderTrait.php b/Mailer/extensions/mailer-module/src/Models/Mailer/MailHeaderTrait.php new file mode 100644 index 000000000..6d09670a3 --- /dev/null +++ b/Mailer/extensions/mailer-module/src/Models/Mailer/MailHeaderTrait.php @@ -0,0 +1,22 @@ + + * $headerValue = 'Content-Disposition: attachment; filename="invoice-2024-09-24.pdf"'; + + * // $filename will contain string "invoice-2024-09-24.pdf" + * $filename = $this->getHeaderParameter($headerValue, 'filename'); + * + */ + public function getHeaderParameter(string $headerValue, string $parameter): ?string + { + preg_match('/.*' . $parameter . '="(?([^"]*))"/', $headerValue, $result); + return $result['value'] ?? null; + } +} diff --git a/Mailer/extensions/mailer-module/src/Models/Mailer/MailgunMailer.php b/Mailer/extensions/mailer-module/src/Models/Mailer/MailgunMailer.php index e90c67d12..19a94f8ab 100644 --- a/Mailer/extensions/mailer-module/src/Models/Mailer/MailgunMailer.php +++ b/Mailer/extensions/mailer-module/src/Models/Mailer/MailgunMailer.php @@ -16,6 +16,8 @@ class MailgunMailer extends Mailer { + use MailHeaderTrait; + public const ALIAS = 'remp_mailgun'; private ?LoggerInterface $logger = null; @@ -117,10 +119,11 @@ public function send(Message $mail): void $attachments = []; foreach ($mail->getAttachments() as $attachment) { - preg_match('/(?\w+\.\w+)/', $attachment->getHeader('Content-Disposition'), $attachmentName); + // example header with attachment: `Content-Disposition: attachment; filename="invoice-2024-09-24.pdf"` + $filename = $this->getHeaderParameter($attachment->getHeader('Content-Disposition'), 'filename'); $attachments[] = [ 'fileContent' => $attachment->getBody(), - 'filename' => $attachmentName['filename'], + 'filename' => $filename, ]; } diff --git a/Mailer/extensions/mailer-module/src/Tests/Feature/Mails/MailHeaderTraitTest.php b/Mailer/extensions/mailer-module/src/Tests/Feature/Mails/MailHeaderTraitTest.php new file mode 100644 index 000000000..a8b6e0373 --- /dev/null +++ b/Mailer/extensions/mailer-module/src/Tests/Feature/Mails/MailHeaderTraitTest.php @@ -0,0 +1,73 @@ + [ + 'headerValue' => 'Content-Disposition: attachment; filename="invoice-2024-09-24.pdf"', + 'parameter' => 'filename', + 'result' => 'invoice-2024-09-24.pdf', + ], + 'TestRegularHeader_NotRealParameter' => [ + 'headerValue' => 'Content-Disposition: attachment; filename="invoice-2024-09-24.pdf"', + 'parameter' => 'dummy', + 'result' => null, + ], + 'TestRegularHeader_AdditionalParameters' => [ + 'headerValue' => 'Content-Disposition: attachment; filename="invoice-2024-09-24.pdf"; foo="bar"', + 'parameter' => 'filename', + 'result' => 'invoice-2024-09-24.pdf', + ], + ]; + } + + #[DataProvider('dataProvider')] + public function testMailHeaderTrait( + string $headerValue, + string $parameter, + ?string $result, + ) { + $parameter = $this->getHeaderParameter($headerValue, $parameter); + $this->assertSame($result, $parameter); + } + + /** + * Nette Mailer adds attachments to email as `Content-Disposition` headers with format: + * + * Content-Disposition: attachment; filename="invoice-2024-09-24.pdf" + * + * Mailgun requires attachments as API parameters, so in MailgunMailer we need to load attachment + * and parse filename from header. This is handled by preg_match within `MailHeaderTrait`. + * + * This is simple unit test which checks: + * - If regex which loads attachment's filename from mail header is correct. + * - If Nette didn't change how attachments (filenames) are attached to message. + * E.g. using different approach or attaching filename without quotes (allowed by specification). + */ + public function testMessageMimePartHeaderForContentDisposition() + { + $filename = 'invoice-2024-09-24.pdf'; + + // create attachment as Nette Mailer creates it + $message = new Message(); + $message->addAttachment($filename, 'dummy content', 'text/plain'); + $attachments = $message->getAttachments(); + $attachment = reset($attachments); + + // parse header created by Nette with our Trait + $attachmentName = $this->getHeaderParameter($attachment->getHeader('Content-Disposition'), 'filename'); + $this->assertEquals($filename, $attachmentName); + } +}