diff --git a/README.md b/README.md index a1b3a7c..26fcf82 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/dist/feca_mailshots_plugin-1.1.20.zip b/dist/feca_mailshots_plugin-1.1.20.zip new file mode 100644 index 0000000..ac54d40 Binary files /dev/null and b/dist/feca_mailshots_plugin-1.1.20.zip differ diff --git a/feca_mailshots_plugin/feca_mailshots_plugin.php b/feca_mailshots_plugin/feca_mailshots_plugin.php index 7b651df..67917d1 100644 --- a/feca_mailshots_plugin/feca_mailshots_plugin.php +++ b/feca_mailshots_plugin/feca_mailshots_plugin.php @@ -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 diff --git a/feca_mailshots_plugin/src/Admin/DownloadPdfAdminPage.php b/feca_mailshots_plugin/src/Admin/DownloadPdfAdminPage.php index bd4ddb9..ae3bf41 100644 --- a/feca_mailshots_plugin/src/Admin/DownloadPdfAdminPage.php +++ b/feca_mailshots_plugin/src/Admin/DownloadPdfAdminPage.php @@ -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()) { diff --git a/feca_mailshots_plugin/src/Application/MailshotRunService.php b/feca_mailshots_plugin/src/Application/MailshotRunService.php index f833e4b..ce388ba 100644 --- a/feca_mailshots_plugin/src/Application/MailshotRunService.php +++ b/feca_mailshots_plugin/src/Application/MailshotRunService.php @@ -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 $mailshot @param array $row */ @@ -1284,6 +1332,78 @@ final class MailshotRunService return $out; } + /** + * @param list $attachments + * @return array{ok:bool,estimated_mime_bytes:int,raw_attachment_bytes:int,max_estimated_mime_bytes:int,warnings:list,errors:list} + */ + 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> $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); diff --git a/tests/integration/test_phase4_run_flow.php b/tests/integration/test_phase4_run_flow.php index 8e3422c..32e3bfa 100644 --- a/tests/integration/test_phase4_run_flow.php +++ b/tests/integration/test_phase4_run_flow.php @@ -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) {}