Merge pull request #277 from buggregator/issue/274

SMTP module: Fix response code and Cyrillic character handling issues

Author: butschster

.github/workflows/docker-dev-image.yml

@@ -43,7 +43,7 @@ jobs:
           push: true
           build-args: |
             APP_VERSION=${{ steps.previoustag.outputs.tag }}
-            FRONTEND_IMAGE_TAG=dev
+            FRONTEND_IMAGE_TAG=latest
             BRANCH=${{ steps.previoustag.outputs.tag }}
           tags:
             ghcr.io/${{ github.repository }}:dev, ghcr.io/${{ github.repository }}:${{ steps.previoustag.outputs.tag }}

.github/workflows/phpunit-mysql.yml

@@ -62,7 +62,7 @@ jobs:
         run: echo "::set-output name=dir::$(composer config cache-files-dir)"
 
       - name: Restore Composer Cache
-        uses: actions/cache@v2
+        uses: actions/cache@v3
         with:
           path: ${{ steps.composer-cache.outputs.dir }}
           key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ hashFiles('**/composer.json') }}

.github/workflows/phpunit-pgsql.yml

@@ -60,7 +60,7 @@ jobs:
         run: echo "::set-output name=dir::$(composer config cache-files-dir)"
 
       - name: Restore Composer Cache
-        uses: actions/cache@v2
+        uses: actions/cache@v3
         with:
           path: ${{ steps.composer-cache.outputs.dir }}
           key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ hashFiles('**/composer.json') }}

.gitignore

@@ -13,3 +13,4 @@ protoc-gen-php-grpc*
 .db
 .sqlhistory
 *Zone.Identifier
+.context

.php-cs-fixer.php

@@ -15,6 +15,6 @@
         (new PhpCsFixer\Finder())
             ->files()
             ->name('*.php')
-            ->in([__DIR__ . '/app/src', __DIR__ . '/app/modules']),
+            ->in([__DIR__ . '/app/src', __DIR__ . '/app/modules', __DIR__ . '/tests']),
     )
     ->setCacheFile('.cache/.php-cs-fixer.cache');

app/modules/Smtp/Application/Mail/Message.php

@@ -36,7 +36,8 @@ public function getBccs(): array
         return \array_values(
             \array_filter($this->allRecipients, function (string $recipient) {
                 foreach (\array_merge($this->recipients, $this->ccs) as $publicRecipient) {
-                    if (\str_contains($publicRecipient, $recipient)) {
+                    // Use email from the public recipient array
+                    if (isset($publicRecipient['email']) && \str_contains($publicRecipient['email'], $recipient)) {
                         return false;
                     }
                 }

app/modules/Smtp/Application/Storage/Message.php

@@ -4,8 +4,6 @@
 
 namespace Modules\Smtp\Application\Storage;
 
-use Modules\Smtp\Application\Mail\Parser;
-
 final class Message
 {
     public function __construct(
@@ -45,21 +43,32 @@ public function setFrom(string $from): void
 
     public function appendBody(string $body): void
     {
-        $this->body .= \preg_replace("/^(\.\.)/m", '.', $body);
+        // Handle escaped periods at the beginning of lines per SMTP spec
+        $safeBody = \preg_replace("/^(\.\.)/m", '.', $body);
+
+        // Ensure body is properly appended even with multi-byte characters
+        $this->body .= $safeBody;
     }
 
     public function bodyHasEos(): bool
     {
-        return \str_ends_with($this->body, "\r\n.\r\n");
+        // More robust check for end of stream marker
+        // This handles potential encoding issues with multi-byte characters
+        return \mb_substr($this->body, -5) === "\r\n.\r\n";
     }
 
     public function getBody(): string
     {
-        return \str_replace("\r\n.\r\n", '', $this->body);
+        // Remove the end of stream marker in a way that's safe for multi-byte strings
+        if ($this->bodyHasEos()) {
+            return \mb_substr($this->body, 0, \mb_strlen($this->body) - 5);
+        }
+
+        return $this->body;
     }
 
     public function parse(): \Modules\Smtp\Application\Mail\Message
     {
-        return (new Parser())->parse($this->getBody());
+        return ParserFactory::getInstance()->create()->parse($this->getBody(), $this->recipients);
     }
 }

app/modules/Smtp/Application/Storage/ParserFactory.php

@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Modules\Smtp\Application\Storage;
+
+use Modules\Smtp\Application\Mail\Parser;
+
+/**
+ * Factory for creating Parser instances
+ * This class exists primarily to facilitate testing by allowing
+ * the Parser instantiation to be mocked or overridden.
+ *
+ * @internal
+ */
+final class ParserFactory
+{
+    // Static instance for singleton pattern
+    private static ?self $instance = null;
+
+    // The Parser instance or null
+    private ?Parser $parser = null;
+
+    /**
+     * Get the factory instance
+     */
+    public static function getInstance(): self
+    {
+        if (!self::$instance instanceof \Modules\Smtp\Application\Storage\ParserFactory) {
+            self::$instance = new self();
+        }
+
+        return self::$instance;
+    }
+
+    /**
+     * Reset the factory instance (useful for testing)
+     */
+    public static function reset(): void
+    {
+        self::$instance = null;
+    }
+
+    /**
+     * Set a custom Parser instance
+     *
+     * @param Parser $parser The Parser instance to use
+     */
+    public function setParser(Parser $parser): void
+    {
+        $this->parser = $parser;
+    }
+
+    /**
+     * Create a Parser instance
+     *
+     * @return Parser The Parser instance
+     */
+    public function create(): Parser
+    {
+        return $this->parser ?? new Parser();
+    }
+}

app/modules/Smtp/Interfaces/TCP/Service.php

@@ -65,6 +65,9 @@ public function handle(Request $request): ResponseInterface
             $response = $this->makeResponse(ResponseMessage::closing(), close: true);
             $message = $this->emailBodyStorage->cleanup($request->connectionUuid);
         } elseif ($request->body === "DATA\r\n") {
+            // Reset the body to empty string when starting a new DATA command
+            // This prevents confusion between multiple DATA commands in the same session
+            $message->body = '';
             $response = $this->makeResponse(ResponseMessage::provideBody());
             $message->waitBody = true;
         } elseif ($request->body === "RSET\r\n") {
@@ -75,13 +78,16 @@ public function handle(Request $request): ResponseInterface
         } elseif ($message->waitBody) {
             $message->appendBody($request->body);
 
-            $response = $this->makeResponse(ResponseMessage::ok());
-
+            // FIX: Only send one response when data ends
             if ($message->bodyHasEos()) {
                 $uuid = $this->dispatchMessage($message->parse(), project: $message->username);
-
                 $response = $this->makeResponse(ResponseMessage::accepted($uuid));
                 $dispatched = true;
+                // Reset the waitBody flag to false since we've processed the message
+                $message->waitBody = false;
+            } else {
+                // Only send "OK" response if we're not at the end of data
+                $response = $this->makeResponse(ResponseMessage::ok());
             }
         }
 

app/modules/context.yaml

@@ -0,0 +1,17 @@
+$schema: 'https://raw.githubusercontent.com/context-hub/generator/refs/heads/main/json-schema.json'
+
+documents:
+  - description: Events module
+    outputPath: module/events.md
+    sources:
+      - type: file
+        sourcePaths:
+          - ./Events
+
+  - description: SMTP module
+    outputPath: module/smtp.md
+    sources:
+      - type: file
+        sourcePaths:
+          - ./Smtp
+          - ../../docs/smtp.md