Return false from failed mb_convert_variables()

If we fail to detect the encoding return false, just like
mb_convert_encoding() does, and the implementation here clearly
intended. Previously the "pass" pseudo-incoding was returned.

diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c
index a529f848091186c0271e88c036cea4aa44f70a3b..e105e8638d7a3039dc2cbb40eebd760f02e3ace0 100644 (file)
--- a/ext/mbstring/mbstring.c
+++ b/ext/mbstring/mbstring.c
@@ -3217,9 +3217,7 @@ PHP_FUNCTION(mb_convert_variables)
                        from_encoding = mbfl_encoding_detector_judge(identd);
                        mbfl_encoding_detector_delete(identd);
                        if (recursion_error) {
-                               if (elist != NULL) {
-                                       efree((void *)elist);
-                               }
+                               efree(elist);
                                php_error_docref(NULL, E_WARNING, "Cannot handle recursive references");
                                RETURN_FALSE;
                        }
@@ -3227,49 +3225,42 @@ PHP_FUNCTION(mb_convert_variables)
 
                if (!from_encoding) {
                        php_error_docref(NULL, E_WARNING, "Unable to detect encoding");
-                       from_encoding = &mbfl_encoding_pass;
+                       efree(elist);
+                       RETURN_FALSE;
                }
        }
-       efree((void *)elist);
-       /* create converter */
-       convd = NULL;
-       if (from_encoding != &mbfl_encoding_pass) {
-               convd = mbfl_buffer_converter_new(from_encoding, to_encoding, 0);
-               /* If this assertion fails this means some memory allocation failure which is a bug */
-               ZEND_ASSERT(convd != NULL);
 
-               mbfl_buffer_converter_illegal_mode(convd, MBSTRG(current_filter_illegal_mode));
-               mbfl_buffer_converter_illegal_substchar(convd, MBSTRG(current_filter_illegal_substchar));
-       }
+       efree(elist);
 
-       /* convert */
-       if (convd != NULL) {
-               n = 0;
-               while (n < argc) {
-                       zval *zv = &args[n];
+       convd = mbfl_buffer_converter_new(from_encoding, to_encoding, 0);
+       /* If this assertion fails this means some memory allocation failure which is a bug */
+       ZEND_ASSERT(convd != NULL);
 
-                       ZVAL_DEREF(zv);
-                       recursion_error = mb_recursive_convert_variable(convd, zv);
-                       if (recursion_error) {
-                               break;
-                       }
-                       n++;
-               }
+       mbfl_buffer_converter_illegal_mode(convd, MBSTRG(current_filter_illegal_mode));
+       mbfl_buffer_converter_illegal_substchar(convd, MBSTRG(current_filter_illegal_substchar));
 
-               MBSTRG(illegalchars) += mbfl_buffer_illegalchars(convd);
-               mbfl_buffer_converter_delete(convd);
+       /* convert */
+       n = 0;
+       while (n < argc) {
+               zval *zv = &args[n];
 
+               ZVAL_DEREF(zv);
+               recursion_error = mb_recursive_convert_variable(convd, zv);
                if (recursion_error) {
-                       php_error_docref(NULL, E_WARNING, "Cannot handle recursive references");
-                       RETURN_FALSE;
+                       break;
                }
+               n++;
        }
 
-       if (from_encoding) {
-               RETURN_STRING(from_encoding->name);
-       } else {
+       MBSTRG(illegalchars) += mbfl_buffer_illegalchars(convd);
+       mbfl_buffer_converter_delete(convd);
+
+       if (recursion_error) {
+               php_error_docref(NULL, E_WARNING, "Cannot handle recursive references");
                RETURN_FALSE;
        }
+
+       RETURN_STRING(from_encoding->name);
 }
 /* }}} */
 

diff --git a/ext/mbstring/tests/mb_convert_encoding_failed_detection.phpt b/ext/mbstring/tests/mb_convert_encoding_failed_detection.phpt
index 961af505ec395c7ca17906ecde32b47f50f668b1..b3c8ba10bba6371d5efc9a3563155203acd39be9 100644 (file)
--- a/ext/mbstring/tests/mb_convert_encoding_failed_detection.phpt
+++ b/ext/mbstring/tests/mb_convert_encoding_failed_detection.phpt
@@ -5,7 +5,15 @@ mb_convert_encoding() when encoding detection fails
 
 var_dump(mb_convert_encoding("\xff", "ASCII", ["UTF-8", "UTF-16"]));
 
+$str = "\xff";
+var_dump(mb_convert_variables("ASCII", ["UTF-8", "UTF-16"], $str));
+var_dump(bin2hex($str));
+
 ?>
 --EXPECTF--
 Warning: mb_convert_encoding(): Unable to detect character encoding in %s on line %d
 bool(false)
+
+Warning: mb_convert_variables(): Unable to detect encoding in %s on line %d
+bool(false)
+string(2) "ff"