Merge pull request #1230 from cypht-org/bugfix/multibyte-imap

fix multibyte handling in imap literals and address splitting, improve unit tests

modules/core/message_functions.php

@@ -503,19 +503,20 @@ function addr_split($str, $seps = array(',', ';')) {
     $capture = false;
     $capture_chars = array('"' => '"', '(' => ')', '<' => '>');
     for ($i=0;$i<$max;$i++) {
-        if ($capture && $capture_chars[$capture] == $str[$i]) {
+        $char = mb_substr($str, $i, 1);
+        if ($capture && $capture_chars[$capture] == $char) {
             $capture = false;
         }
-        elseif (!$capture && in_array($str[$i], array_keys($capture_chars))) {
-            $capture = $str[$i];
+        elseif (!$capture && in_array($char, array_keys($capture_chars))) {
+            $capture = $char;
         }
 
-        if (!$capture && in_array($str[$i], $seps)) {
+        if (!$capture && in_array($char, $seps)) {
             $words[] = trim($word);
             $word = '';
         }
         else {
-            $word .= $str[$i];
+            $word .= $char;
         }
     }
     $words[] = trim($word);

modules/imap/hm-imap-base.php

@@ -58,6 +58,8 @@ private function command_number() {
 
     /**
      * Read IMAP literal found during parse_line().
+     * NOTE: it is important to treat sizes and string functions
+     * in bytes here as literal size is specified in bytes (and not characters). 
      * @param int $size size of the IMAP literal to read
      * @param int $max max size to allow
      * @param int $current current size read
@@ -69,11 +71,11 @@ private function command_number() {
     private function read_literal($size, $max, $current, $line_length) {
         $left_over = false;
         $literal_data = $this->fgets($line_length);
-        $lit_size = mb_strlen($literal_data);
+        $lit_size = strlen($literal_data);
         $current += $lit_size;
         while ($lit_size < $size) {
             $chunk = $this->fgets($line_length);
-            $chunk_size = mb_strlen($chunk);
+            $chunk_size = strlen($chunk);
             $lit_size += $chunk_size;
             $current += $chunk_size;
             $literal_data .= $chunk;
@@ -85,12 +87,12 @@ private function read_literal($size, $max, $current, $line_length) {
         if ($this->max_read) {
             while ($lit_size < $size) {
                 $temp = $this->fgets($line_length);
-                $lit_size += mb_strlen($temp);
+                $lit_size += strlen($temp);
             }
         }
-        elseif ($size < mb_strlen($literal_data)) {
-            $left_over = mb_substr($literal_data, $size);
-            $literal_data = mb_substr($literal_data, 0, $size);
+        elseif ($size < strlen($literal_data)) {
+            $left_over = substr($literal_data, $size);
+            $literal_data = substr($literal_data, 0, $size);
         }
         return array($literal_data, $left_over);
     }
@@ -122,35 +124,37 @@ protected function parse_line($line, $current_size, $max, $line_length) {
         /* walk through the line */
         for ($i=0;$i<$len;$i++) {
 
+            $char = mb_substr($line, $i, 1);
+
             /* this will hold one "atom" from the parsed line */
             $chunk = '';
 
             /* if we hit a newline exit the loop */
-            if ($line[$i] == "\r" || $line[$i] == "\n") {
+            if ($char == "\r" || $char == "\n") {
                 $line_cont = false;
                 break;
             }
 
             /* skip spaces */
-            if ($line[$i] == ' ') {
+            if ($char == ' ') {
                 continue;
             }
 
             /* capture special chars as "atoms" */
-            elseif ($line[$i] == '*' || $line[$i] == '[' || $line[$i] == ']' || $line[$i] == '(' || $line[$i] == ')') {
-                $chunk = $line[$i];
+            elseif ($char == '*' || $char == '[' || $char == ']' || $char == '(' || $char == ')') {
+                $chunk = $char;
             }
 
             /* regex match a quoted string */
-            elseif ($line[$i] == '"') {
+            elseif ($char == '"') {
                 if (preg_match("/^(\"[^\"\\\]*(?:\\\.[^\"\\\]*)*\")/", mb_substr($line, $i), $matches)) {
                     $chunk = mb_substr($matches[1], 1, -1);
                 }
                 $i += mb_strlen($chunk) + 1;
             }
 
             /* IMAP literal */
-            elseif ($line[$i] == '{') {
+            elseif ($char == '{') {
                 $end = mb_strpos($line, '}');
                 if ($end !== false) {
                     $literal_size  = mb_substr($line, ($i + 1), ($end - $i - 1));
@@ -218,13 +222,14 @@ protected function fgets($len=false) {
      * loop through "lines" returned from imap and parse them with parse_line() and read_literal.
      * it can return the lines in a raw format, or parsed into atoms. It also supports a maximum
      * number of lines to return, in case we did something stupid like list a loaded unix homedir
+     * used by scram lib, so keep it public
      * @param int $max max size of response allowed
      * @param bool $chunked flag to parse the data into IMAP "atoms"
      * @param int $line_length chunk size to read in literals using fgets
      * @param bool $sort flag for non-compliant sort result parsing speed up
      * @return array of parsed or raw results
      */
-    protected function get_response($max=false, $chunked=false, $line_length=8192, $sort=false) {
+    public function get_response($max=false, $chunked=false, $line_length=8192, $sort=false) {
         /* defaults and results containers */
         $result = array();
         $current_size = 0;
@@ -356,11 +361,12 @@ protected function get_response($max=false, $chunked=false, $line_length=8192, $
 
     /**
      * put a prefix on a command and send it to the server
+     * used by scram lib, so keep it public
      * @param mixed $command IMAP command
      * @param bool $no_prefix flag to skip adding the prefix
      * @return void
      */
-    protected function send_command($command, $no_prefix=false) {
+    public function send_command($command, $no_prefix=false) {
         $this->cached_response = false;
         if (!$no_prefix) {
             $command = 'A'.$this->command_number().' '.$command;

tests/phpunit/imap_commands.php

@@ -116,7 +116,7 @@
     'A5 NOOP' =>
         "* 23 EXISTS\r\n".
         "A5 OK NOOP Completed\r\n",
-    'A8 UID FETCH 1731,1732 (FLAGS INTERNALDATE RFC822.SIZE BODY.PEEK[HEADER.FIELDS (SUBJECT X-AUTO-BCC FROM DATE CONTENT-TYPE X-PRIORITY TO LIST-ARCHIVE REFERENCES MESSAGE-ID)])' =>
+    'A8 UID FETCH 1731,1732 (FLAGS INTERNALDATE RFC822.SIZE BODY.PEEK[HEADER.FIELDS (SUBJECT X-AUTO-BCC FROM DATE CONTENT-TYPE X-PRIORITY TO LIST-ARCHIVE REFERENCES MESSAGE-ID X-SNOOZED)])' =>
         "* 92 FETCH (UID 1731 FLAGS (\Seen) INTERNALDATE \"02-May-2017 16:32:24 -0500\" RFC822.SIZE 1940 BODY[HEADER.FIELDS (SUBJECT X-AUTO-BCC FROM DATE CONTENT-TYPE X-PRIORITY TO LIST-ARCHIVE REFERENCES MESSAGE-ID)] {240}\r\n".
         "Subject: =?utf-8?q?apt-listchanges=3A_news_for_shop?=\r\n".
         "To: [email protected]\r\n".
@@ -137,7 +137,7 @@
         ")\r\n".
         "A5 OK Fetch completed (0.001 + 0.000 secs).\r\n",
 
-    'A5 UID FETCH 1731,1732 (FLAGS INTERNALDATE RFC822.SIZE BODY.PEEK[HEADER.FIELDS (SUBJECT X-AUTO-BCC FROM DATE CONTENT-TYPE X-PRIORITY TO LIST-ARCHIVE REFERENCES MESSAGE-ID)])' =>
+    'A5 UID FETCH 1731,1732 (FLAGS INTERNALDATE RFC822.SIZE BODY.PEEK[HEADER.FIELDS (SUBJECT X-AUTO-BCC FROM DATE CONTENT-TYPE X-PRIORITY TO LIST-ARCHIVE REFERENCES MESSAGE-ID X-SNOOZED)])' =>
         "* 92 FETCH (UID 1731 FLAGS (\Seen) INTERNALDATE \"02-May-2017 16:32:24 -0500\" RFC822.SIZE 1940 BODY[HEADER.FIELDS (SUBJECT X-AUTO-BCC FROM DATE CONTENT-TYPE X-PRIORITY TO LIST-ARCHIVE REFERENCES MESSAGE-ID)] {240}\r\n".
         "Subject: =?utf-8?q?apt-listchanges=3A_news_for_shop?=\r\n".
         "To: [email protected]\r\n".
@@ -194,7 +194,7 @@
         "* SEARCH 1680 1682\r\n".
         "A5 OK Search completed (0.007 + 0.000 + 0.006 secs).\r\n",
 
-    'A5 UID FETCH 1731 (FLAGS BODY[HEADER])' =>
+    'A5 UID FETCH 1731 (FLAGS INTERNALDATE BODY[HEADER])' =>
         "* 92 FETCH (UID 1731 FLAGS (\Seen) BODY[HEADER] {623}\r\n".
         "Return-path: <[email protected]>\r\n".
         "Envelope-to: [email protected]\r\n".
@@ -316,4 +316,10 @@
 
     'A5 COMPRESS DEFLATE' =>
         "A5 OK DEFLATE active\r\n",
+
+    'A5 TEST MULTIBYTE' =>
+        "A1 OK {12}\r\n".
+        "Literäääl\r\n".
+        "A2 OK {7}\r\n".
+        "Literal\r\n",
 );

tests/phpunit/modules/core/message_functions.php

@@ -176,4 +176,14 @@ public function test_process_address_fld() {
         );
         $this->assertEquals($res, process_address_fld('"stuff" foo [email protected] (comment here), bad address <"[email protected]">, good address <[email protected]>, \'[email protected]\' [email protected]'));
     }
+
+    /**
+     * @preserveGlobalState disabled
+     * @runInSeparateProcess
+     */
+    public function test_multibyte_addr_split() {
+        $test = 'Thömas Tester [email protected]';
+        $result = addr_split($test);
+        $this->assertEquals($test, $result[0]);
+    }
 }