Skip to content

Commit

Permalink
Fix parsing of attachment's filename in MailgunMailer
Browse files Browse the repository at this point in the history
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 <dulacka@gmail.com>
  • Loading branch information
markoph and rootpd committed Oct 3, 2024
1 parent c3e1285 commit d336748
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

namespace Remp\MailerModule\Models\Mailer;

trait MailHeaderTrait
{
/**
* Get parameter from header string
*
* <code>
* $headerValue = 'Content-Disposition: attachment; filename="invoice-2024-09-24.pdf"';
* // $filename will contain string "invoice-2024-09-24.pdf"
* $filename = $this->getHeaderParameter($headerValue, 'filename');
* </code>
*/
public function getHeaderParameter(string $headerValue, string $parameter): ?string
{
preg_match('/.*' . $parameter . '="(?<value>([^"]*))"/', $headerValue, $result);
return $result['value'] ?? null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

class MailgunMailer extends Mailer
{
use MailHeaderTrait;

public const ALIAS = 'remp_mailgun';

private ?LoggerInterface $logger = null;
Expand Down Expand Up @@ -117,10 +119,11 @@ public function send(Message $mail): void

$attachments = [];
foreach ($mail->getAttachments() as $attachment) {
preg_match('/(?<filename>\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,
];
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php
declare(strict_types=1);

namespace Feature\Mails;

use Nette\Mail\Message;
use PHPUnit\Framework\Attributes\DataProvider;
use Remp\MailerModule\Models\Mailer\MailHeaderTrait;
use Tests\Feature\BaseFeatureTestCase;

class MailHeaderTraitTest extends BaseFeatureTestCase
{
use MailHeaderTrait;

public static function dataProvider()
{
return [
'TestRegularHeader_ShouldReturnFilename' => [
'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);
}
}

0 comments on commit d336748

Please sign in to comment.