Chasing memory allocation bugs

This commit is contained in:
Adrian Stephens 2026-07-18 18:04:23 +01:00
parent 8fc530c51b
commit 82bba3df03
6 changed files with 249 additions and 30 deletions

View File

@ -75,7 +75,8 @@ Examples:
- SMTP EHLO uses the domain from the configured From email address.
- Standard message headers and MIME formatting are generated by WordPress/PHPMailer.
- BCC recipients are delivered through PHPMailer recipient handling and are not rendered into the message headers.
- Generated in-memory attachments, including rendered PDFs, are added to PHPMailer as string attachments.
- Generated send-time attachments, including rendered PDFs, are added to PHPMailer as string attachments. Before SMTP send, the run service estimates MIME size and fails with a controlled diagnostic when the message would exceed `FECA_MAILSHOTS_MAX_ESTIMATED_MIME_BYTES` (default 25 MB).
- PDF download workflows stream ZIP and merged PDF output from temporary files instead of returning large generated PDF byte arrays through the application/API layer.
## Important References

BIN
dist/feca_mailshots_plugin-1.1.20.zip vendored Normal file

Binary file not shown.

View File

@ -3,7 +3,7 @@
* Plugin Name: FECA Mailshots
* Plugin URI: https://fenedge.co.uk/
* Description: FECA mailshots plugin.
* Version: 1.1.20
* Version: 1.1.21
* Requires at least: 6.0
* Requires PHP: 7.4
* Author: FECA

View File

@ -188,34 +188,22 @@ final class DownloadPdfAdminPage
return;
}
$bytes = (string) ($result['merged_pdf_bytes'] ?? '');
if ($bytes === '') {
$this->wp->updateOption(self::RESULT_OPTION_KEY, ['ok' => false, 'errors' => ['Merged PDF bytes are empty.']]);
$mergedPath = (string) ($result['merged_pdf_path'] ?? '');
if ($mergedPath === '' || !is_file($mergedPath)) {
$this->wp->updateOption(self::RESULT_OPTION_KEY, ['ok' => false, 'errors' => ['Merged PDF generation failed: output file not found.']]);
$this->redirectTo($this->wp->adminUrl('admin.php?page=' . self::PAGE_SLUG . '&mailshot_id=' . $mailshotId));
return;
}
try {
$this->sendBinaryDownload($downloadBaseName . '_merged.pdf', 'application/pdf', $bytes);
$this->sendFileDownload($downloadBaseName . '_merged.pdf', 'application/pdf', $mergedPath);
} catch (\Throwable $e) {
@unlink($mergedPath);
$this->wp->updateOption(self::RESULT_OPTION_KEY, ['ok' => false, 'errors' => ['Merged PDF download failed: ' . $e->getMessage()]]);
$this->redirectTo($this->wp->adminUrl('admin.php?page=' . self::PAGE_SLUG . '&mailshot_id=' . $mailshotId));
return;
}
}
private function sendBinaryDownload(string $filename, string $contentType, string $bytes): void
{
if (headers_sent()) {
throw new \RuntimeException('Cannot send download: headers already sent.');
}
header('Content-Type: ' . $contentType);
header('Content-Disposition: attachment; filename="' . str_replace('"', '', $filename) . '"');
header('Content-Length: ' . strlen($bytes));
header('X-Content-Type-Options: nosniff');
echo $bytes;
exit;
}
private function sendFileDownload(string $filename, string $contentType, string $path): void
{
if (headers_sent()) {

View File

@ -11,6 +11,9 @@ use FecaMailshots\Repository\MailshotRepository;
final class MailshotRunService
{
private const DEFAULT_MAX_ESTIMATED_MIME_BYTES = 26214400;
private const ESTIMATED_MIME_WARNING_RATIO = 0.75;
private MailshotRepository $mailshots;
private MailshotQueryRepository $queries;
private AttachmentRepository $attachments;
@ -166,6 +169,10 @@ final class MailshotRunService
$this->staticAttachments($mailshot),
$this->renderedPdfAttachments($mailshot, (array) ($render['recipient'] ?? []), (array) ($render['rendered'] ?? []))
);
$messageSize = $this->messageSizeDiagnostics((string) $render['rendered']['subject'], (string) $render['rendered']['message'], $attachments);
if (!$messageSize['ok']) {
return ['ok' => false, 'errors' => $messageSize['errors'], 'message_size' => $messageSize];
}
$attemptId = 'test_' . $mailshotId . '_' . $recipientIndex . '_' . gmdate('YmdHis');
@ -190,7 +197,8 @@ final class MailshotRunService
return [
'ok' => true,
'warnings' => $warnings,
'warnings' => array_merge($warnings, $messageSize['warnings']),
'message_size' => $messageSize,
'sent_to' => $testEmail,
'sent_at' => gmdate('c'),
];
@ -243,6 +251,11 @@ final class MailshotRunService
$this->staticAttachments($mailshot),
$this->renderedPdfAttachments($mailshot, (array) $row, $render)
);
$rowStage = 'checking estimated message size';
$messageSize = $this->messageSizeDiagnostics((string) $render['subject'], (string) $render['message'], $attachments);
if (!$messageSize['ok']) {
throw new \RuntimeException(implode('; ', $messageSize['errors']));
}
$rowStage = 'sending SMTP message';
$send = $this->smtp->send(
$creds,
@ -493,6 +506,7 @@ final class MailshotRunService
}
$files = [];
$filesTmpDir = '';
$ghostscriptBinary = $this->findExecutableBinary('gs');
if ($includeMerged && $ghostscriptBinary === '') {
return ['ok' => false, 'errors' => ['Merged PDF generation requires Ghostscript (gs) to be installed and available on PATH.']];
@ -514,6 +528,13 @@ final class MailshotRunService
return ['ok' => false, 'errors' => ['Unable to prepare temporary directory for merged PDF build.']];
}
}
if ($includeFiles) {
$filesTmpDir = sys_get_temp_dir() . '/feca_mailshots_pdf_files_' . str_replace('.', '_', uniqid('', true));
if (!@mkdir($filesTmpDir, 0700, true) && !is_dir($filesTmpDir)) {
$this->cleanupMergedPartFiles($mergedPdfTmpDir, $mergedPdfPartPaths);
return ['ok' => false, 'errors' => ['Unable to prepare temporary directory for generated PDF files.']];
}
}
foreach (array_values($rows) as $index => $row) {
try {
@ -533,10 +554,16 @@ final class MailshotRunService
$baseName = $this->pdfFilename($mailshot, (array) $row);
$filename = $this->uniqueFilename($baseName, $nameCounts);
$pdfBytes = $this->renderPdfBytesFromHtml($pdfHtml);
$pdfPath = $filesTmpDir . '/' . $filename;
if (@file_put_contents($pdfPath, $pdfBytes) === false) {
throw new \RuntimeException('Unable to write generated PDF file.');
}
$files[] = [
'filename' => $filename,
'content_bytes' => $pdfBytes,
'path' => $pdfPath,
'size' => strlen($pdfBytes),
];
unset($pdfBytes);
}
if ($useGhostscriptMerge) {
$pdfBytesForMerge = $this->renderPdfBytesFromHtml($pdfHtml);
@ -560,10 +587,17 @@ final class MailshotRunService
if ($errors !== []) {
$this->cleanupMergedPartFiles($mergedPdfTmpDir, $mergedPdfPartPaths);
$this->cleanupGeneratedFileRows($files);
if ($filesTmpDir !== '') {
@rmdir($filesTmpDir);
}
return ['ok' => false, 'errors' => $errors];
}
if ($includeFiles && $files === []) {
$this->cleanupMergedPartFiles($mergedPdfTmpDir, $mergedPdfPartPaths);
if ($filesTmpDir !== '') {
@rmdir($filesTmpDir);
}
return ['ok' => false, 'errors' => ['No PDF attachments were generated from this mailshot.']];
}
if ($includeMerged && $mergedSectionCount === 0) {
@ -571,12 +605,19 @@ final class MailshotRunService
return ['ok' => false, 'errors' => ['No merged PDF content was generated from this mailshot.']];
}
$mergedPdfBytes = '';
$mergedPdfPath = '';
$mergedPdfSize = 0;
if ($includeMerged) {
try {
$mergedPdfBytes = $this->mergePdfFilesWithGhostscript($ghostscriptBinary, $mergedPdfPartPaths);
$mergedPdfPath = $this->mergePdfFilesWithGhostscript($ghostscriptBinary, $mergedPdfPartPaths);
$mergedPdfSizeRaw = @filesize($mergedPdfPath);
$mergedPdfSize = is_int($mergedPdfSizeRaw) ? $mergedPdfSizeRaw : 0;
} catch (\Throwable $e) {
$this->cleanupMergedPartFiles($mergedPdfTmpDir, $mergedPdfPartPaths);
$this->cleanupGeneratedFileRows($files);
if ($filesTmpDir !== '') {
@rmdir($filesTmpDir);
}
return ['ok' => false, 'errors' => ['Merged PDF generation failed: ' . $e->getMessage()]];
}
$this->cleanupMergedPartFiles($mergedPdfTmpDir, $mergedPdfPartPaths);
@ -588,7 +629,9 @@ final class MailshotRunService
'generated_count' => $includeFiles ? count($files) : $mergedSectionCount,
'skipped_count' => $skipped,
'files' => $files,
'merged_pdf_bytes' => $mergedPdfBytes,
'files_dir' => $filesTmpDir,
'merged_pdf_path' => $mergedPdfPath,
'merged_pdf_size' => $mergedPdfSize,
];
}
@ -873,6 +916,11 @@ final class MailshotRunService
$this->staticAttachments($mailshot),
$this->renderedPdfAttachments($mailshot, $row, $render)
);
$stage = 'checking estimated message size for recipient ' . $recipientKey;
$messageSize = $this->messageSizeDiagnostics((string) $render['subject'], (string) $render['message'], $attachments);
if (!$messageSize['ok']) {
throw new \RuntimeException(implode('; ', $messageSize['errors']));
}
$stage = 'sending SMTP message for recipient ' . $recipientKey;
$smtp = $this->smtp->send(
$creds,
@ -1139,13 +1187,13 @@ final class MailshotRunService
);
}
$mergedBytes = @file_get_contents($outputPdfPath);
@unlink($outputPdfPath);
if (!is_string($mergedBytes) || $mergedBytes === '') {
$mergedSize = @filesize($outputPdfPath);
if (!is_int($mergedSize) || $mergedSize <= 0) {
@unlink($outputPdfPath);
throw new \RuntimeException('Ghostscript merge produced an empty output file.');
}
return $mergedBytes;
return $outputPdfPath;
}
/** @param array<string,mixed> $mailshot @param array<string,mixed> $row */
@ -1284,6 +1332,78 @@ final class MailshotRunService
return $out;
}
/**
* @param list<array{filename:string,mime_type:string,content_bytes:string}> $attachments
* @return array{ok:bool,estimated_mime_bytes:int,raw_attachment_bytes:int,max_estimated_mime_bytes:int,warnings:list<string>,errors:list<string>}
*/
private function messageSizeDiagnostics(string $subject, string $htmlBody, array $attachments): array
{
$rawAttachmentBytes = 0;
foreach ($attachments as $attachment) {
$rawAttachmentBytes += strlen((string) ($attachment['content_bytes'] ?? ''));
}
$estimatedMimeBytes = strlen($subject) + strlen($htmlBody) + 4096;
foreach ($attachments as $attachment) {
$bytes = strlen((string) ($attachment['content_bytes'] ?? ''));
$estimatedMimeBytes += (int) ceil($bytes * 1.37) + 1024 + strlen((string) ($attachment['filename'] ?? ''));
}
$maxBytes = $this->maxEstimatedMimeBytes();
$warnings = [];
$errors = [];
if ($estimatedMimeBytes > $maxBytes) {
$errors[] = 'Estimated message size is ' . $this->formatBytes($estimatedMimeBytes)
. ', above the configured send limit of ' . $this->formatBytes($maxBytes) . '.';
} elseif ($estimatedMimeBytes >= (int) floor($maxBytes * self::ESTIMATED_MIME_WARNING_RATIO)) {
$warnings[] = 'Estimated message size is ' . $this->formatBytes($estimatedMimeBytes)
. ', close to the configured send limit of ' . $this->formatBytes($maxBytes) . '.';
}
return [
'ok' => $errors === [],
'estimated_mime_bytes' => $estimatedMimeBytes,
'raw_attachment_bytes' => $rawAttachmentBytes,
'max_estimated_mime_bytes' => $maxBytes,
'warnings' => $warnings,
'errors' => $errors,
];
}
private function maxEstimatedMimeBytes(): int
{
$envValue = getenv('FECA_MAILSHOTS_MAX_ESTIMATED_MIME_BYTES');
if (is_string($envValue) && trim($envValue) !== '' && ctype_digit(trim($envValue))) {
$value = (int) trim($envValue);
if ($value > 0) {
return $value;
}
}
return self::DEFAULT_MAX_ESTIMATED_MIME_BYTES;
}
private function formatBytes(int $bytes): string
{
if ($bytes >= 1048576) {
return number_format($bytes / 1048576, 1) . ' MB';
}
if ($bytes >= 1024) {
return number_format($bytes / 1024, 1) . ' KB';
}
return (string) $bytes . ' bytes';
}
/** @param list<array<string,mixed>> $files */
private function cleanupGeneratedFileRows(array $files): void
{
foreach ($files as $file) {
$path = (string) ($file['path'] ?? '');
if ($path !== '') {
@unlink($path);
}
}
}
private function inferMimeTypeFromFilename(string $fileName): string
{
$fileName = trim($fileName);

View File

@ -115,6 +115,8 @@ $queryId = null;
$mailshotId = null;
$attachmentId = null;
$attachmentName = 'phase4_att_' . $uniq;
$generatedPdfPaths = [];
$generatedPdfDirs = [];
try {
$savedQ = $dataSourceService->save(null, $queryName, $dsl);
@ -160,8 +162,63 @@ try {
fwrite(STDERR, "generatePdfBatch should generate at least one PDF\n");
exit(1);
}
if (strpos((string) ($pdfBatch['merged_pdf_bytes'] ?? ''), '%PDF') !== 0) {
fwrite(STDERR, "generatePdfBatch merged_pdf_bytes should be a PDF payload\n");
$mergedPdfPath = (string) ($pdfBatch['merged_pdf_path'] ?? '');
if ($mergedPdfPath === '' || !is_file($mergedPdfPath)) {
fwrite(STDERR, "generatePdfBatch should return a merged PDF temp file path\n");
exit(1);
}
$generatedPdfPaths[] = $mergedPdfPath;
if (file_get_contents($mergedPdfPath, false, null, 0, 4) !== '%PDF') {
fwrite(STDERR, "generatePdfBatch merged_pdf_path should point to a PDF payload\n");
exit(1);
}
if (array_key_exists('merged_pdf_bytes', $pdfBatch)) {
fwrite(STDERR, "generatePdfBatch should not return merged PDF bytes\n");
exit(1);
}
$pdfFiles = is_array($pdfBatch['files'] ?? null) ? $pdfBatch['files'] : [];
if ($pdfFiles === []) {
fwrite(STDERR, "generatePdfBatch should return individual PDF file rows\n");
exit(1);
}
foreach ($pdfFiles as $file) {
$path = (string) ($file['path'] ?? '');
if ($path === '' || !is_file($path)) {
fwrite(STDERR, "generatePdfBatch individual PDF path missing\n");
exit(1);
}
$generatedPdfPaths[] = $path;
if (array_key_exists('content_bytes', $file)) {
fwrite(STDERR, "generatePdfBatch file rows should not return PDF bytes\n");
exit(1);
}
if (file_get_contents($path, false, null, 0, 4) !== '%PDF') {
fwrite(STDERR, "generatePdfBatch individual file should be a PDF payload\n");
exit(1);
}
}
$filesDir = (string) ($pdfBatch['files_dir'] ?? '');
if ($filesDir !== '') {
$generatedPdfDirs[] = $filesDir;
}
$mergedOnly = $run->generatePdfBatch($mailshotId, true, false);
if (($mergedOnly['ok'] ?? false) !== true) {
fwrite(STDERR, 'generatePdfBatch merged-only failed: ' . json_encode($mergedOnly) . "\n");
exit(1);
}
$mergedOnlyPath = (string) ($mergedOnly['merged_pdf_path'] ?? '');
if ($mergedOnlyPath === '' || !is_file($mergedOnlyPath)) {
fwrite(STDERR, "generatePdfBatch merged-only should return a temp file path\n");
exit(1);
}
$generatedPdfPaths[] = $mergedOnlyPath;
if (($mergedOnly['files'] ?? []) !== []) {
fwrite(STDERR, "generatePdfBatch merged-only should not return individual files\n");
exit(1);
}
if (array_key_exists('merged_pdf_bytes', $mergedOnly)) {
fwrite(STDERR, "generatePdfBatch merged-only should not return merged PDF bytes\n");
exit(1);
}
@ -204,6 +261,49 @@ try {
fwrite(STDERR, "sendTest PDF attachment content is not a PDF payload\n");
exit(1);
}
if (!is_array($sendTest['message_size'] ?? null)) {
fwrite(STDERR, "sendTest should return message size diagnostics\n");
exit(1);
}
$oldSizeLimit = getenv('FECA_MAILSHOTS_MAX_ESTIMATED_MIME_BYTES');
try {
putenv('FECA_MAILSHOTS_MAX_ESTIMATED_MIME_BYTES=1024');
$sendTooLarge = $run->sendTest($mailshotId, 0, 'receiver@example.org');
if (($sendTooLarge['ok'] ?? true) !== false) {
fwrite(STDERR, "sendTest should fail when estimated MIME size exceeds configured limit\n");
exit(1);
}
$tooLargeText = implode('; ', (array) ($sendTooLarge['errors'] ?? []));
if (strpos($tooLargeText, 'Estimated message size') === false) {
fwrite(STDERR, "sendTest size-limit error text mismatch: {$tooLargeText}\n");
exit(1);
}
if (!is_array($sendTooLarge['message_size'] ?? null)) {
fwrite(STDERR, "sendTest size-limit failure should include diagnostics\n");
exit(1);
}
$sendAllTooLarge = $run->sendTestAll($mailshotId, 'receiver@example.org');
if (($sendAllTooLarge['ok'] ?? true) !== false) {
fwrite(STDERR, "sendTestAll should fail when estimated MIME size exceeds configured limit\n");
exit(1);
}
if ((int) ($sendAllTooLarge['failed'] ?? 0) <= 0) {
fwrite(STDERR, "sendTestAll size-limit failure should count failed recipients\n");
exit(1);
}
$sendAllTooLargeText = implode('; ', (array) ($sendAllTooLarge['errors'] ?? []));
if (strpos($sendAllTooLargeText, 'Estimated message size') === false) {
fwrite(STDERR, "sendTestAll size-limit error text mismatch: {$sendAllTooLargeText}\n");
exit(1);
}
} finally {
if ($oldSizeLimit === false) {
putenv('FECA_MAILSHOTS_MAX_ESTIMATED_MIME_BYTES');
} else {
putenv('FECA_MAILSHOTS_MAX_ESTIMATED_MIME_BYTES=' . $oldSizeLimit);
}
}
// Regression: configured-but-missing attachment must return a controlled error, not throw/fatal.
$mailshotRepo->update($mailshotId, [
@ -257,6 +357,16 @@ try {
echo "smtp_calls={$smtp->count}\n";
echo "imap_calls={$imap->count}\n";
} finally {
foreach ($generatedPdfPaths as $path) {
if (is_string($path) && $path !== '') {
@unlink($path);
}
}
foreach ($generatedPdfDirs as $dir) {
if (is_string($dir) && $dir !== '') {
@rmdir($dir);
}
}
if ($mailshotId !== null) {
try { $lastRunRepo->clearForMailshot($mailshotId); } catch (Throwable $e) {}
try { $mailshotRepo->delete($mailshotId); } catch (Throwable $e) {}