Test message rendering

.ci/compose.yaml

@@ -1,6 +1,10 @@
 services:
   mailhost:
     image: docker.io/greenmail/standalone
+    environment:
+      GREENMAIL_OPTS: '-Dgreenmail.setup.test.all -Dgreenmail.hostname=0.0.0.0 -Dgreenmail.auth.disabled -Dgreenmail.preload.dir=/emails'
+    volumes:
+      - "../tests/MessageRendering/data/greenmail:/emails"
   browserhost:
     image: docker.io/selenium/standalone-chromium
     ports:
@@ -40,6 +44,17 @@ services:
     command:
       - .ci/run_tests.sh
 
+  test_message_rendering:
+    depends_on:
+      - mailhost
+    image: ghcr.io/roundcube/roundcubemail-testrunner:php8.3
+    environment:
+      RC_CONFIG_IMAP_HOST: 'tls://mailhost:3143'
+    volumes:
+      - '..:/app'
+    command:
+      - .ci/run_test_message_rendering.sh
+
   codespell:
     image: ghcr.io/roundcube/roundcubemail-testrunner:php8.3
     volumes:

.ci/run_test_message_rendering.sh

@@ -0,0 +1,11 @@
+#!/bin/bash -ex
+
+if ! test -f config/config-test.inc.php; then
+	cp -v .ci/config-test.inc.php config/config-test.inc.php
+fi
+
+# Install dependencies, prefer highest.
+composer update --prefer-dist --no-interaction --no-progress
+
+# Execute tests.
+vendor/bin/phpunit -c ./tests/MessageRendering/phpunit.xml --fail-on-warning --fail-on-risky

.github/workflows/message_rendering.yml

@@ -0,0 +1,37 @@
+name: Message Rendering
+
+on:
+  push:
+  pull_request:
+
+permissions:
+  contents: read
+
+jobs:
+  message_rendering:
+    runs-on: ubuntu-latest
+    if: "!contains(github.event.head_commit.message, '[skip ci]') && !contains(github.event.head_commit.message, '[ci skip]')"
+
+    strategy:
+      fail-fast: false
+
+    name: Linux / PHP 8.3
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      # Run via docker compose because we can't run greenmail in a server here
+      # (it requires the testing emails to be present when starting but
+      # services are started before the repo is cloned). And instead of
+      # re-building what our compose-file contains we can just use it.
+      - name: Run tests via docker compose
+        run: docker compose -f .ci/compose.yaml run test_message_rendering
+
+      - name: Upload artifacts
+        uses: actions/upload-artifact@master
+        if: failure()
+        with:
+          name: Logs
+          path: logs/errors.log
+

.gitignore

@@ -34,3 +34,6 @@ skins/elastic/styles/_variables.less
 .vscode
 .DS_Store
 .idea
+
+# Ignore files used for local overriding.
+/tests/MessageRendering/.env

program/actions/mail/show.php

@@ -172,7 +172,7 @@ public function run($args = [])
             }
         }
 
-        exit;
+        $rcmail->output->sendExit();
     }
 
     /**

tests/MessageRendering/.gitignore

@@ -0,0 +1,11 @@
+src/maildir/subscriptions
+src/maildir/dovecot-uidvalidity
+src/maildir/dovecot.*.log
+src/maildir/dovecot.*.cache
+src/maildir/dovecot.list.index
+src/maildir/dovecot-uid*
+src/maildir/.Drafts
+src/maildir/.Junk
+src/maildir/.Sent
+src/maildir/.Trash
+.phpunit.result.cache

tests/MessageRendering/BasicMessagesTest.php

@@ -0,0 +1,69 @@
+<?php
+
+namespace Tests\MessageRendering;
+
+/**
+ * Test class to test simple messages.
+ */
+class BasicMessagesTest extends MessageRenderingTestCase
+{
+    /**
+     * Test that two text mime-parts with disposition "attachment" are shown as
+     * attachments.
+     */
+    public function testList00()
+    {
+        $domxpath = $this->runAndGetHtmlOutputDomxpath('99839b8ec12482419372f1edafa9de75@woodcrest.local');
+        $this->assertSame('Lines', $this->getScrubbedSubject($domxpath));
+
+        $this->assertStringStartsWith('Plain text message body.', $this->getBody($domxpath));
+
+        $attchElems = $domxpath->query('//span[@class="attachment-name"]');
+        $this->assertCount(2, $attchElems, 'Attachments');
+        $this->assertStringStartsWith('lines.txt', $attchElems[0]->textContent);
+        $this->assertStringStartsWith('lines_lf.txt', $attchElems[1]->textContent);
+    }
+
+    /**
+     * Test that one inline image is not shown as attachment.
+     */
+    public function testList01()
+    {
+        $domxpath = $this->runAndGetHtmlOutputDomxpath('3ef8a0120cd7dc2fd776468c8515e29a@domain.tld');
+
+        $this->assertSame('Test HTML with local and remote image', $this->getScrubbedSubject($domxpath));
+
+        $this->assertSame("Attached image: \nRemote image:", $this->getBody($domxpath));
+
+        $attchNames = $domxpath->query('//span[@class="attachment-name"]');
+        $this->assertCount(0, $attchNames, 'Attachments');
+    }
+
+    /**
+     * Test that text parts are shown and also listed as attachments, and that
+     * filenames are properly listed.
+     */
+    public function testFilename()
+    {
+        $domxpath = $this->runAndGetHtmlOutputDomxpath('de75@tester.local');
+
+        $this->assertSame('Attachment filename encoding', $this->getScrubbedSubject($domxpath));
+
+        $msgParts = $domxpath->query('//div[@class="message-part"]');
+        $this->assertCount(3, $msgParts, 'Message text parts');
+
+        $this->assertSame("foo\nbar\ngna", $msgParts[0]->textContent);
+        $this->assertSame('潦੯慢ੲ湧', $msgParts[1]->textContent);
+        $this->assertSame("foo\nbar\ngna", $msgParts[2]->textContent);
+
+        $attchNames = $domxpath->query('//span[@class="attachment-name"]');
+        $this->assertCount(6, $attchNames, 'Attachments');
+
+        $this->assertSame('A011.txt', $attchNames[0]->textContent);
+        $this->assertSame('A012.txt', $attchNames[1]->textContent);
+        $this->assertSame('A014.txt', $attchNames[2]->textContent);
+        $this->assertSame('żółć.png', $attchNames[3]->textContent);
+        $this->assertSame('żółć.png', $attchNames[4]->textContent);
+        $this->assertSame('very very very very long very very very very long ćććććć very very very long name.txt', $attchNames[5]->textContent);
+    }
+}

tests/MessageRendering/MessageRenderingTestCase.php

@@ -0,0 +1,95 @@
+<?php
+
+namespace Tests\MessageRendering;
+
+use Masterminds\HTML5;
+use Roundcube\Tests\ActionTestCase;
+use Roundcube\Tests\ExitException;
+
+/**
+ * Class to base actual test classes on, which test specific message rendering.
+ */
+class MessageRenderingTestCase extends ActionTestCase
+{
+    /**
+     * Get the body from the document, trimmed from surrounding whitespace.
+     */
+    protected function getBody(\DOMXPath $domxpath): string
+    {
+        $bodyElem = $domxpath->query('//div[@id="messagebody"]');
+        $this->assertCount(1, $bodyElem, 'Message body');
+        return trim($bodyElem[0]->textContent);
+    }
+
+    /**
+     * Get the subject from the document, stripped by the prefix "Subject: ",
+     * the suffix "Open in new window", and trimmed from surrounding whitespace.
+     */
+    protected function getScrubbedSubject(\DOMXPath $domxpath): string
+    {
+        $subjectElem = $domxpath->query('//h2[@class="subject"][1]');
+        $subject = preg_replace('/^\s*Subject:\s*(.*)\s*Open in new window$/', '$1', trim($subjectElem[0]->textContent));
+        return trim($subject);
+    }
+
+    /**
+     * Execute run() to render the message with the given $msgId.
+     *
+     * This is useful to check how rcmail_action_mail_show() renders messages.
+     * It requires a running dovecot to fetch the messages from. Messages need
+     * to be placed as individual files in
+     * `tests/src/emails/test@example/Mail/cur/`.
+     */
+    protected function runAndGetHtmlOutputDomxpath(string $msgId): \DOMXPath
+    {
+        $imap_host = getenv('RC_CONFIG_IMAP_HOST') ?: 'tls://localhost:143';
+        $rcmail = \rcmail::get_instance();
+        // We need to overwrite the storage object, else storage_init() just
+        // returns the cached one (which might be a StorageMock instance).
+        $mockStorage = $rcmail->storage = null;
+        $rcmail->storage_init();
+        // Login our test user so we can fetch messages from the imap server.
+        $rcmail->login('test-message-rendering@localhost', 'pass', $imap_host);
+        $storage = $rcmail->get_storage();
+        $storage->set_options(['all_headers' => true]);
+        // We need to set the folder, else no message can be fetched.
+        $storage->set_folder('INBOX');
+        $output = $this->initOutput(\rcmail_action::MODE_HTTP, 'mail', 'preview');
+        // TODO: Why do we need to set the skin manually?
+        $output->set_skin('elastic');
+
+        $action = new \rcmail_action_mail_show();
+        $this->assertTrue($action->checks());
+
+        $messagesList = $storage->list_messages();
+
+        // Find the UID of the wanted message.
+        $messageUid = null;
+        foreach ($messagesList as $messageHeaders) {
+            if ($messageHeaders->get('message-id') === "<{$msgId}>") {
+                $messageUid = $messageHeaders->uid;
+                break;
+            }
+        }
+        if ($messageUid === null) {
+            throw new \Exception("No message found in messages list with Message-Id '{$msgId}'");
+        }
+
+        // Prepare and trigger the rendering.
+        $_GET = ['_uid' => $messageUid];
+        $html = '';
+        try {
+            $action->run();
+        } catch (ExitException $e) {
+            $html = $output->getOutput();
+        }
+
+        // Reset the storage to the mocked one most other tests expect.
+        $rcmail->storage = $mockStorage;
+
+        // disabled_html_ns=true is a workaround for the performance issue
+        // https://github.com/Masterminds/html5-php/issues/181
+        $html5 = new HTML5(['disable_html_ns' => true]);
+        return new \DOMXPath($html5->loadHTML($html));
+    }
+}

tests/MessageRendering/SingleImageNoTextTest.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace Tests\MessageRendering;
+
+/**
+ * Test class to test "interesting" messages.
+ */
+class SingleImageNoTextTest extends MessageRenderingTestCase
+{
+    /**
+     * Test that of a multipart/mixed message which contains only one
+     * image, that image is shown.
+     */
+    public function testShowMultipartMixedSingleImageToo()
+    {
+        $this->markTestSkipped('TBD: test for fixing GH issue 9443');
+        // This next comment line prevents phpstan from reporting this as
+        // unreachable code (technically it is right, but that's on purpose
+        // here...).
+        // @phpstan-ignore-next-line
+        $domxpath = $this->runAndGetHtmlOutputDomxpath('XXXXXXXXXXXXX@mx01.lytzenitmail.dk');
+
+        $this->assertSame('Not OK', $this->getScrubbedSubject($domxpath));
+
+        $attchNames = $domxpath->query('//span[@class="attachment-name"]');
+        $this->assertCount(1, $attchNames, 'Attachments');
+        $this->assertStringStartsWith('Resized_20240427_200026(1).jpeg', $attchNames[0]->textContent);
+    }
+}

tests/MessageRendering/bootstrap.php

@@ -0,0 +1,44 @@
+<?php
+
+/*
+ +-----------------------------------------------------------------------+
+ | This file is part of the Roundcube Webmail client                     |
+ |                                                                       |
+ | Copyright (C) The Roundcube Dev Team                                  |
+ |                                                                       |
+ | Licensed under the GNU General Public License version 3 or            |
+ | any later version with exceptions for skins & plugins.                |
+ | See the README file for a full license statement.                     |
+ |                                                                       |
+ | PURPOSE:                                                              |
+ |   Environment initialization script for unit tests                    |
+ +-----------------------------------------------------------------------+
+ | Author: Thomas Bruederli <roundcube@gmail.com>                        |
+ | Author: Aleksander Machniak <alec@alec.pl>                            |
+ +-----------------------------------------------------------------------+
+*/
+
+error_reporting(\E_ALL);
+
+if (\PHP_SAPI != 'cli') {
+    exit('Not in shell mode (php-cli)');
+}
+
+if (!defined('INSTALL_PATH')) {
+    define('INSTALL_PATH', realpath(__DIR__ . '/../../') . '/');
+}
+
+define('ROUNDCUBE_TEST_MODE', true);
+define('ROUNDCUBE_TEST_SESSION', microtime(true));
+define('TESTS_DIR', __DIR__ . '/');
+
+if (@is_dir(TESTS_DIR . 'config')) {
+    define('RCUBE_CONFIG_DIR', TESTS_DIR . 'config');
+}
+
+// Some tests depend on the way phpunit is executed
+$_SERVER['SCRIPT_NAME'] = 'vendor/bin/phpunit';
+
+require_once INSTALL_PATH . 'program/include/iniset.php';
+
+rcmail::get_instance(0, 'test')->config->set('devel_mode', false);

tests/MessageRendering/data/greenmail/test-message-rendering@localhost/INBOX/attachment-gh9443.eml

@@ -0,0 +1,34 @@
+Message-Id: <XXXXXXXXXXXXX@mx01.lytzenitmail.dk>
+Date:Mon, 06 May 2024 15:24:47 +0200
+From: <+45XXXXXXXX@telenor.dk>
+To: XXXXXX@jubii.dk
+Subject: Not OK
+Mime-Version:1.0
+Content-Type:multipart/mixed;boundary="--------------------------------------------=_NextPart_0_24856"
+
+this is a multi-part message in MIME format.
+
+----------------------------------------------=_NextPart_0_24856
+Content-Type:image/jpeg; name="Resized_20240427_200026(1).jpeg"
+Content-Transfer-Encoding:base64
+Content-Location:Resized_20240427_200026(1).jpeg
+Content-ID:<Resized_20240427_200026(1)>
+
+iVBORw0KGgoAAAANSUhEUgAAAQAAAAEACAIAAADTED8xAAADMElEQVR4nOzVwQnAIBQFQYXff81R
+UkQCOyDj1YOPnbXWPmeTRef+/3O/OyBjzh3CD95BfqICMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0C
+MK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMK0CMO0TAAD//2An
+hf4QtqobAAAAAElFTkSuQmCC
+
+----------------------------------------------=_NextPart_0_24856--