Fix #66828: iconv_mime_encode Q-encoding longer than it should be

Before the fix for bug 48289 has been applied, the algorithm to
construct a Q-encoded-word has been optimistic, i.e. try to encode as
many bytes that *may* fit in the remaining space, calculate the actual
length of the Q-encoded word, and if it's too long, try again with a
reduced size.  However, the fix for the mentioned bug replaced this by
a pessimistic algorithm, which always terminates[1] the for loop[2]
during the first iteration (which renders the following 3 lines as dead
code), and as such easily produces unnecessarily short encoded-words.
Instead the proper fix for the bug would have been to make sure that
`out_size` is always decremented, if the space isn't sufficient for the
encoded-word.

[1] <https://github.com/php/php-src/blob/php-7.3.0beta3/ext/iconv/iconv.c#L1421>
[2] <https://github.com/php/php-src/blob/php-7.3.0beta3/ext/iconv/iconv.c#L1360>

diff --git a/NEWS b/NEWS
index 1add82df498b50150e82f4643ba3df113c7a71d8..00ee93ea437770ec75c0b6dbc6f5f103996302fe 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,10 @@ PHP                                                                        NEWS
   . Fixed bug #76480 (Use curl_multi_wait() so that timeouts are respected).
     (Pierrick)
 
+- iconv:
+  . Fixed bug #66828 (iconv_mime_encode Q-encoding longer than it should be).
+    (cmb)
+
 - Opcache:
   . Fixed bug #76832 (ZendOPcache.MemoryBase periodically deleted by the OS).
     (Anatol)

diff --git a/ext/iconv/iconv.c b/ext/iconv/iconv.c
index de5bccbb9773639fba80137cceaf434952c6b029..ef27b20645be0321aff03bb2b96ca348638b7cea 100644 (file)
--- a/ext/iconv/iconv.c
+++ b/ext/iconv/iconv.c
@@ -1367,7 +1367,7 @@ static php_iconv_err_t _php_iconv_mime_encode(smart_str *pretval, const char *fn
                                prev_in_left = ini_in_left = in_left;
                                ini_in_p = in_p;
 
-                               for (out_size = (char_cnt - 2) / 3; out_size > 0;) {
+                               for (out_size = (char_cnt - 2); out_size > 0;) {
 #if !ICONV_SUPPORTS_ERRNO
                                        size_t prev_out_left;
 #endif
@@ -1431,7 +1431,7 @@ static php_iconv_err_t _php_iconv_mime_encode(smart_str *pretval, const char *fn
                                                break;
                                        }
 
-                                       out_size -= ((nbytes_required - (char_cnt - 2)) + 1) / 3;
+                                       out_size -= ((nbytes_required - (char_cnt - 2)) + 2) / 3;
                                        in_left = ini_in_left;
                                        in_p = ini_in_p;
                                }

diff --git a/ext/iconv/tests/bug66828.phpt b/ext/iconv/tests/bug66828.phpt
new file mode 100644 (file)
index 0000000..9914b41
--- /dev/null
+++ b/ext/iconv/tests/bug66828.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Bug #66828 (iconv_mime_encode Q-encoding longer than it should be)
+--SKIPIF--
+<?php
+if (!extension_loaded('iconv')) die('skip iconv extension not available');
+?>
+--FILE--
+<?php
+$preferences = array(
+    "input-charset" => "ISO-8859-1",
+    "output-charset" => "UTF-8",
+    "line-length" => 76,
+    "line-break-chars" => "\n",
+    "scheme" => "Q"
+);
+var_dump(iconv_mime_encode("Subject", "Test Test Test Test Test Test Test Test", $preferences));
+?>
+===DONE===
+--EXPECT--
+string(74) "Subject: =?UTF-8?Q?Test=20Test=20Test=20Test=20Test=20Test=20Test=20Test?="
+===DONE===