From f2ce54489fc785bb5f71a94c24ec75c51c39ad5f Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Thu, 17 Sep 2020 17:22:01 +0200 Subject: [PATCH 1/4] Promote warnings to Error in IMAP --- ext/imap/php_imap.c | 375 +++++++++++------- ext/imap/tests/imap_body.phpt | 21 +- ext/imap/tests/imap_close_variation4.phpt | 19 +- .../tests/imap_fetch_overview_variation3.phpt | 29 +- ext/imap/tests/imap_fetchbody_variation4.phpt | 25 +- ext/imap/tests/imap_fetchbody_variation6.phpt | 13 +- .../tests/imap_fetchheader_variation3.phpt | 24 +- .../tests/imap_fetchheader_variation5.phpt | 13 +- ext/imap/tests/imap_fetchstructure_basic.phpt | 7 +- ext/imap/tests/imap_gc_error.phpt | 11 +- ext/imap/tests/imap_open_error.phpt | 22 +- 11 files changed, 323 insertions(+), 236 deletions(-) diff --git a/ext/imap/php_imap.c b/ext/imap/php_imap.c index 27ff2058c7a3b..62e91f3c77fe6 100644 --- a/ext/imap/php_imap.c +++ b/ext/imap/php_imap.c @@ -141,9 +141,14 @@ ZEND_GET_MODULE(imap) /* True globals, no need for thread safety */ static int le_imap; -#define PHP_IMAP_CHECK_MSGNO(msgindex) \ - if ((msgindex < 1) || ((unsigned) msgindex > imap_le_struct->imap_stream->nmsgs)) { \ - php_error_docref(NULL, E_WARNING, "Bad message number"); \ +// TODO Promote to ValueError? +#define PHP_IMAP_CHECK_MSGNO(msgindex, arg_pos) \ + if (msgindex < 1) { \ + zend_argument_value_error(arg_pos, "must be greater than 0"); \ + RETURN_THROWS(); \ + } \ + if (((unsigned) msgindex) > imap_le_struct->imap_stream->nmsgs) { \ + php_error_docref(NULL, E_WARNING, "Bad message number"); \ RETURN_FALSE; \ } \ @@ -695,18 +700,29 @@ PHP_MINFO_FUNCTION(imap) } /* }}} */ -/* {{{ imap_do_open */ -static void php_imap_do_open(INTERNAL_FUNCTION_PARAMETERS, int persistent) +/* {{{ Open an IMAP stream to a mailbox */ +PHP_FUNCTION(imap_open) { zend_string *mailbox, *user, *passwd; zend_long retries = 0, flags = NIL, cl_flags = NIL; MAILSTREAM *imap_stream; pils *imap_le_struct; - zval *params = NULL; + HashTable *params = NULL; int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "PSS|lla", &mailbox, &user, - &passwd, &flags, &retries, ¶ms) == FAILURE) { + if (zend_parse_parameters(argc, "PSS|llh", &mailbox, &user, &passwd, &flags, &retries, ¶ms) == FAILURE) { + RETURN_THROWS(); + } + + if (flags && ((flags & ~(OP_READONLY | OP_ANONYMOUS | OP_HALFOPEN | CL_EXPUNGE | OP_DEBUG | OP_SHORTCACHE + | OP_SILENT | OP_PROTOTYPE | OP_SECURE)) != 0)) { + zend_argument_value_error(4, "must be a bitmask of OP_READONLY, OP_ANONYMOUS, OP_HALFOPEN, CL_EXPUNGE, " + "OP_DEBUG, OP_SHORTCACHE, OP_SILENT, OP_PROTOTYPE, and OP_SECURE"); + RETURN_THROWS(); + } + + if (retries < 0) { + zend_argument_value_error(5, "must be greater than or equal to 0"); RETURN_THROWS(); } @@ -723,7 +739,7 @@ static void php_imap_do_open(INTERNAL_FUNCTION_PARAMETERS, int persistent) if (params) { zval *disabled_auth_method; - if ((disabled_auth_method = zend_hash_str_find(Z_ARRVAL_P(params), "DISABLE_AUTHENTICATOR", sizeof("DISABLE_AUTHENTICATOR") - 1)) != NULL) { + if ((disabled_auth_method = zend_hash_str_find(params, "DISABLE_AUTHENTICATOR", sizeof("DISABLE_AUTHENTICATOR") - 1)) != NULL) { switch (Z_TYPE_P(disabled_auth_method)) { case IS_STRING: if (Z_STRLEN_P(disabled_auth_method) > 1) { @@ -746,16 +762,16 @@ static void php_imap_do_open(INTERNAL_FUNCTION_PARAMETERS, int persistent) mail_parameters(NIL, DISABLE_AUTHENTICATOR, (void *)Z_STRVAL_P(z_auth_method)); } } else { - php_error_docref(NULL, E_WARNING, "Invalid argument, expect string or array of strings"); + zend_argument_type_error(6, "option \"DISABLE_AUTHENTICATOR\" must be a string or an array of strings"); + RETURN_THROWS(); } } } } break; - case IS_LONG: default: - php_error_docref(NULL, E_WARNING, "Invalid argument, expect string or array of strings"); - break; + zend_argument_type_error(6, "option \"DISABLE_AUTHENTICATOR\" must be a string or an array of strings"); + RETURN_THROWS(); } } } @@ -780,11 +796,7 @@ static void php_imap_do_open(INTERNAL_FUNCTION_PARAMETERS, int persistent) #ifdef SET_MAXLOGINTRIALS if (argc >= 5) { - if (retries < 0) { - php_error_docref(NULL, E_WARNING ,"Retries must be greater or equal to 0"); - } else { - mail_parameters(NIL, SET_MAXLOGINTRIALS, (void *) retries); - } + mail_parameters(NIL, SET_MAXLOGINTRIALS, (void *) retries); } #endif @@ -805,13 +817,6 @@ static void php_imap_do_open(INTERNAL_FUNCTION_PARAMETERS, int persistent) } /* }}} */ -/* {{{ Open an IMAP stream to a mailbox */ -PHP_FUNCTION(imap_open) -{ - php_imap_do_open(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0); -} -/* }}} */ - /* {{{ Reopen an IMAP stream to a new mailbox */ PHP_FUNCTION(imap_reopen) { @@ -830,6 +835,17 @@ PHP_FUNCTION(imap_reopen) RETURN_THROWS(); } + if (options && ((options & ~(OP_READONLY | OP_ANONYMOUS | OP_HALFOPEN | OP_EXPUNGE | CL_EXPUNGE)) != 0)) { + zend_argument_value_error(3, "must be a bitmask of OP_READONLY, OP_ANONYMOUS, OP_HALFOPEN, " + "OP_EXPUNGE, and CL_EXPUNGE"); + RETURN_THROWS(); + } + + if (retries < 0) { + zend_argument_value_error(4, "must be greater than or equal to 0"); + RETURN_THROWS(); + } + if (options) { flags = options; if (flags & PHP_EXPUNGE) { @@ -888,11 +904,14 @@ PHP_FUNCTION(imap_append) 0, Z_L(0), Z_L(0)); if (!Z_LVAL_P(return_value)) { + // TODO Promoto to error? php_error_docref(NULL, E_WARNING, "Internal date not correctly formatted"); internal_date = NULL; } } + /* TODO Check if flags are valid (documentation is not present on php.net so need to check this first) */ + if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { RETURN_THROWS(); } @@ -1126,17 +1145,18 @@ PHP_FUNCTION(imap_gc) RETURN_THROWS(); } - if (flags && ((flags & ~(GC_TEXTS | GC_ELT | GC_ENV)) != 0)) { - php_error_docref(NULL, E_WARNING, "Invalid value for the flags parameter"); - RETURN_FALSE; + if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { + RETURN_THROWS(); } - if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { + if (flags && ((flags & ~(GC_TEXTS | GC_ELT | GC_ENV)) != 0)) { + zend_argument_value_error(2, "must be a bitmask of IMAP_GC_TEXTS, IMAP_GC_ELT, and IMAP_GC_ENV"); RETURN_THROWS(); } mail_gc(imap_le_struct->imap_stream, flags); + // TODO Return void? RETURN_TRUE; } /* }}} */ @@ -1147,9 +1167,9 @@ PHP_FUNCTION(imap_close) zval *streamind; pils *imap_le_struct=NULL; zend_long options = 0, flags = NIL; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "r|l", &streamind, &options) == FAILURE) { + /* TODO Change options to a boolean expunge flag? As it is the only valid flag. */ + if (zend_parse_parameters(ZEND_NUM_ARGS(), "r|l", &streamind, &options) == FAILURE) { RETURN_THROWS(); } @@ -1157,13 +1177,13 @@ PHP_FUNCTION(imap_close) RETURN_THROWS(); } - if (argc == 2) { + if (options) { flags = options; /* Check that flags is exactly equal to PHP_EXPUNGE or zero */ if (flags && ((flags & ~PHP_EXPUNGE) != 0)) { - php_error_docref(NULL, E_WARNING, "Invalid value for the flags parameter"); - RETURN_FALSE; + zend_argument_value_error(2, "must be CL_EXPUNGE or 0"); + RETURN_THROWS(); } /* Do the translation from PHP's internal PHP_EXPUNGE define to c-client's CL_EXPUNGE */ @@ -1176,6 +1196,7 @@ PHP_FUNCTION(imap_close) zend_list_close(Z_RES_P(streamind)); + // TODO Return void? RETURN_TRUE; } /* }}} */ @@ -1245,16 +1266,21 @@ PHP_FUNCTION(imap_body) RETURN_THROWS(); } - if (flags && ((flags & ~(FT_UID|FT_PEEK|FT_INTERNAL)) != 0)) { - php_error_docref(NULL, E_WARNING, "Invalid value for the options parameter"); - RETURN_FALSE; + if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { + RETURN_THROWS(); } - if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { + if (msgno < 1) { + zend_argument_value_error(2, "must be greater than 0"); RETURN_THROWS(); } - if ((argc == 3) && (flags & FT_UID)) { + if (flags && ((flags & ~(FT_UID|FT_PEEK|FT_INTERNAL)) != 0)) { + zend_argument_value_error(3, "must be a bitmask of FT_UID, FT_PEEK, and FT_INTERNAL"); + RETURN_THROWS(); + } + + if (flags && (flags & FT_UID)) { /* This should be cached; if it causes an extra RTT to the IMAP server, then that's the price we pay for making sure we don't crash. */ @@ -1262,11 +1288,11 @@ PHP_FUNCTION(imap_body) } else { msgindex = msgno; } - if ((msgindex < 1) || ((unsigned) msgindex > imap_le_struct->imap_stream->nmsgs)) { - php_error_docref(NULL, E_WARNING, "Bad message number"); - RETURN_FALSE; - } + // TODO int overflow as can be seen in imap_body.phpt will result in 999 => <0 trigerring a ValueError + PHP_IMAP_CHECK_MSGNO(msgindex, 2); + + /* TODO Shouldn't this pass msgindex??? */ body = mail_fetchtext_full (imap_le_struct->imap_stream, msgno, &body_len, (argc == 3 ? flags : NIL)); if (body_len == 0) { RETVAL_EMPTY_STRING(); @@ -1293,6 +1319,11 @@ PHP_FUNCTION(imap_mail_copy) RETURN_THROWS(); } + if (options && ((options & ~(CP_UID | CP_MOVE)) != 0)) { + zend_argument_value_error(4, "must be a bitmask of CP_UID, and CP_MOVE"); + RETURN_THROWS(); + } + if (mail_copy_full(imap_le_struct->imap_stream, ZSTR_VAL(seq), ZSTR_VAL(folder), (argc == 4 ? options : NIL)) == T) { RETURN_TRUE; } else { @@ -1318,6 +1349,11 @@ PHP_FUNCTION(imap_mail_move) RETURN_THROWS(); } + if (options && ((options & ~CP_UID) != 0)) { + zend_argument_value_error(4, "must be CP_UID or 0"); + RETURN_THROWS(); + } + if (mail_copy_full(imap_le_struct->imap_stream, ZSTR_VAL(seq), ZSTR_VAL(folder), (argc == 4 ? (options | CP_MOVE) : CP_MOVE)) == T) { RETURN_TRUE; } else { @@ -1552,12 +1588,13 @@ PHP_FUNCTION(imap_check) /* {{{ Mark a message for deletion */ PHP_FUNCTION(imap_delete) { - zval *streamind, *sequence; + zval *streamind; pils *imap_le_struct; + zend_string *sequence; zend_long flags = 0; int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rz|l", &streamind, &sequence, &flags) == FAILURE) { + if (zend_parse_parameters(argc, "rS|l", &streamind, &sequence, &flags) == FAILURE) { RETURN_THROWS(); } @@ -1565,11 +1602,12 @@ PHP_FUNCTION(imap_delete) RETURN_THROWS(); } - if (!try_convert_to_string(sequence)) { + if (flags && ((flags & ~FT_UID) != 0)) { + zend_argument_value_error(3, "must be FT_UID or 0"); RETURN_THROWS(); } - mail_setflag_full(imap_le_struct->imap_stream, Z_STRVAL_P(sequence), "\\DELETED", (argc == 3 ? flags : NIL)); + mail_setflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), "\\DELETED", (argc == 3 ? flags : NIL)); RETVAL_TRUE; } /* }}} */ @@ -1577,12 +1615,13 @@ PHP_FUNCTION(imap_delete) /* {{{ Remove the delete flag from a message */ PHP_FUNCTION(imap_undelete) { - zval *streamind, *sequence; + zval *streamind; + zend_string *sequence; zend_long flags = 0; pils *imap_le_struct; int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rz|l", &streamind, &sequence, &flags) == FAILURE) { + if (zend_parse_parameters(argc, "rS|l", &streamind, &sequence, &flags) == FAILURE) { RETURN_THROWS(); } @@ -1590,11 +1629,9 @@ PHP_FUNCTION(imap_undelete) RETURN_THROWS(); } - if (!try_convert_to_string(sequence)) { - RETURN_THROWS(); - } + /* TODO Check if flags are valid (documentation is not present on php.net so need to check this first) */ - mail_clearflag_full(imap_le_struct->imap_stream, Z_STRVAL_P(sequence), "\\DELETED", (argc == 3 ? flags : NIL)); + mail_clearflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), "\\DELETED", (argc == 3 ? flags : NIL)); RETVAL_TRUE; } /* }}} */ @@ -1604,14 +1641,13 @@ PHP_FUNCTION(imap_headerinfo) { zval *streamind; zend_string *defaulthost = NULL; - int argc = ZEND_NUM_ARGS(); - zend_long msgno, fromlength, subjectlength; + zend_long msgno, fromlength = 0, subjectlength = 0; pils *imap_le_struct; MESSAGECACHE *cache; ENVELOPE *en; char dummy[2000], fulladdress[MAILTMPLEN + 1]; - if (zend_parse_parameters(argc, "rl|llS", &streamind, &msgno, &fromlength, &subjectlength, &defaulthost) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rl|llS", &streamind, &msgno, &fromlength, &subjectlength, &defaulthost) == FAILURE) { RETURN_THROWS(); } @@ -1619,24 +1655,17 @@ PHP_FUNCTION(imap_headerinfo) RETURN_THROWS(); } - if (argc >= 3) { - if (fromlength < 0 || fromlength > MAILTMPLEN) { - php_error_docref(NULL, E_WARNING, "From length has to be between 0 and %d", MAILTMPLEN); - RETURN_FALSE; - } - } else { - fromlength = 0x00; - } - if (argc >= 4) { - if (subjectlength < 0 || subjectlength > MAILTMPLEN) { - php_error_docref(NULL, E_WARNING, "Subject length has to be between 0 and %d", MAILTMPLEN); - RETURN_FALSE; - } - } else { - subjectlength = 0x00; + PHP_IMAP_CHECK_MSGNO(msgno, 2); + + if (fromlength < 0 || fromlength > MAILTMPLEN) { + RETURN_THROWS(); + zend_argument_value_error(3, "must be between 0 and %d", MAILTMPLEN); } - PHP_IMAP_CHECK_MSGNO(msgno); + if (subjectlength < 0 || subjectlength > MAILTMPLEN) { + zend_argument_value_error(4, "must be between 0 and %d", MAILTMPLEN); + RETURN_THROWS(); + } if (mail_fetchstructure(imap_le_struct->imap_stream, msgno, NIL)) { cache = mail_elt(imap_le_struct->imap_stream, msgno); @@ -1688,13 +1717,12 @@ PHP_FUNCTION(imap_rfc822_parse_headers) { zend_string *headers, *defaulthost = NULL; ENVELOPE *en; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "S|S", &headers, &defaulthost) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|S", &headers, &defaulthost) == FAILURE) { RETURN_THROWS(); } - if (argc == 2) { + if (defaulthost) { rfc822_parse_msg(&en, NULL, ZSTR_VAL(headers), ZSTR_LEN(headers), NULL, ZSTR_VAL(defaulthost), NIL); } else { rfc822_parse_msg(&en, NULL, ZSTR_VAL(headers), ZSTR_LEN(headers), NULL, "UNKNOWN", NIL); @@ -1847,28 +1875,29 @@ PHP_FUNCTION(imap_fetchstructure) zend_long msgno, flags = 0; pils *imap_le_struct; BODY *body; - int msgindex, argc = ZEND_NUM_ARGS(); + int msgindex; - if (zend_parse_parameters(argc, "rl|l", &streamind, &msgno, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rl|l", &streamind, &msgno, &flags) == FAILURE) { RETURN_THROWS(); } - if (flags && ((flags & ~FT_UID) != 0)) { - php_error_docref(NULL, E_WARNING, "Invalid value for the options parameter"); - RETURN_FALSE; - } - if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { RETURN_THROWS(); } if (msgno < 1) { - RETURN_FALSE; + zend_argument_value_error(2, "must be greater than 0"); + RETURN_THROWS(); + } + + if (flags && ((flags & ~FT_UID) != 0)) { + zend_argument_value_error(3, "must be FT_UID or 0"); + RETURN_THROWS(); } object_init(return_value); - if ((argc == 3) && (flags & FT_UID)) { + if (flags & FT_UID) { /* This should be cached; if it causes an extra RTT to the IMAP server, then that's the price we pay for making sure we don't crash. */ @@ -1876,9 +1905,10 @@ PHP_FUNCTION(imap_fetchstructure) } else { msgindex = msgno; } - PHP_IMAP_CHECK_MSGNO(msgindex); + PHP_IMAP_CHECK_MSGNO(msgindex, 2); - mail_fetchstructure_full(imap_le_struct->imap_stream, msgno, &body , (argc == 3 ? flags : NIL)); + /* TODO Shouldn't this pass msgindex??? */ + mail_fetchstructure_full(imap_le_struct->imap_stream, msgno, &body , (ZEND_NUM_ARGS() == 3 ? flags : NIL)); if (!body) { php_error_docref(NULL, E_WARNING, "No body information available"); @@ -1904,18 +1934,23 @@ PHP_FUNCTION(imap_fetchbody) RETURN_THROWS(); } - if (flags && ((flags & ~(FT_UID|FT_PEEK|FT_INTERNAL)) != 0)) { - php_error_docref(NULL, E_WARNING, "Invalid value for the options parameter"); - RETURN_FALSE; + if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { + RETURN_THROWS(); } - if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { + if (msgno < 1) { + zend_argument_value_error(2, "must be greater than 0"); RETURN_THROWS(); } - if (argc < 4 || !(flags & FT_UID)) { + if (flags && ((flags & ~(FT_UID|FT_PEEK|FT_INTERNAL)) != 0)) { + zend_argument_value_error(4, "must be a bitmask of FT_UID, FT_PEEK, and FT_INTERNAL"); + RETURN_THROWS(); + } + + if (!(flags & FT_UID)) { /* only perform the check if the msgno is a message number and not a UID */ - PHP_IMAP_CHECK_MSGNO(msgno); + PHP_IMAP_CHECK_MSGNO(msgno, 2); } body = mail_fetchbody_full(imap_le_struct->imap_stream, msgno, ZSTR_VAL(sec), &len, (argc == 4 ? flags : NIL)); @@ -1945,18 +1980,23 @@ PHP_FUNCTION(imap_fetchmime) RETURN_THROWS(); } + if (msgno < 1) { + zend_argument_value_error(2, "must be greater than 0"); + RETURN_THROWS(); + } + if (flags && ((flags & ~(FT_UID|FT_PEEK|FT_INTERNAL)) != 0)) { - php_error_docref(NULL, E_WARNING, "Invalid value for the options parameter"); - RETURN_FALSE; + zend_argument_value_error(4, "must be a bitmask of FT_UID, FT_PEEK, and FT_INTERNAL"); + RETURN_THROWS(); } if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { RETURN_THROWS(); } - if (argc < 4 || !(flags & FT_UID)) { + if (!(flags & FT_UID)) { /* only perform the check if the msgno is a message number and not a UID */ - PHP_IMAP_CHECK_MSGNO(msgno); + PHP_IMAP_CHECK_MSGNO(msgno, 2); } body = mail_fetch_mime(imap_le_struct->imap_stream, msgno, ZSTR_VAL(sec), &len, (argc == 4 ? flags : NIL)); @@ -1974,7 +2014,7 @@ PHP_FUNCTION(imap_fetchmime) PHP_FUNCTION(imap_savebody) { zval *stream, *out; - pils *imap_ptr = NULL; + pils *imap_le_struct = NULL; php_stream *writer = NULL; zend_string *section = NULL; int close_stream = 1; @@ -1984,11 +2024,19 @@ PHP_FUNCTION(imap_savebody) RETURN_THROWS(); } - if ((imap_ptr = (pils *)zend_fetch_resource(Z_RES_P(stream), "imap", le_imap)) == NULL) { + if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(stream), "imap", le_imap)) == NULL) { + RETURN_THROWS(); + } + + PHP_IMAP_CHECK_MSGNO(msgno, 3); + + if (flags && ((flags & ~(FT_UID|FT_PEEK|FT_INTERNAL)) != 0)) { + zend_argument_value_error(5, "must be a bitmask of FT_UID, FT_PEEK, and FT_INTERNAL"); RETURN_THROWS(); } - if (!imap_ptr) { + // TODO Drop this? + if (!imap_le_struct) { RETURN_FALSE; } @@ -2004,6 +2052,7 @@ PHP_FUNCTION(imap_savebody) if (!try_convert_to_string(out)) { RETURN_THROWS(); } + // TODO Need to check for null bytes? writer = php_stream_open_wrapper(Z_STRVAL_P(out), "wb", REPORT_ERRORS, NULL); break; } @@ -2014,7 +2063,7 @@ PHP_FUNCTION(imap_savebody) IMAPG(gets_stream) = writer; mail_parameters(NIL, SET_GETS, (void *) php_mail_gets); - mail_fetchbody_full(imap_ptr->imap_stream, msgno, section?ZSTR_VAL(section):"", NULL, flags); + mail_fetchbody_full(imap_le_struct->imap_stream, msgno, section?ZSTR_VAL(section):"", NULL, flags); mail_parameters(NIL, SET_GETS, (void *) NULL); IMAPG(gets_stream) = NULL; @@ -2607,6 +2656,11 @@ PHP_FUNCTION(imap_setflag_full) RETURN_THROWS(); } + if (flags && ((flags & ~ST_UID) != 0)) { + zend_argument_value_error(4, "must be ST_UID or 0"); + RETURN_THROWS(); + } + mail_setflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), ZSTR_VAL(flag), (flags ? flags : NIL)); RETURN_TRUE; } @@ -2629,6 +2683,11 @@ PHP_FUNCTION(imap_clearflag_full) RETURN_THROWS(); } + if (flags && ((flags & ~ST_UID) != 0)) { + zend_argument_value_error(4, "must be ST_UID or 0"); + RETURN_THROWS(); + } + mail_clearflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), ZSTR_VAL(flag), (argc == 4 ? flags : NIL)); RETURN_TRUE; } @@ -2639,7 +2698,7 @@ PHP_FUNCTION(imap_sort) { zval *streamind; zend_string *criteria = NULL, *charset = NULL; - zend_long pgm, rev, flags = 0; + zend_long sort, rev, flags = 0; pils *imap_le_struct; unsigned long *slst, *sl; char *search_criteria; @@ -2647,7 +2706,8 @@ PHP_FUNCTION(imap_sort) SEARCHPGM *spg=NIL; int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rll|lSS", &streamind, &pgm, &rev, &flags, &criteria, &charset) == FAILURE) { + /* TODO Make reverse a boolean argument? */ + if (zend_parse_parameters(argc, "rll|lSS", &streamind, &sort, &rev, &flags, &criteria, &charset) == FAILURE) { RETURN_THROWS(); } @@ -2655,17 +2715,18 @@ PHP_FUNCTION(imap_sort) RETURN_THROWS(); } - if (pgm > SORTSIZE) { - php_error_docref(NULL, E_WARNING, "Unrecognized sort criteria"); - RETURN_FALSE; + if (!(sort == SORTDATE || sort == SORTARRIVAL || sort == SORTFROM || sort == SORTSUBJECT || sort == SORTTO || + sort == SORTCC || sort == SORTSIZE) ) { + zend_argument_value_error(2, "must be one of SORTDATE, SORTARRIVAL, SORTFROM, SORTSUBJECT, SORTTO, SORTCC, or SORTSIZE"); + RETURN_THROWS(); } - if (argc >= 4) { - if (flags < 0) { - php_error_docref(NULL, E_WARNING, "Search options parameter has to be greater than or equal to 0"); - RETURN_FALSE; - } + + if (flags && ((flags & ~(SE_UID|SE_NOPREFETCH )) != 0)) { + zend_argument_value_error(4, "must be a bitmask of SE_UID, and SE_NOPREFETCH"); + RETURN_THROWS(); } - if (argc >= 5) { + + if (criteria) { search_criteria = estrndup(ZSTR_VAL(criteria), ZSTR_LEN(criteria)); spg = mail_criteria(search_criteria); efree(search_criteria); @@ -2675,7 +2736,7 @@ PHP_FUNCTION(imap_sort) mypgm = mail_newsortpgm(); mypgm->reverse = rev; - mypgm->function = (short) pgm; + mypgm->function = (short) sort; mypgm->next = NIL; slst = mail_sort(imap_le_struct->imap_stream, (argc == 6 ? ZSTR_VAL(charset) : NIL), spg, mypgm, (argc >= 4 ? flags : NIL)); @@ -2700,22 +2761,24 @@ PHP_FUNCTION(imap_fetchheader) zval *streamind; zend_long msgno, flags = 0L; pils *imap_le_struct; - int msgindex, argc = ZEND_NUM_ARGS(); + int msgindex; - if (zend_parse_parameters(argc, "rl|l", &streamind, &msgno, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rl|l", &streamind, &msgno, &flags) == FAILURE) { RETURN_THROWS(); } - if (flags && ((flags & ~(FT_UID|FT_INTERNAL|FT_PREFETCHTEXT)) != 0)) { - php_error_docref(NULL, E_WARNING, "Invalid value for the options parameter"); - RETURN_FALSE; + if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { + RETURN_THROWS(); } - if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { + // TODO Check for msgno < 1 now or wait later for PHP_IMAP_CHECK_MSGNO check? + + if (flags && ((flags & ~(FT_UID|FT_INTERNAL|FT_PREFETCHTEXT)) != 0)) { + zend_argument_value_error(3, "must be a bitmask of FT_UID, FT_PREFETCHTEXT, and FT_INTERNAL"); RETURN_THROWS(); } - if ((argc == 3) && (flags & FT_UID)) { + if (flags & FT_UID) { /* This should be cached; if it causes an extra RTT to the IMAP server, then that's the price we pay for making sure we don't crash. */ @@ -2724,9 +2787,10 @@ PHP_FUNCTION(imap_fetchheader) msgindex = msgno; } - PHP_IMAP_CHECK_MSGNO(msgindex); + PHP_IMAP_CHECK_MSGNO(msgindex, 2); - RETVAL_STRING(mail_fetchheader_full(imap_le_struct->imap_stream, msgno, NIL, NIL, (argc == 3 ? flags : NIL))); + /* TODO Check shouldn't this pass msgindex???? */ + RETVAL_STRING(mail_fetchheader_full(imap_le_struct->imap_stream, msgno, NIL, NIL, (ZEND_NUM_ARGS() == 3 ? flags : NIL))); } /* }}} */ @@ -2736,7 +2800,6 @@ PHP_FUNCTION(imap_uid) zval *streamind; zend_long msgno; pils *imap_le_struct; - int msgindex; if (zend_parse_parameters(ZEND_NUM_ARGS(), "rl", &streamind, &msgno) == FAILURE) { RETURN_THROWS(); @@ -2746,11 +2809,7 @@ PHP_FUNCTION(imap_uid) RETURN_THROWS(); } - msgindex = msgno; - if ((msgindex < 1) || ((unsigned) msgindex > imap_le_struct->imap_stream->nmsgs)) { - php_error_docref(NULL, E_WARNING, "Bad message number"); - RETURN_FALSE; - } + PHP_IMAP_CHECK_MSGNO(msgno, 2); RETURN_LONG(mail_uid(imap_le_struct->imap_stream, msgno)); } @@ -2771,6 +2830,8 @@ PHP_FUNCTION(imap_msgno) RETURN_THROWS(); } + PHP_IMAP_CHECK_MSGNO(msgno, 2); + RETURN_LONG(mail_msgno(imap_le_struct->imap_stream, msgno)); } /* }}} */ @@ -2791,6 +2852,12 @@ PHP_FUNCTION(imap_status) RETURN_THROWS(); } + if (flags && ((flags & ~(SA_MESSAGES | SA_RECENT | SA_UNSEEN | SA_UIDNEXT | SA_UIDVALIDITY /*| SA_ALL*/)) != 0)) { + zend_argument_value_error(3, "must be a bitmask of SA_MESSAGES, SA_RECENT, SA_UNSEEN, " + "SA_UIDNEXT, SA_UIDVALIDITY, and SA_ALL"); + RETURN_THROWS(); + } + object_init(return_value); if (mail_status(imap_le_struct->imap_stream, ZSTR_VAL(mbx), flags)) { @@ -2820,14 +2887,14 @@ PHP_FUNCTION(imap_status) PHP_FUNCTION(imap_bodystruct) { zval *streamind; - zend_long msg; + zend_long msgno; zend_string *section; pils *imap_le_struct; zval parametres, param, dparametres, dparam; PARAMETER *par, *dpar; BODY *body; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "rlS", &streamind, &msg, §ion) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rlS", &streamind, &msgno, §ion) == FAILURE) { RETURN_THROWS(); } @@ -2835,13 +2902,9 @@ PHP_FUNCTION(imap_bodystruct) RETURN_THROWS(); } - if (!msg || msg < 1 || (unsigned) msg > imap_le_struct->imap_stream->nmsgs) { - php_error_docref(NULL, E_WARNING, "Bad message number"); - RETURN_FALSE; - } - + PHP_IMAP_CHECK_MSGNO(msgno, 2); - body=mail_body(imap_le_struct->imap_stream, msg, (unsigned char*)ZSTR_VAL(section)); + body=mail_body(imap_le_struct->imap_stream, msgno, (unsigned char*)ZSTR_VAL(section)); if (body == NULL) { RETURN_FALSE; } @@ -2937,15 +3000,14 @@ PHP_FUNCTION(imap_fetch_overview) zval myoverview; zend_string *address; zend_long status, flags = 0L; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rS|l", &streamind, &sequence, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rS|l", &streamind, &sequence, &flags) == FAILURE) { RETURN_THROWS(); } if (flags && ((flags & ~FT_UID) != 0)) { - php_error_docref(NULL, E_WARNING, "Invalid value for the options parameter"); - RETURN_FALSE; + zend_argument_value_error(3, "must be FT_UID or 0"); + RETURN_THROWS(); } if ((imap_le_struct = (pils *)zend_fetch_resource(Z_RES_P(streamind), "imap", le_imap)) == NULL) { @@ -3104,6 +3166,7 @@ PHP_FUNCTION(imap_mail_compose) 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"); RETURN_FALSE; } @@ -3296,6 +3359,7 @@ 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"); RETURN_FALSE; } @@ -3573,28 +3637,28 @@ int _php_imap_mail(char *to, char *subject, char *message, char *headers, char * PHP_FUNCTION(imap_mail) { zend_string *to=NULL, *message=NULL, *headers=NULL, *subject=NULL, *cc=NULL, *bcc=NULL, *rpath=NULL; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "SSS|SSSS", &to, &subject, &message, + if (zend_parse_parameters(ZEND_NUM_ARGS(), "SSS|SSSS", &to, &subject, &message, &headers, &cc, &bcc, &rpath) == FAILURE) { RETURN_THROWS(); } /* To: */ - if (!ZSTR_LEN(to)) { - php_error_docref(NULL, E_WARNING, "No to field in mail command"); - RETURN_FALSE; + if (ZSTR_LEN(to) == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); } /* Subject: */ - if (!ZSTR_LEN(subject)) { - php_error_docref(NULL, E_WARNING, "No subject field in mail command"); - RETURN_FALSE; + if (ZSTR_LEN(subject) == 0) { + zend_argument_value_error(2, "cannot be empty"); + RETURN_THROWS(); } /* message body */ - if (!ZSTR_LEN(message)) { + if (ZSTR_LEN(message) == 0) { /* this is not really an error, so it is allowed. */ + // TODO Really???? php_error_docref(NULL, E_WARNING, "No message string in mail command"); } @@ -3627,6 +3691,12 @@ PHP_FUNCTION(imap_search) RETURN_THROWS(); } + /* TODO Update docs to allow SE_FREE as an option */ + if (flags && ((flags & ~(SE_FREE | SE_UID)) != 0)) { + zend_argument_value_error(3, "must be a bitmask of SE_FREE, and SE_UID"); + RETURN_THROWS(); + } + search_criteria = estrndup(ZSTR_VAL(criteria), ZSTR_LEN(criteria)); IMAPG(imap_messages) = IMAPG(imap_messages_tail) = NIL; @@ -4205,10 +4275,9 @@ PHP_FUNCTION(imap_thread) zend_long flags = SE_FREE; char criteria[] = "ALL"; THREADNODE *top; - int argc = ZEND_NUM_ARGS(); SEARCHPGM *pgm = NIL; - if (zend_parse_parameters(argc, "r|l", &streamind, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "r|l", &streamind, &flags) == FAILURE) { RETURN_THROWS(); } @@ -4216,19 +4285,21 @@ PHP_FUNCTION(imap_thread) RETURN_THROWS(); } + /* TODO Check if flags are valid (documentation is not present on php.net so need to check this first) */ + pgm = mail_criteria(criteria); top = mail_thread(imap_le_struct->imap_stream, "REFERENCES", NIL, pgm, flags); if (pgm && !(flags & SE_FREE)) { mail_free_searchpgm(&pgm); } - if(top == NIL) { + if (top == NIL) { php_error_docref(NULL, E_WARNING, "Function returned an empty tree"); RETURN_FALSE; } /* Populate our return value data structure here. */ - if(build_thread_tree(top, &return_value) == FAILURE) { + if (build_thread_tree(top, &return_value) == FAILURE) { mail_free_threadnode(&top); RETURN_FALSE; } diff --git a/ext/imap/tests/imap_body.phpt b/ext/imap/tests/imap_body.phpt index 4d4133c343fc7..179a263fafe9a 100644 --- a/ext/imap/tests/imap_body.phpt +++ b/ext/imap/tests/imap_body.phpt @@ -1,5 +1,5 @@ --TEST-- -imap_body() incorrect parameter count +imap_body() ValueError --CREDITS-- Paul Sohier #phptestfest utrecht @@ -7,6 +7,8 @@ Paul Sohier +--XFAIL-- +zend_long overflow when trying to fit integer into int --FILE-- getMessage() . \PHP_EOL; +} +try { + imap_body($stream_id,1,-1); +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} //Access not existing var_dump(imap_body($stream_id, 999, FT_UID)); @@ -24,9 +34,8 @@ imap_close($stream_id); ?> --EXPECTF-- -Warning: imap_body(): Bad message number in %s on line %d - -Warning: imap_body(): Invalid value for the options parameter in %s on line %d +imap_body(): Argument #2 ($msg_no) must be greater than 0 +imap_body(): Argument #3 ($options) must be a bitmask of FT_UID, FT_PEEK, and FT_INTERNAL Warning: imap_body(): Bad message number in %s on line %d bool(false) diff --git a/ext/imap/tests/imap_close_variation4.phpt b/ext/imap/tests/imap_close_variation4.phpt index c6f13678cc5b9..56e3722e9e382 100644 --- a/ext/imap/tests/imap_close_variation4.phpt +++ b/ext/imap/tests/imap_close_variation4.phpt @@ -28,7 +28,12 @@ foreach($inputs as $input) { imap_delete($stream_id, $i); } echo "\n-- Iteration $iterator --\n"; - var_dump( $check = imap_close($stream_id, $input) ); + try { + var_dump( $check = imap_close($stream_id, $input) ); + } catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; + $check = false; + } // check that imap_close was successful, if not call imap_close and explicitly set CL_EXPUNGE if(false === $check) { @@ -71,16 +76,10 @@ bool(true) CL_EXPUNGE was set -- Iteration 3 -- - -Warning: imap_close(): Invalid value for the flags parameter in %s on line %d -bool(false) +imap_close(): Argument #2 ($options) must be CL_EXPUNGE or 0 -- Iteration 4 -- - -Warning: imap_close(): Invalid value for the flags parameter in %s on line %d -bool(false) +imap_close(): Argument #2 ($options) must be CL_EXPUNGE or 0 -- Iteration 5 -- - -Warning: imap_close(): Invalid value for the flags parameter in %s on line %d -bool(false) +imap_close(): Argument #2 ($options) must be CL_EXPUNGE or 0 diff --git a/ext/imap/tests/imap_fetch_overview_variation3.phpt b/ext/imap/tests/imap_fetch_overview_variation3.phpt index 419de4c95152f..2aa832c0d6d0c 100644 --- a/ext/imap/tests/imap_fetch_overview_variation3.phpt +++ b/ext/imap/tests/imap_fetch_overview_variation3.phpt @@ -25,21 +25,21 @@ $options = array ('1', true, 1.000000000000001, 0.00001e5, - PHP_INT_MAX, - -PHP_INT_MAX + 245 ); -// iterate over each element of $options array -$iterator = 1; imap_check($stream_id); foreach($options as $option) { echo "\nTesting with option value:"; var_dump($option); - $overview = imap_fetch_overview($stream_id, $msg_uid, $option); - if ($overview) { - echo "imap_fetch_overview() returns an object\n"; + try { + $overview = imap_fetch_overview($stream_id, $msg_uid, $option); + if ($overview) { + echo "imap_fetch_overview() returns an object\n"; } - $iterator++; + } catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; + } } ?> @@ -47,10 +47,10 @@ foreach($options as $option) { ---EXPECTF-- +--EXPECT-- *** Testing imap_fetch_overview() : usage variations *** Create a temporary mailbox and add 1 msgs -.. mailbox '{%s}%s' created +.. mailbox '{127.0.0.1:143/norsh}INBOX.phpttest' created Testing with option value:string(1) "1" imap_fetch_overview() returns an object @@ -64,10 +64,5 @@ imap_fetch_overview() returns an object Testing with option value:float(1) imap_fetch_overview() returns an object -Testing with option value:int(%d) - -Warning: imap_fetch_overview(): Invalid value for the options parameter in %s on line %d - -Testing with option value:int(-%d) - -Warning: imap_fetch_overview(): Invalid value for the options parameter in %s on line %d +Testing with option value:int(245) +imap_fetch_overview(): Argument #3 ($options) must be FT_UID or 0 diff --git a/ext/imap/tests/imap_fetchbody_variation4.phpt b/ext/imap/tests/imap_fetchbody_variation4.phpt index 45158a9ed0295..055e1b06f74b4 100644 --- a/ext/imap/tests/imap_fetchbody_variation4.phpt +++ b/ext/imap/tests/imap_fetchbody_variation4.phpt @@ -31,11 +31,16 @@ $iterator = 1; imap_check($stream_id); foreach($options as $option) { echo "\n-- Iteration $iterator --\n"; - if(is_string(imap_fetchbody($stream_id, $msg_uid, $section, $option))) { - echo "FT_UID valid\n"; - } else { - echo "FT_UID not valid\n"; + + try { + if(is_string(imap_fetchbody($stream_id, $msg_uid, $section, $option))) { + echo "FT_UID valid\n"; + } else { + echo "FT_UID not valid\n"; } + } catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; + } $iterator++; } @@ -44,10 +49,10 @@ foreach($options as $option) { ---EXPECTF-- +--EXPECT-- *** Testing imap_fetchbody() : usage variations *** Create a temporary mailbox and add 1 msgs -.. mailbox '{%s}%s' created +.. mailbox '{127.0.0.1:143/norsh}INBOX.phpttest' created -- Iteration 1 -- FT_UID valid @@ -62,11 +67,7 @@ FT_UID valid FT_UID valid -- Iteration 5 -- - -Warning: imap_fetchbody(): Invalid value for the options parameter in %s on line %d -FT_UID not valid +imap_fetchbody(): Argument #4 ($options) must be a bitmask of FT_UID, FT_PEEK, and FT_INTERNAL -- Iteration 6 -- - -Warning: imap_fetchbody(): Invalid value for the options parameter in %s on line %d -FT_UID not valid +imap_fetchbody(): Argument #4 ($options) must be a bitmask of FT_UID, FT_PEEK, and FT_INTERNAL diff --git a/ext/imap/tests/imap_fetchbody_variation6.phpt b/ext/imap/tests/imap_fetchbody_variation6.phpt index 75516f8381454..fd5a898b680d9 100644 --- a/ext/imap/tests/imap_fetchbody_variation6.phpt +++ b/ext/imap/tests/imap_fetchbody_variation6.phpt @@ -23,9 +23,10 @@ $sequences = [0, /* out of range */ 4, 1]; foreach($sequences as $msg_no) { echo "\n-- \$msg_no is $msg_no --\n"; - var_dump($overview = imap_fetchbody($stream_id, $msg_no, $section)); - if (!$overview) { - echo imap_last_error() . "\n"; + try { + var_dump(imap_fetchbody($stream_id, $msg_no, $section)); + } catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; } } ?> @@ -39,17 +40,13 @@ Create a temporary mailbox and add 3 msgs .. mailbox '{%s}%s' created -- $msg_no is 0 -- - -Warning: imap_fetchbody(): Bad message number in %s on line %d -bool(false) - +imap_fetchbody(): Argument #2 ($msg_no) must be greater than 0 -- $msg_no is 4 -- Warning: imap_fetchbody(): Bad message number in %s on line %d bool(false) - -- $msg_no is 1 -- string(42) "1: this is a test message, please ignore " diff --git a/ext/imap/tests/imap_fetchheader_variation3.phpt b/ext/imap/tests/imap_fetchheader_variation3.phpt index 8f6b5b5a12191..5347a19ffc0d6 100644 --- a/ext/imap/tests/imap_fetchheader_variation3.phpt +++ b/ext/imap/tests/imap_fetchheader_variation3.phpt @@ -30,11 +30,15 @@ $iterator = 1; imap_check($stream_id); foreach($options as $option) { echo "\n-- Iteration $iterator --\n"; - if(is_string(imap_fetchheader($stream_id, $msg_uid, $option))) { - echo "FT_UID valid\n"; - } else { - echo "FT_UID not valid\n"; + try { + if (is_string(imap_fetchheader($stream_id, $msg_uid, $option))) { + echo "FT_UID valid\n"; + } else { + echo "FT_UID not valid\n"; } + } catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; + } $iterator++; } ?> @@ -42,10 +46,10 @@ foreach($options as $option) { ---EXPECTF-- +--EXPECT-- *** Testing imap_fetchheader() : usage variations *** Create a temporary mailbox and add 1 msgs -.. mailbox '{%s}%s' created +.. mailbox '{127.0.0.1:143/norsh}INBOX.phpttest' created -- Iteration 1 -- FT_UID valid @@ -60,11 +64,7 @@ FT_UID valid FT_UID valid -- Iteration 5 -- - -Warning: imap_fetchheader(): Invalid value for the options parameter in %s on line %d -FT_UID not valid +imap_fetchheader(): Argument #3 ($options) must be a bitmask of FT_UID, FT_PREFETCHTEXT, and FT_INTERNAL -- Iteration 6 -- - -Warning: imap_fetchheader(): Invalid value for the options parameter in %s on line %d -FT_UID not valid +imap_fetchheader(): Argument #3 ($options) must be a bitmask of FT_UID, FT_PREFETCHTEXT, and FT_INTERNAL diff --git a/ext/imap/tests/imap_fetchheader_variation5.phpt b/ext/imap/tests/imap_fetchheader_variation5.phpt index 2283861df8e8a..7e45f1ba46ac9 100644 --- a/ext/imap/tests/imap_fetchheader_variation5.phpt +++ b/ext/imap/tests/imap_fetchheader_variation5.phpt @@ -21,9 +21,10 @@ $sequences = [0, /* out of range */ 4, 1]; foreach($sequences as $msg_no) { echo "\n-- \$msg_no is $msg_no --\n"; - var_dump($overview = imap_fetchheader($stream_id, $msg_no)); - if (!$overview) { - echo imap_last_error() . "\n"; + try { + var_dump(imap_fetchheader($stream_id, $msg_no)); + } catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; } } @@ -40,17 +41,13 @@ Create a temporary mailbox and add 3 msgs .. mailbox '{%s}%s' created -- $msg_no is 0 -- - -Warning: imap_fetchheader(): Bad message number in %s on line %d -bool(false) - +imap_fetchheader(): Argument #2 ($msg_no) must be greater than 0 -- $msg_no is 4 -- Warning: imap_fetchheader(): Bad message number in %s on line %d bool(false) - -- $msg_no is 1 -- string(%d) "From: foo@anywhere.com Subject: Test msg 1 diff --git a/ext/imap/tests/imap_fetchstructure_basic.phpt b/ext/imap/tests/imap_fetchstructure_basic.phpt index 572556e6e13d1..11c1f7efb253c 100644 --- a/ext/imap/tests/imap_fetchstructure_basic.phpt +++ b/ext/imap/tests/imap_fetchstructure_basic.phpt @@ -12,7 +12,11 @@ require_once(__DIR__.'/skipif.inc'); require_once(__DIR__.'/imap_include.inc'); $stream_id = setup_test_mailbox('', 1); -imap_fetchstructure($stream_id,0); +try { + imap_fetchstructure($stream_id,0); +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} $z = imap_fetchstructure($stream_id,1); @@ -39,6 +43,7 @@ require_once('clean.inc'); --EXPECTF-- Create a temporary mailbox and add 1 msgs .. mailbox '{127.0.0.1:143/norsh}INBOX.phpttest' created +imap_fetchstructure(): Argument #2 ($msg_no) must be greater than 0 bool(true) bool(true) bool(true) diff --git a/ext/imap/tests/imap_gc_error.phpt b/ext/imap/tests/imap_gc_error.phpt index 072af103930cb..5b5b65d271422 100644 --- a/ext/imap/tests/imap_gc_error.phpt +++ b/ext/imap/tests/imap_gc_error.phpt @@ -13,8 +13,13 @@ require_once(__DIR__.'/skipif.inc'); require_once(__DIR__.'/imap_include.inc'); $stream_id = imap_open($default_mailbox, $username, $password) or die("Cannot connect to mailbox $default_mailbox: " . imap_last_error()); -imap_gc($stream_id, -1); + +try { + imap_gc($stream_id, -1); +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} ?> ---EXPECTF-- -Warning: imap_gc(): Invalid value for the flags parameter in %s on line %d +--EXPECT-- +imap_gc(): Argument #2 ($flags) must be a bitmask of IMAP_GC_TEXTS, IMAP_GC_ELT, and IMAP_GC_ENV diff --git a/ext/imap/tests/imap_open_error.phpt b/ext/imap/tests/imap_open_error.phpt index f9410519f2159..dc335c9b85e7e 100644 --- a/ext/imap/tests/imap_open_error.phpt +++ b/ext/imap/tests/imap_open_error.phpt @@ -1,5 +1,5 @@ --TEST-- -imap_open() incorrect parameter count +imap_open() ValueErrors --CREDITS-- Paul Sohier #phptestfest utrecht @@ -12,19 +12,27 @@ require_once(__DIR__.'/skipif.inc'); echo "Checking with incorrect parameters\n" ; imap_open('', '', ''); -imap_open('', '', '', -1); + +try { + imap_open('', '', '', -1); +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} require_once(__DIR__.'/imap_include.inc'); -imap_open($default_mailbox, $username, $password, NIL, -1); + +try { + imap_open($default_mailbox, $username, $password, NIL, -1); +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} ?> --EXPECTF-- Checking with incorrect parameters Warning: imap_open(): Couldn't open stream in %s on line %d - -Warning: imap_open(): Couldn't open stream in %s on line %d - -Warning: imap_open(): Retries must be greater or equal to 0 in %s on line %d +imap_open(): Argument #4 ($options) must be a bitmask of OP_READONLY, OP_ANONYMOUS, OP_HALFOPEN, CL_EXPUNGE, OP_DEBUG, OP_SHORTCACHE, OP_SILENT, OP_PROTOTYPE, and OP_SECURE +imap_open(): Argument #5 ($n_retries) must be greater than or equal to 0 Notice: Unknown: Can't open mailbox : no such mailbox (errflg=2) in Unknown on line 0 From f2ff235508af110753b0da3c9e8ae960dcdd5e32 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 21 Sep 2020 15:02:42 +0100 Subject: [PATCH 2/4] Drop some unnecessary comparisons as NIL == 0 --- ext/imap/php_imap.c | 58 +++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/ext/imap/php_imap.c b/ext/imap/php_imap.c index 62e91f3c77fe6..87fd48e6f8619 100644 --- a/ext/imap/php_imap.c +++ b/ext/imap/php_imap.c @@ -704,13 +704,12 @@ PHP_MINFO_FUNCTION(imap) PHP_FUNCTION(imap_open) { zend_string *mailbox, *user, *passwd; - zend_long retries = 0, flags = NIL, cl_flags = NIL; + zend_long retries = 0, flags = 0, cl_flags = 0; MAILSTREAM *imap_stream; pils *imap_le_struct; HashTable *params = NULL; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "PSS|llh", &mailbox, &user, &passwd, &flags, &retries, ¶ms) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "PSS|llh", &mailbox, &user, &passwd, &flags, &retries, ¶ms) == FAILURE) { RETURN_THROWS(); } @@ -726,7 +725,7 @@ PHP_FUNCTION(imap_open) RETURN_THROWS(); } - if (argc >= 4) { + if (flags) { if (flags & PHP_EXPUNGE) { cl_flags = CL_EXPUNGE; flags ^= PHP_EXPUNGE; @@ -795,7 +794,7 @@ PHP_FUNCTION(imap_open) IMAPG(imap_password) = estrndup(ZSTR_VAL(passwd), ZSTR_LEN(passwd)); #ifdef SET_MAXLOGINTRIALS - if (argc >= 5) { + if (retries) { mail_parameters(NIL, SET_MAXLOGINTRIALS, (void *) retries); } #endif @@ -1168,7 +1167,6 @@ PHP_FUNCTION(imap_close) pils *imap_le_struct=NULL; zend_long options = 0, flags = NIL; - /* TODO Change options to a boolean expunge flag? As it is the only valid flag. */ if (zend_parse_parameters(ZEND_NUM_ARGS(), "r|l", &streamind, &options) == FAILURE) { RETURN_THROWS(); } @@ -1258,11 +1256,11 @@ PHP_FUNCTION(imap_body) zval *streamind; zend_long msgno, flags = 0; pils *imap_le_struct; - int msgindex, argc = ZEND_NUM_ARGS(); + int msgindex; char *body; unsigned long body_len = 0; - if (zend_parse_parameters(argc, "rl|l", &streamind, &msgno, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rl|l", &streamind, &msgno, &flags) == FAILURE) { RETURN_THROWS(); } @@ -1293,7 +1291,7 @@ PHP_FUNCTION(imap_body) PHP_IMAP_CHECK_MSGNO(msgindex, 2); /* TODO Shouldn't this pass msgindex??? */ - body = mail_fetchtext_full (imap_le_struct->imap_stream, msgno, &body_len, (argc == 3 ? flags : NIL)); + body = mail_fetchtext_full (imap_le_struct->imap_stream, msgno, &body_len, flags); if (body_len == 0) { RETVAL_EMPTY_STRING(); } else { @@ -1308,10 +1306,9 @@ PHP_FUNCTION(imap_mail_copy) zval *streamind; zend_long options = 0; zend_string *seq, *folder; - int argc = ZEND_NUM_ARGS(); pils *imap_le_struct; - if (zend_parse_parameters(argc, "rSS|l", &streamind, &seq, &folder, &options) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rSS|l", &streamind, &seq, &folder, &options) == FAILURE) { RETURN_THROWS(); } @@ -1324,7 +1321,7 @@ PHP_FUNCTION(imap_mail_copy) RETURN_THROWS(); } - if (mail_copy_full(imap_le_struct->imap_stream, ZSTR_VAL(seq), ZSTR_VAL(folder), (argc == 4 ? options : NIL)) == T) { + if (mail_copy_full(imap_le_struct->imap_stream, ZSTR_VAL(seq), ZSTR_VAL(folder), options) == T) { RETURN_TRUE; } else { RETURN_FALSE; @@ -1339,9 +1336,8 @@ PHP_FUNCTION(imap_mail_move) zend_string *seq, *folder; zend_long options = 0; pils *imap_le_struct; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rSS|l", &streamind, &seq, &folder, &options) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rSS|l", &streamind, &seq, &folder, &options) == FAILURE) { RETURN_THROWS(); } @@ -1354,7 +1350,10 @@ PHP_FUNCTION(imap_mail_move) RETURN_THROWS(); } - if (mail_copy_full(imap_le_struct->imap_stream, ZSTR_VAL(seq), ZSTR_VAL(folder), (argc == 4 ? (options | CP_MOVE) : CP_MOVE)) == T) { + /* Add CP_MOVE flag */ + options = (options | CP_MOVE); + + if (mail_copy_full(imap_le_struct->imap_stream, ZSTR_VAL(seq), ZSTR_VAL(folder), options) == T) { RETURN_TRUE; } else { RETURN_FALSE; @@ -1592,9 +1591,8 @@ PHP_FUNCTION(imap_delete) pils *imap_le_struct; zend_string *sequence; zend_long flags = 0; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rS|l", &streamind, &sequence, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rS|l", &streamind, &sequence, &flags) == FAILURE) { RETURN_THROWS(); } @@ -1607,7 +1605,7 @@ PHP_FUNCTION(imap_delete) RETURN_THROWS(); } - mail_setflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), "\\DELETED", (argc == 3 ? flags : NIL)); + mail_setflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), "\\DELETED", flags); RETVAL_TRUE; } /* }}} */ @@ -1619,9 +1617,8 @@ PHP_FUNCTION(imap_undelete) zend_string *sequence; zend_long flags = 0; pils *imap_le_struct; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rS|l", &streamind, &sequence, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rS|l", &streamind, &sequence, &flags) == FAILURE) { RETURN_THROWS(); } @@ -1631,7 +1628,9 @@ PHP_FUNCTION(imap_undelete) /* TODO Check if flags are valid (documentation is not present on php.net so need to check this first) */ - mail_clearflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), "\\DELETED", (argc == 3 ? flags : NIL)); + mail_clearflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), "\\DELETED", flags); + + // TODO Return void? RETVAL_TRUE; } /* }}} */ @@ -1928,9 +1927,8 @@ PHP_FUNCTION(imap_fetchbody) char *body; zend_string *sec; unsigned long len; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rlS|l", &streamind, &msgno, &sec, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rlS|l", &streamind, &msgno, &sec, &flags) == FAILURE) { RETURN_THROWS(); } @@ -1953,7 +1951,7 @@ PHP_FUNCTION(imap_fetchbody) PHP_IMAP_CHECK_MSGNO(msgno, 2); } - body = mail_fetchbody_full(imap_le_struct->imap_stream, msgno, ZSTR_VAL(sec), &len, (argc == 4 ? flags : NIL)); + body = mail_fetchbody_full(imap_le_struct->imap_stream, msgno, ZSTR_VAL(sec), &len, flags); if (!body) { php_error_docref(NULL, E_WARNING, "No body information available"); @@ -1974,9 +1972,8 @@ PHP_FUNCTION(imap_fetchmime) char *body; zend_string *sec; unsigned long len; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rlS|l", &streamind, &msgno, &sec, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rlS|l", &streamind, &msgno, &sec, &flags) == FAILURE) { RETURN_THROWS(); } @@ -1999,7 +1996,7 @@ PHP_FUNCTION(imap_fetchmime) PHP_IMAP_CHECK_MSGNO(msgno, 2); } - body = mail_fetch_mime(imap_le_struct->imap_stream, msgno, ZSTR_VAL(sec), &len, (argc == 4 ? flags : NIL)); + body = mail_fetch_mime(imap_le_struct->imap_stream, msgno, ZSTR_VAL(sec), &len, flags); if (!body) { php_error_docref(NULL, E_WARNING, "No body MIME information available"); @@ -2673,9 +2670,8 @@ PHP_FUNCTION(imap_clearflag_full) zend_string *sequence, *flag; zend_long flags = 0; pils *imap_le_struct; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "rSS|l", &streamind, &sequence, &flag, &flags) ==FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "rSS|l", &streamind, &sequence, &flag, &flags) ==FAILURE) { RETURN_THROWS(); } @@ -2688,7 +2684,7 @@ PHP_FUNCTION(imap_clearflag_full) RETURN_THROWS(); } - mail_clearflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), ZSTR_VAL(flag), (argc == 4 ? flags : NIL)); + mail_clearflag_full(imap_le_struct->imap_stream, ZSTR_VAL(sequence), ZSTR_VAL(flag), flags); RETURN_TRUE; } /* }}} */ @@ -2739,7 +2735,7 @@ PHP_FUNCTION(imap_sort) mypgm->function = (short) sort; mypgm->next = NIL; - slst = mail_sort(imap_le_struct->imap_stream, (argc == 6 ? ZSTR_VAL(charset) : NIL), spg, mypgm, (argc >= 4 ? flags : NIL)); + slst = mail_sort(imap_le_struct->imap_stream, (argc == 6 ? ZSTR_VAL(charset) : NIL), spg, mypgm, flags); if (spg && !(flags & SE_FREE)) { mail_free_searchpgm(&spg); From 7b11212c8ee208e7baa450de7c580ce42dbf75af Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 21 Sep 2020 15:19:00 +0100 Subject: [PATCH 3/4] Specific warning if UID message number does not exist --- ext/imap/php_imap.c | 17 ++++++++++++++--- ext/imap/tests/imap_body.phpt | 6 ++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/ext/imap/php_imap.c b/ext/imap/php_imap.c index 87fd48e6f8619..b64ecee7a039a 100644 --- a/ext/imap/php_imap.c +++ b/ext/imap/php_imap.c @@ -1256,7 +1256,7 @@ PHP_FUNCTION(imap_body) zval *streamind; zend_long msgno, flags = 0; pils *imap_le_struct; - int msgindex; + unsigned long msgindex; char *body; unsigned long body_len = 0; @@ -1283,11 +1283,14 @@ PHP_FUNCTION(imap_body) IMAP server, then that's the price we pay for making sure we don't crash. */ msgindex = mail_msgno(imap_le_struct->imap_stream, msgno); + if (msgindex == 0) { + php_error_docref(NULL, E_WARNING, "UID does not exist"); + RETURN_FALSE; + } } else { - msgindex = msgno; + msgindex = (unsigned long) msgno; } - // TODO int overflow as can be seen in imap_body.phpt will result in 999 => <0 trigerring a ValueError PHP_IMAP_CHECK_MSGNO(msgindex, 2); /* TODO Shouldn't this pass msgindex??? */ @@ -1901,6 +1904,10 @@ PHP_FUNCTION(imap_fetchstructure) IMAP server, then that's the price we pay for making sure we don't crash. */ msgindex = mail_msgno(imap_le_struct->imap_stream, msgno); + if (msgindex == 0) { + php_error_docref(NULL, E_WARNING, "UID does not exist"); + RETURN_FALSE; + } } else { msgindex = msgno; } @@ -2779,6 +2786,10 @@ PHP_FUNCTION(imap_fetchheader) IMAP server, then that's the price we pay for making sure we don't crash. */ msgindex = mail_msgno(imap_le_struct->imap_stream, msgno); + if (msgindex == 0) { + php_error_docref(NULL, E_WARNING, "UID does not exist"); + RETURN_FALSE; + } } else { msgindex = msgno; } diff --git a/ext/imap/tests/imap_body.phpt b/ext/imap/tests/imap_body.phpt index 179a263fafe9a..752ca1c39db77 100644 --- a/ext/imap/tests/imap_body.phpt +++ b/ext/imap/tests/imap_body.phpt @@ -7,8 +7,6 @@ Paul Sohier ---XFAIL-- -zend_long overflow when trying to fit integer into int --FILE-- Date: Mon, 21 Sep 2020 15:23:32 +0100 Subject: [PATCH 4/4] Amend error messages --- ext/imap/php_imap.c | 8 +++----- ext/imap/tests/imap_open_error.phpt | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ext/imap/php_imap.c b/ext/imap/php_imap.c index b64ecee7a039a..c42bd62e63ca5 100644 --- a/ext/imap/php_imap.c +++ b/ext/imap/php_imap.c @@ -715,8 +715,7 @@ PHP_FUNCTION(imap_open) if (flags && ((flags & ~(OP_READONLY | OP_ANONYMOUS | OP_HALFOPEN | CL_EXPUNGE | OP_DEBUG | OP_SHORTCACHE | OP_SILENT | OP_PROTOTYPE | OP_SECURE)) != 0)) { - zend_argument_value_error(4, "must be a bitmask of OP_READONLY, OP_ANONYMOUS, OP_HALFOPEN, CL_EXPUNGE, " - "OP_DEBUG, OP_SHORTCACHE, OP_SILENT, OP_PROTOTYPE, and OP_SECURE"); + zend_argument_value_error(4, "must be a bitmask of the OP_* constants, and CL_EXPUNGE"); RETURN_THROWS(); } @@ -2720,7 +2719,7 @@ PHP_FUNCTION(imap_sort) if (!(sort == SORTDATE || sort == SORTARRIVAL || sort == SORTFROM || sort == SORTSUBJECT || sort == SORTTO || sort == SORTCC || sort == SORTSIZE) ) { - zend_argument_value_error(2, "must be one of SORTDATE, SORTARRIVAL, SORTFROM, SORTSUBJECT, SORTTO, SORTCC, or SORTSIZE"); + zend_argument_value_error(2, "must be one of the SORT* constants"); RETURN_THROWS(); } @@ -2860,8 +2859,7 @@ PHP_FUNCTION(imap_status) } if (flags && ((flags & ~(SA_MESSAGES | SA_RECENT | SA_UNSEEN | SA_UIDNEXT | SA_UIDVALIDITY /*| SA_ALL*/)) != 0)) { - zend_argument_value_error(3, "must be a bitmask of SA_MESSAGES, SA_RECENT, SA_UNSEEN, " - "SA_UIDNEXT, SA_UIDVALIDITY, and SA_ALL"); + zend_argument_value_error(3, "must be a bitmask of SA_* constants"); RETURN_THROWS(); } diff --git a/ext/imap/tests/imap_open_error.phpt b/ext/imap/tests/imap_open_error.phpt index dc335c9b85e7e..aa964c32f11c3 100644 --- a/ext/imap/tests/imap_open_error.phpt +++ b/ext/imap/tests/imap_open_error.phpt @@ -32,7 +32,7 @@ try { Checking with incorrect parameters Warning: imap_open(): Couldn't open stream in %s on line %d -imap_open(): Argument #4 ($options) must be a bitmask of OP_READONLY, OP_ANONYMOUS, OP_HALFOPEN, CL_EXPUNGE, OP_DEBUG, OP_SHORTCACHE, OP_SILENT, OP_PROTOTYPE, and OP_SECURE +imap_open(): Argument #4 ($options) must be a bitmask of the OP_* constants, and CL_EXPUNGE imap_open(): Argument #5 ($n_retries) must be greater than or equal to 0 Notice: Unknown: Can't open mailbox : no such mailbox (errflg=2) in Unknown on line 0