From 50da980200ea04399f918bdc36ee9173037b83f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 Jan 2026 18:02:59 +0000 Subject: [PATCH 1/5] Initial plan From e8cf080317977a7f84cc4d68e7f3fd470d9f564d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 Jan 2026 18:09:12 +0000 Subject: [PATCH 2/5] Implement If-Modified-Since caching for file downloads Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com> --- organizer/src/class/ThreadStorageManager.php | 57 +++++++++++++++++++ .../src/e2e-tests/pages/FilePageTest.php | 43 ++++++++++++++ .../pages/common/E2EPageTestCase.php | 13 +++-- organizer/src/file.php | 44 +++++++++++++- 4 files changed, 151 insertions(+), 6 deletions(-) diff --git a/organizer/src/class/ThreadStorageManager.php b/organizer/src/class/ThreadStorageManager.php index 53015613..d33430db 100644 --- a/organizer/src/class/ThreadStorageManager.php +++ b/organizer/src/class/ThreadStorageManager.php @@ -46,6 +46,23 @@ public function getThreadEmailContent($thread_id, $email_id) { return stream_get_contents($content); } + + public function getThreadEmailContentWithTimestamp($thread_id, $email_id) { + // Get email content and timestamp from database + $row = Database::queryOne( + "SELECT content, timestamp_received FROM thread_emails WHERE thread_id = ? AND id = ?", + [$thread_id, $email_id] + ); + + if (empty($row)) { + return null; + } + + return [ + 'content' => stream_get_contents($row['content']), + 'timestamp' => $row['timestamp_received'] + ]; + } public function getThreadEmailAttachment(Thread $thread, $attachment_location) { // Get attachment from database $rows = Database::queryOne( @@ -82,4 +99,44 @@ public function getThreadEmailAttachment(Thread $thread, $attachment_location) { return $attachment; } + + public function getThreadEmailAttachmentWithTimestamp(Thread $thread, $attachment_location) { + // Get attachment from database with timestamp + $rows = Database::queryOne( + "SELECT tea.name, tea.filename, tea.filetype, tea.location, tea.status_type, tea.status_text, + tea.content, tea.created_at + FROM thread_email_attachments tea + JOIN thread_emails te ON tea.email_id = te.id + WHERE te.thread_id = ? AND tea.location = ?", + [$thread->id, $attachment_location] + ); + + if (empty($rows)) { + throw new Exception("Thread Email Attachment not found [thread_id=$thread->id, attachment_id=$attachment_location]"); + } + + $attachment = new ThreadEmailAttachment(); + $attachment->name = $rows['name']; + $attachment->filename = $rows['filename']; + $attachment->filetype = $rows['filetype']; + $attachment->location = $rows['location']; + $attachment->status_type = $rows['status_type']; + $attachment->status_text = $rows['status_text']; + + // Handle the encoded bytea data + $content = $rows['content']; + if (is_resource($content)) { + $content = stream_get_contents($content); + } + if (substr($content, 0, 2) === '\\x') { + // Convert hex format to binary + $content = hex2bin(substr($content, 2)); + } + $attachment->content = $content; + + return [ + 'attachment' => $attachment, + 'timestamp' => $rows['created_at'] + ]; + } } diff --git a/organizer/src/e2e-tests/pages/FilePageTest.php b/organizer/src/e2e-tests/pages/FilePageTest.php index f62c153a..9cad05d0 100644 --- a/organizer/src/e2e-tests/pages/FilePageTest.php +++ b/organizer/src/e2e-tests/pages/FilePageTest.php @@ -17,11 +17,54 @@ public function testFileEmailBodyHappy() { // :: Assert content type header for file download $this->assertStringContainsString('Content-Type: text/html; charset=UTF-8', $response->headers); + + // :: Assert Last-Modified header is present + $this->assertStringContainsString('Last-Modified:', $response->headers); + + // :: Assert Cache-Control header is present + $this->assertStringContainsString('Cache-Control:', $response->headers); // Contain "mailHeaders" as part of the output $this->assertStringContainsString('HEADERS', $response->body); $this->assertStringContainsString('From: sender@example.com', $response->body); } + + public function testFileEmailBodyIfModifiedSince() { + $testData = E2ETestSetup::createTestThread(); + $file = [ + 'email_id' => $testData['email_id'], + 'thread_id' => $testData['thread']->id, + 'entity_id' => $testData['entity_id'] + ]; + + // First request - get the Last-Modified header + $response = $this->renderPage('/file?entityId=' . $file['entity_id'] . '&threadId=' . $file['thread_id'] . '&body=' . $file['email_id']); + + // Extract Last-Modified header from response + $lastModified = null; + $lines = explode("\n", $response->headers); + foreach($lines as $line) { + if (stripos($line, 'Last-Modified:') !== false) { + $lastModified = trim(substr($line, strlen('Last-Modified:'))); + break; + } + } + + $this->assertNotNull($lastModified, 'Last-Modified header should be present'); + + // Second request - with If-Modified-Since header set to the same time + $response2 = $this->renderPage( + '/file?entityId=' . $file['entity_id'] . '&threadId=' . $file['thread_id'] . '&body=' . $file['email_id'], + 'dev-user-id', + 'GET', + '304 Not Modified', + null, + ['If-Modified-Since: ' . $lastModified] + ); + + // Body should be empty for 304 response + $this->assertEmpty($response2->body, '304 response should have empty body'); + } /* public function testFilePdfHappy() { // TODO: diff --git a/organizer/src/e2e-tests/pages/common/E2EPageTestCase.php b/organizer/src/e2e-tests/pages/common/E2EPageTestCase.php index 55d7c8f7..534c1bf2 100644 --- a/organizer/src/e2e-tests/pages/common/E2EPageTestCase.php +++ b/organizer/src/e2e-tests/pages/common/E2EPageTestCase.php @@ -7,7 +7,7 @@ class E2EPageTestCase extends TestCase { private static $session_cookies = array(); - protected function renderPage($path, $user = 'dev-user-id', $method = 'GET', $expected_status = '200 OK', $post_data = null) { + protected function renderPage($path, $user = 'dev-user-id', $method = 'GET', $expected_status = '200 OK', $post_data = null, $extra_headers = array()) { $url = 'http://localhost:25081' . $path; if ($user !== null) { if (!isset(self::$session_cookies[$user])) { @@ -15,9 +15,9 @@ protected function renderPage($path, $user = 'dev-user-id', $method = 'GET', $ex self::$session_cookies[$user] = $session_cookie; } $session_cookie = self::$session_cookies[$user]; - $response = $this->curl($url, $method, $session_cookie, post_data: $post_data); + $response = $this->curl($url, $method, $session_cookie, post_data: $post_data, extra_headers: $extra_headers); } else { - $response = $this->curl($url, $method, post_data: $post_data); + $response = $this->curl($url, $method, post_data: $post_data, extra_headers: $extra_headers); } if ($expected_status != null) { @@ -55,7 +55,7 @@ private function authenticate($user) { return $response->cookies[0]; } - private function curl($url, $method = 'GET', $session_cookie = null, $headers = array(), $post_data = null) { + private function curl($url, $method = 'GET', $session_cookie = null, $headers = array(), $post_data = null, $extra_headers = array()) { //echo date('Y-m-d H:i:s') . " - $method $url $session_cookie\n"; $ch = curl_init(); @@ -68,6 +68,11 @@ private function curl($url, $method = 'GET', $session_cookie = null, $headers = $headers[] = 'Cookie: ' . $session_cookie; } $headers[] = 'User-Agent: Offpost E2E Test'; + + // Add any extra headers + foreach ($extra_headers as $header) { + $headers[] = $header; + } if (!empty($headers)) { curl_setopt($ch, CURLOPT_HTTPHEADER, $headers); diff --git a/organizer/src/file.php b/organizer/src/file.php index d6f2950c..2d868929 100644 --- a/organizer/src/file.php +++ b/organizer/src/file.php @@ -44,8 +44,30 @@ foreach ($thread->emails as $email) { if (isset($_GET['body']) && $_GET['body'] == $email->id) { - $eml = ThreadStorageManager::getInstance()->getThreadEmailContent($thread->id, $email->id); + $emailData = ThreadStorageManager::getInstance()->getThreadEmailContentWithTimestamp($thread->id, $email->id); + if (!$emailData) { + http_response_code(404); + header('Content-Type: text/plain'); + die("Email content not found"); + } + + $eml = $emailData['content']; + $lastModified = strtotime($emailData['timestamp']); + + // Set Last-Modified header + header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $lastModified) . ' GMT'); + // Set Cache-Control header + header('Cache-Control: private, max-age=3600'); + + // Check If-Modified-Since header + if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) { + $ifModifiedSince = strtotime($_SERVER['HTTP_IF_MODIFIED_SINCE']); + if ($ifModifiedSince >= $lastModified) { + http_response_code(304); + exit; + } + } header('Content-Type: text/html; charset=UTF-8'); @@ -109,11 +131,29 @@ // New format of location $filename = $att->location; } - $att = ThreadStorageManager::getInstance()->getThreadEmailAttachment($thread, $att->location); + $attachmentData = ThreadStorageManager::getInstance()->getThreadEmailAttachmentWithTimestamp($thread, $att->location); + $att = $attachmentData['attachment']; + $lastModified = strtotime($attachmentData['timestamp']); + if (empty($att->content)) { throw new Exception("Attachment content empty: threadId={$threadId}, attachmentId={$att->attachment_id}", 404); } + // Set Last-Modified header + header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $lastModified) . ' GMT'); + + // Set Cache-Control header + header('Cache-Control: private, max-age=31536000'); // 1 year for attachments + + // Check If-Modified-Since header + if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) { + $ifModifiedSince = strtotime($_SERVER['HTTP_IF_MODIFIED_SINCE']); + if ($ifModifiedSince >= $lastModified) { + http_response_code(304); + exit; + } + } + if ($att->filetype == 'pdf') { header("Content-type:application/pdf"); } From 76ba61684ab1a1dff4b74a3205b07aaa00e5d75f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 24 Jan 2026 23:31:38 +0000 Subject: [PATCH 3/5] Address PR feedback: fix queryOne, move headers, fix attachment_id, add immutable, add attachment test Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com> --- organizer/src/class/ThreadStorageManager.php | 2 +- .../src/e2e-tests/pages/FilePageTest.php | 47 +++++++++++++++++++ organizer/src/file.php | 19 ++++---- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/organizer/src/class/ThreadStorageManager.php b/organizer/src/class/ThreadStorageManager.php index d33430db..603bd5b2 100644 --- a/organizer/src/class/ThreadStorageManager.php +++ b/organizer/src/class/ThreadStorageManager.php @@ -49,7 +49,7 @@ public function getThreadEmailContent($thread_id, $email_id) { public function getThreadEmailContentWithTimestamp($thread_id, $email_id) { // Get email content and timestamp from database - $row = Database::queryOne( + $row = Database::queryOneOrNone( "SELECT content, timestamp_received FROM thread_emails WHERE thread_id = ? AND id = ?", [$thread_id, $email_id] ); diff --git a/organizer/src/e2e-tests/pages/FilePageTest.php b/organizer/src/e2e-tests/pages/FilePageTest.php index 9cad05d0..8d16325f 100644 --- a/organizer/src/e2e-tests/pages/FilePageTest.php +++ b/organizer/src/e2e-tests/pages/FilePageTest.php @@ -65,6 +65,53 @@ public function testFileEmailBodyIfModifiedSince() { // Body should be empty for 304 response $this->assertEmpty($response2->body, '304 response should have empty body'); } + + public function testFileAttachmentIfModifiedSince() { + $testData = E2ETestSetup::createTestThread(); + $file = [ + 'attachment_id' => $testData['attachment_id'], + 'thread_id' => $testData['thread']->id, + 'entity_id' => $testData['entity_id'] + ]; + + // First request - get the Last-Modified header + $response = $this->renderPage('/file?entityId=' . $file['entity_id'] . '&threadId=' . $file['thread_id'] . '&attachmentId=' . $file['attachment_id']); + + // :: Assert content type header for attachment download + $this->assertStringContainsString('Content-Type: application/pdf', $response->headers); + + // :: Assert Last-Modified header is present + $this->assertStringContainsString('Last-Modified:', $response->headers); + + // :: Assert Cache-Control header with immutable directive + $this->assertStringContainsString('Cache-Control:', $response->headers); + $this->assertStringContainsString('immutable', $response->headers); + + // Extract Last-Modified header from response + $lastModified = null; + $lines = explode("\n", $response->headers); + foreach($lines as $line) { + if (stripos($line, 'Last-Modified:') !== false) { + $lastModified = trim(substr($line, strlen('Last-Modified:'))); + break; + } + } + + $this->assertNotNull($lastModified, 'Last-Modified header should be present'); + + // Second request - with If-Modified-Since header set to the same time + $response2 = $this->renderPage( + '/file?entityId=' . $file['entity_id'] . '&threadId=' . $file['thread_id'] . '&attachmentId=' . $file['attachment_id'], + 'dev-user-id', + 'GET', + '304 Not Modified', + null, + ['If-Modified-Since: ' . $lastModified] + ); + + // Body should be empty for 304 response + $this->assertEmpty($response2->body, '304 response should have empty body'); + } /* public function testFilePdfHappy() { // TODO: diff --git a/organizer/src/file.php b/organizer/src/file.php index 2d868929..fe2e5371 100644 --- a/organizer/src/file.php +++ b/organizer/src/file.php @@ -54,16 +54,13 @@ $eml = $emailData['content']; $lastModified = strtotime($emailData['timestamp']); - // Set Last-Modified header - header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $lastModified) . ' GMT'); - - // Set Cache-Control header - header('Cache-Control: private, max-age=3600'); - - // Check If-Modified-Since header + // Check If-Modified-Since header early for 304 response if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) { $ifModifiedSince = strtotime($_SERVER['HTTP_IF_MODIFIED_SINCE']); if ($ifModifiedSince >= $lastModified) { + // Set headers before 304 response + header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $lastModified) . ' GMT'); + header('Cache-Control: private, max-age=3600'); http_response_code(304); exit; } @@ -75,6 +72,10 @@ header("Content-Security-Policy: default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self' data:; frame-src 'none'; object-src 'none'; base-uri 'none'; form-action 'none';"); $email_content = ThreadEmailExtractorEmailBody::extractContentFromEmail($eml); + + // Set caching headers after successful extraction + header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $lastModified) . ' GMT'); + header('Cache-Control: private, max-age=3600'); $subject = Imap\ImapEmail::getEmailSubject($eml); echo '

Subject: ' . htmlescape($subject) . '

' . chr(10); @@ -136,14 +137,14 @@ $lastModified = strtotime($attachmentData['timestamp']); if (empty($att->content)) { - throw new Exception("Attachment content empty: threadId={$threadId}, attachmentId={$att->attachment_id}", 404); + throw new Exception("Attachment content empty: threadId={$threadId}, attachmentId={$att->id}", 404); } // Set Last-Modified header header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $lastModified) . ' GMT'); // Set Cache-Control header - header('Cache-Control: private, max-age=31536000'); // 1 year for attachments + header('Cache-Control: private, max-age=31536000, immutable'); // 1 year for attachments // Check If-Modified-Since header if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) { From 066bf1425f850b3be0f3bf6cebb493fed24bcc8f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 24 Jan 2026 23:33:01 +0000 Subject: [PATCH 4/5] Use queryOneOrNone consistently in getThreadEmailAttachmentWithTimestamp Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com> --- organizer/src/class/ThreadStorageManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/organizer/src/class/ThreadStorageManager.php b/organizer/src/class/ThreadStorageManager.php index 603bd5b2..8df194ba 100644 --- a/organizer/src/class/ThreadStorageManager.php +++ b/organizer/src/class/ThreadStorageManager.php @@ -102,7 +102,7 @@ public function getThreadEmailAttachment(Thread $thread, $attachment_location) { public function getThreadEmailAttachmentWithTimestamp(Thread $thread, $attachment_location) { // Get attachment from database with timestamp - $rows = Database::queryOne( + $rows = Database::queryOneOrNone( "SELECT tea.name, tea.filename, tea.filetype, tea.location, tea.status_type, tea.status_text, tea.content, tea.created_at FROM thread_email_attachments tea From 5f4561f54e4838ba757760475f5e967b66c460fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 25 Jan 2026 08:37:35 +0000 Subject: [PATCH 5/5] Refactor: Extract attachment mapping to private helper method Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com> --- organizer/src/class/ThreadStorageManager.php | 60 ++++++++------------ 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/organizer/src/class/ThreadStorageManager.php b/organizer/src/class/ThreadStorageManager.php index 8df194ba..65bb6061 100644 --- a/organizer/src/class/ThreadStorageManager.php +++ b/organizer/src/class/ThreadStorageManager.php @@ -63,21 +63,12 @@ public function getThreadEmailContentWithTimestamp($thread_id, $email_id) { 'timestamp' => $row['timestamp_received'] ]; } - public function getThreadEmailAttachment(Thread $thread, $attachment_location) { - // Get attachment from database - $rows = Database::queryOne( - "SELECT tea.name, tea.filename, tea.filetype, tea.location, tea.status_type, tea.status_text, - tea.content - FROM thread_email_attachments tea - JOIN thread_emails te ON tea.email_id = te.id - WHERE te.thread_id = ? AND tea.location = ?", - [$thread->id, $attachment_location] - ); - - if (empty($rows)) { - throw new Exception("Thread Email Attachment not found [thread_id=$thread->id, attachment_id=$attachment_location]"); - } - + /** + * Maps database row to ThreadEmailAttachment object + * @param array $rows Database row with attachment data + * @return ThreadEmailAttachment + */ + private function mapRowToAttachment(array $rows) { $attachment = new ThreadEmailAttachment(); $attachment->name = $rows['name']; $attachment->filename = $rows['filename']; @@ -100,6 +91,24 @@ public function getThreadEmailAttachment(Thread $thread, $attachment_location) { return $attachment; } + public function getThreadEmailAttachment(Thread $thread, $attachment_location) { + // Get attachment from database + $rows = Database::queryOne( + "SELECT tea.name, tea.filename, tea.filetype, tea.location, tea.status_type, tea.status_text, + tea.content + FROM thread_email_attachments tea + JOIN thread_emails te ON tea.email_id = te.id + WHERE te.thread_id = ? AND tea.location = ?", + [$thread->id, $attachment_location] + ); + + if (empty($rows)) { + throw new Exception("Thread Email Attachment not found [thread_id=$thread->id, attachment_id=$attachment_location]"); + } + + return $this->mapRowToAttachment($rows); + } + public function getThreadEmailAttachmentWithTimestamp(Thread $thread, $attachment_location) { // Get attachment from database with timestamp $rows = Database::queryOneOrNone( @@ -115,27 +124,8 @@ public function getThreadEmailAttachmentWithTimestamp(Thread $thread, $attachmen throw new Exception("Thread Email Attachment not found [thread_id=$thread->id, attachment_id=$attachment_location]"); } - $attachment = new ThreadEmailAttachment(); - $attachment->name = $rows['name']; - $attachment->filename = $rows['filename']; - $attachment->filetype = $rows['filetype']; - $attachment->location = $rows['location']; - $attachment->status_type = $rows['status_type']; - $attachment->status_text = $rows['status_text']; - - // Handle the encoded bytea data - $content = $rows['content']; - if (is_resource($content)) { - $content = stream_get_contents($content); - } - if (substr($content, 0, 2) === '\\x') { - // Convert hex format to binary - $content = hex2bin(substr($content, 2)); - } - $attachment->content = $content; - return [ - 'attachment' => $attachment, + 'attachment' => $this->mapRowToAttachment($rows), 'timestamp' => $rows['created_at'] ]; }