Throw Value/TypeError for invalid $bodies in imap_mail_compose()

Small drive by refactoring to use HashTables

Closes GH-6371

diff --git a/ext/imap/php_imap.c b/ext/imap/php_imap.c
index 147df9d4faacfb49f843781a9626be4b8984678e..c561a5e72c0209fee77a3bce9439baac59fb91e4 100644 (file)
--- a/ext/imap/php_imap.c
+++ b/ext/imap/php_imap.c
@@ -3093,7 +3093,7 @@ PHP_FUNCTION(imap_fetch_overview)
 /* {{{ Create a MIME message based on given envelope and body sections */
 PHP_FUNCTION(imap_mail_compose)
 {
-       zval *envelope, *body;
+       HashTable *envelope, *body;
        zend_string *key;
        zval *data, *pvalue, *disp_data, *env_data;
        char *cookie = NIL;
@@ -3105,62 +3105,66 @@ PHP_FUNCTION(imap_mail_compose)
        int toppart = 0;
        int first;
 
-       if (zend_parse_parameters(ZEND_NUM_ARGS(), "a/a/", &envelope, &body) == FAILURE) {
+       if (zend_parse_parameters(ZEND_NUM_ARGS(), "h/h/", &envelope, &body) == FAILURE) {
                RETURN_THROWS();
        }
 
+       if (zend_hash_num_elements(body) == 0) {
+               zend_argument_value_error(2, "cannot be empty");
+       }
+
 #define PHP_RFC822_PARSE_ADRLIST(target, value) \
        str_copy = estrndup(Z_STRVAL_P(value), Z_STRLEN_P(value)); \
        rfc822_parse_adrlist(target, str_copy, "NO HOST"); \
        efree(str_copy);
 
        env = mail_newenvelope();
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "remail", sizeof("remail") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "remail", sizeof("remail") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                env->remail = cpystr(Z_STRVAL_P(pvalue));
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "return_path", sizeof("return_path") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "return_path", sizeof("return_path") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                PHP_RFC822_PARSE_ADRLIST(&env->return_path, pvalue);
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "date", sizeof("date") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "date", sizeof("date") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                env->date = (unsigned char*)cpystr(Z_STRVAL_P(pvalue));
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "from", sizeof("from") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "from", sizeof("from") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                PHP_RFC822_PARSE_ADRLIST(&env->from, pvalue);
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "reply_to", sizeof("reply_to") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "reply_to", sizeof("reply_to") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                PHP_RFC822_PARSE_ADRLIST(&env->reply_to, pvalue);
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "in_reply_to", sizeof("in_reply_to") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "in_reply_to", sizeof("in_reply_to") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                env->in_reply_to = cpystr(Z_STRVAL_P(pvalue));
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "subject", sizeof("subject") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "subject", sizeof("subject") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                env->subject = cpystr(Z_STRVAL_P(pvalue));
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "to", sizeof("to") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "to", sizeof("to") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                PHP_RFC822_PARSE_ADRLIST(&env->to, pvalue);
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "cc", sizeof("cc") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "cc", sizeof("cc") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                PHP_RFC822_PARSE_ADRLIST(&env->cc, pvalue);
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "bcc", sizeof("bcc") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "bcc", sizeof("bcc") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                PHP_RFC822_PARSE_ADRLIST(&env->bcc, pvalue);
        }
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "message_id", sizeof("message_id") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "message_id", sizeof("message_id") - 1)) != NULL) {
                convert_to_string_ex(pvalue);
                env->message_id=cpystr(Z_STRVAL_P(pvalue));
        }
 
-       if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(envelope), "custom_headers", sizeof("custom_headers") - 1)) != NULL) {
+       if ((pvalue = zend_hash_str_find(envelope, "custom_headers", sizeof("custom_headers") - 1)) != NULL) {
                if (Z_TYPE_P(pvalue) == IS_ARRAY) {
                        custom_headers_param = tmp_param = NULL;
                        SEPARATE_ARRAY(pvalue);
@@ -3177,14 +3181,17 @@ PHP_FUNCTION(imap_mail_compose)
        }
 
        first = 1;
-       ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(body), data) {
+       ZEND_HASH_FOREACH_VAL(body, data) {
                if (first) {
                        first = 0;
 
                        if (Z_TYPE_P(data) != IS_ARRAY) {
-                               // TODO ValueError
-                               php_error_docref(NULL, E_WARNING, "body parameter must be a non-empty array");
-                               RETVAL_FALSE;
+                               zend_argument_type_error(2, "individual body must be of type array, %s given",
+                                       zend_zval_type_name(data));
+                               goto done;
+                       }
+                       if (zend_hash_num_elements(Z_ARRVAL_P(data)) == 0) {
+                               zend_argument_value_error(2, "individual body cannot be empty");
                                goto done;
                        }
                        SEPARATE_ARRAY(data);
@@ -3402,13 +3409,6 @@ PHP_FUNCTION(imap_mail_compose)
                }
        } ZEND_HASH_FOREACH_END();
 
-       if (first) {
-               // TODO ValueError
-               php_error_docref(NULL, E_WARNING, "body parameter must be a non-empty array");
-               RETVAL_FALSE;
-               goto done;
-       }
-
        if (bod && bod->type == TYPEMULTIPART && (!bod->nested.part || !bod->nested.part->next)) {
                php_error_docref(NULL, E_WARNING, "Cannot generate multipart e-mail without components.");
                RETVAL_FALSE;

diff --git a/ext/imap/tests/bug45705_2.phpt b/ext/imap/tests/bug45705_2.phpt
index 3de50290f7e386447c87c3cb5d82f2876722ff06..97c4118614c5311a1a307b73eda0590f8b8e021d 100644 (file)
--- a/ext/imap/tests/bug45705_2.phpt
+++ b/ext/imap/tests/bug45705_2.phpt
@@ -18,7 +18,7 @@ $envelope = array('return_path' => 'John Doe <john@example.com>',
 );
 
 var_dump($envelope);
-imap_mail_compose($envelope, array(1 => array()));
+imap_mail_compose($envelope, [1 => ['cc' => 'Steve Doe <steve@example.com>',]]);
 var_dump($envelope);
 
 ?>

diff --git a/ext/imap/tests/bug80223.phpt b/ext/imap/tests/bug80223.phpt
index 4acfb8d02364eb4512d7c3830c1d59aa6e8a82a8..eeb2c3e75663e5e7eca369e045ee5529abbf34ca 100644 (file)
--- a/ext/imap/tests/bug80223.phpt
+++ b/ext/imap/tests/bug80223.phpt
@@ -6,10 +6,23 @@ if (!extension_loaded('imap')) die('skip imap extension not available');
 ?>
 --FILE--
 <?php
-imap_mail_compose([], []);
-imap_mail_compose([], [1]);
+try {
+    imap_mail_compose([], []);
+} catch (\ValueError $e) {
+    echo $e->getMessage(), \PHP_EOL;
+}
+try {
+    imap_mail_compose([], [1]);
+} catch (\TypeError $e) {
+    echo $e->getMessage(), \PHP_EOL;
+}
+try {
+    imap_mail_compose([], [[]]);
+} catch (\ValueError $e) {
+    echo $e->getMessage(), \PHP_EOL;
+}
 ?>
---EXPECTF--
-Warning: imap_mail_compose(): body parameter must be a non-empty array in %s on line %d
-
-Warning: imap_mail_compose(): body parameter must be a non-empty array in %s on line %d
+--EXPECT--
+imap_mail_compose(): Argument #2 ($bodies) cannot be empty
+imap_mail_compose(): Argument #2 ($bodies) individual body must be of type array, int given
+imap_mail_compose(): Argument #2 ($bodies) individual body cannot be empty

diff --git a/ext/imap/tests/imap_body_basic.phpt b/ext/imap/tests/imap_body_basic.phpt
index af434cffb65a62eda6542fcbf9d50e8d0b6beb1a..3034785a90f139125a5c728d049db1dfe1436f92 100644 (file)
--- a/ext/imap/tests/imap_body_basic.phpt
+++ b/ext/imap/tests/imap_body_basic.phpt
@@ -37,5 +37,7 @@ Create a new mailbox for test
 Create a temporary mailbox and add 1 msgs
 .. mailbox '%s' created
 Msg Count in new mailbox: 1
-string(%d) "1: this is a test message, please ignore%a"
-string(%d) "1: this is a test message, please ignore%a"
+string(%d) "1: this is a test message, please ignore
+"
+string(%d) "1: this is a test message, please ignore
+"