]> granicus.if.org Git - php/commitdiff
Promote warnings in ext/zip
authorGeorge Peter Banyard <girgias@php.net>
Wed, 8 Jul 2020 13:51:28 +0000 (15:51 +0200)
committerRemi Collet <remi@php.net>
Mon, 17 Aug 2020 13:52:20 +0000 (15:52 +0200)
ext/zip/php_zip.c
ext/zip/tests/oo_getcomment.phpt
ext/zip/tests/oo_open.phpt
ext/zip/tests/oo_setcomment_error.phpt
ext/zip/tests/zip_open_error.phpt

index de3a695d96401fd1b903da2dcf0e717cf2b46777..43c2da9af66605e8038811cdc1ee3d3075ba7ca1 100644 (file)
@@ -52,11 +52,12 @@ static int le_zip_entry;
        }
 /* }}} */
 
-/* {{{  PHP_ZIP_STAT_PATH(za, path, path_len, flags, sb) */
+/* {{{  PHP_ZIP_STAT_PATH(za, path, path_len, flags, sb)
+       This is always used for the first argument*/
 #define PHP_ZIP_STAT_PATH(za, path, path_len, flags, sb) \
-       if (path_len < 1) { \
-               php_error_docref(NULL, E_NOTICE, "Empty string as entry name"); \
-               RETURN_FALSE; \
+       if (path_len == 0) { \
+               zend_argument_value_error(1, "cannot be empty"); \
+               RETURN_THROWS(); \
        } \
        if (zip_stat(za, path, flags, &sb) != 0) { \
                RETURN_FALSE; \
@@ -348,24 +349,41 @@ static int php_zip_parse_options(HashTable *options, zip_options *opts)
 #endif
 
        if ((option = zend_hash_str_find(options, "remove_all_path", sizeof("remove_all_path") - 1)) != NULL) {
+               if (Z_TYPE_P(option) != IS_FALSE && Z_TYPE_P(option) != IS_TRUE) {
+                       php_error_docref(NULL, E_WARNING, "Option \"remove_all_path\" must be of type bool, %s given",
+                               zend_zval_type_name(option));
+               }
                opts->remove_all_path = zval_get_long(option);
        }
 
        if ((option = zend_hash_str_find(options, "comp_method", sizeof("comp_method") - 1)) != NULL) {
+               if (Z_TYPE_P(option) != IS_LONG) {
+                       php_error_docref(NULL, E_WARNING, "Option \"comp_method\" must be of type int, %s given",
+                               zend_zval_type_name(option));
+               }
                opts->comp_method = zval_get_long(option);
 
                if ((option = zend_hash_str_find(options, "comp_flags", sizeof("comp_flags") - 1)) != NULL) {
+                       if (Z_TYPE_P(option) != IS_LONG) {
+                               php_error_docref(NULL, E_WARNING, "Option \"comp_flags\" must be of type int, %s given",
+                                       zend_zval_type_name(option));
+                       }
                        opts->comp_flags = zval_get_long(option);
                }
        }
 
 #ifdef HAVE_ENCRYPTION
        if ((option = zend_hash_str_find(options, "enc_method", sizeof("enc_method") - 1)) != NULL) {
+               if (Z_TYPE_P(option) != IS_LONG) {
+                       php_error_docref(NULL, E_WARNING, "Option \"enc_method\" must be of type int, %s given",
+                               zend_zval_type_name(option));
+               }
                opts->enc_method = zval_get_long(option);
 
                if ((option = zend_hash_str_find(options, "enc_password", sizeof("enc_password") - 1)) != NULL) {
                        if (Z_TYPE_P(option) != IS_STRING) {
-                               php_error_docref(NULL, E_WARNING, "enc_password option expected to be a string");
+                               zend_type_error("Option \"enc_password\" must be of type string, %s given",
+                                       zend_zval_type_name(option));
                                return -1;
                        }
                        opts->enc_password = Z_STRVAL_P(option);
@@ -375,18 +393,18 @@ static int php_zip_parse_options(HashTable *options, zip_options *opts)
 
        if ((option = zend_hash_str_find(options, "remove_path", sizeof("remove_path") - 1)) != NULL) {
                if (Z_TYPE_P(option) != IS_STRING) {
-                       php_error_docref(NULL, E_WARNING, "remove_path option expected to be a string");
+                       zend_type_error("Option \"remove_path\" must be of type string, %s given",
+                               zend_zval_type_name(option));
                        return -1;
                }
 
-               if (Z_STRLEN_P(option) < 1) {
-                       php_error_docref(NULL, E_NOTICE, "Empty string given as remove_path option");
+               if (Z_STRLEN_P(option) == 0) {
+                       zend_value_error("Option \"remove_path\" cannot be empty");
                        return -1;
                }
 
                if (Z_STRLEN_P(option) >= MAXPATHLEN) {
-                       php_error_docref(NULL, E_WARNING, "remove_path string is too long (max: %d, %zd given)",
-                                               MAXPATHLEN - 1, Z_STRLEN_P(option));
+                       zend_value_error("Option \"remove_path\" must be less than %d bytes", MAXPATHLEN - 1);
                        return -1;
                }
                opts->remove_path_len = Z_STRLEN_P(option);
@@ -395,18 +413,18 @@ static int php_zip_parse_options(HashTable *options, zip_options *opts)
 
        if ((option = zend_hash_str_find(options, "add_path", sizeof("add_path") - 1)) != NULL) {
                if (Z_TYPE_P(option) != IS_STRING) {
-                       php_error_docref(NULL, E_WARNING, "add_path option expected to be a string");
+                       zend_type_error("Option \"add_path\" must be of type string, %s given",
+                               zend_zval_type_name(option));
                        return -1;
                }
 
-               if (Z_STRLEN_P(option) < 1) {
-                       php_error_docref(NULL, E_NOTICE, "Empty string given as the add_path option");
+               if (Z_STRLEN_P(option) == 0) {
+                       zend_value_error("Option \"add_path\" cannot be empty");
                        return -1;
                }
 
                if (Z_STRLEN_P(option) >= MAXPATHLEN) {
-                       php_error_docref(NULL, E_WARNING, "add_path string too long (max: %d, %zd given)",
-                                               MAXPATHLEN - 1, Z_STRLEN_P(option));
+                       zend_value_error("Option \"add_path\" must be less than %d bytes", MAXPATHLEN - 1);
                        return -1;
                }
                opts->add_path_len = Z_STRLEN_P(option);
@@ -415,7 +433,8 @@ static int php_zip_parse_options(HashTable *options, zip_options *opts)
 
        if ((option = zend_hash_str_find(options, "flags", sizeof("flags") - 1)) != NULL) {
                if (Z_TYPE_P(option) != IS_LONG) {
-                       php_error_docref(NULL, E_WARNING, "flags option expected to be a integer");
+                       zend_type_error("Option \"flags\" must be of type int, %s given",
+                               zend_zval_type_name(option));
                        return -1;
                }
                opts->flags = Z_LVAL_P(option);
@@ -599,6 +618,7 @@ int php_zip_glob(char *pattern, int pattern_len, zend_long flags, zval *return_v
        }
 
        if ((GLOB_AVAILABLE_FLAGS & flags) != flags) {
+
                php_error_docref(NULL, E_WARNING, "At least one of the passed flags is invalid or not supported on this platform");
                return -1;
        }
@@ -1134,8 +1154,8 @@ PHP_FUNCTION(zip_open)
        }
 
        if (ZSTR_LEN(filename) == 0) {
-               php_error_docref(NULL, E_WARNING, "Empty string as source");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        if (ZIP_OPENBASEDIR_CHECKPATH(ZSTR_VAL(filename))) {
@@ -1414,8 +1434,8 @@ PHP_METHOD(ZipArchive, open)
        ze_obj = Z_ZIP_P(self);
 
        if (ZSTR_LEN(filename) == 0) {
-               php_error_docref(NULL, E_WARNING, "Empty string as source");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        if (ZIP_OPENBASEDIR_CHECKPATH(ZSTR_VAL(filename))) {
@@ -1677,11 +1697,11 @@ static void php_zip_add_from_pattern(INTERNAL_FUNCTION_PARAMETERS, int type) /*
        }
 
        if (ZSTR_LEN(pattern) == 0) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as pattern");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
        if (options && zend_hash_num_elements(options) > 0 && (php_zip_parse_options(options, &opts) < 0)) {
-               RETURN_FALSE;
+               RETURN_THROWS();
        }
 
        if (type == 1) {
@@ -1795,8 +1815,8 @@ PHP_METHOD(ZipArchive, addFile)
        }
 
        if (ZSTR_LEN(filename) == 0) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as filename");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        if (entry_name_len == 0) {
@@ -1828,13 +1848,13 @@ PHP_METHOD(ZipArchive, replaceFile)
        }
 
        if (ZSTR_LEN(filename) == 0) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as filename");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        if (index < 0) {
-               php_error_docref(NULL, E_NOTICE, "Invalid negative index");
-               RETURN_FALSE;
+               zend_argument_value_error(2, "must be greater than or equal to 0");
+               RETURN_THROWS();
        }
 
        if (php_zip_add_file(Z_ZIP_P(self), ZSTR_VAL(filename), ZSTR_LEN(filename),
@@ -2008,8 +2028,8 @@ PHP_METHOD(ZipArchive, setArchiveComment)
        ZIP_FROM_OBJECT(intern, self);
 
        if (comment_len > 0xffff) {
-               php_error_docref(NULL, E_WARNING, "Comment must not exceed 65535 bytes");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "must be less than 65535 bytes");
+               RETURN_THROWS();
        }
 
        if (zip_set_archive_comment(intern, (const char *)comment, comment_len)) {
@@ -2057,15 +2077,16 @@ PHP_METHOD(ZipArchive, setCommentName)
                RETURN_THROWS();
        }
 
-       if (name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as entry name");
+       if (name_len == 0) {
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        ZIP_FROM_OBJECT(intern, self);
 
        if (comment_len > 0xffff) {
-               php_error_docref(NULL, E_WARNING, "Comment must not exceed 65535 bytes");
-               RETURN_FALSE;
+               zend_argument_value_error(2, "must be less than 65535 bytes");
+               RETURN_THROWS();
        }
 
        idx = zip_name_locate(intern, name, 0);
@@ -2094,8 +2115,8 @@ PHP_METHOD(ZipArchive, setCommentIndex)
        ZIP_FROM_OBJECT(intern, self);
 
        if (comment_len > 0xffff) {
-               php_error_docref(NULL, E_WARNING, "Comment must not exceed 65535 bytes");
-               RETURN_FALSE;
+               zend_argument_value_error(2, "must be less than 65535 bytes");
+               RETURN_THROWS();
        }
 
        PHP_ZIP_STAT_INDEX(intern, index, 0, sb);
@@ -2123,11 +2144,13 @@ PHP_METHOD(ZipArchive, setExternalAttributesName)
 
        ZIP_FROM_OBJECT(intern, self);
 
-       if (name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as entry name");
+       if (name_len == 0) {
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        idx = zip_name_locate(intern, name, 0);
+
        if (idx < 0) {
                RETURN_FALSE;
        }
@@ -2182,11 +2205,13 @@ PHP_METHOD(ZipArchive, getExternalAttributesName)
 
        ZIP_FROM_OBJECT(intern, self);
 
-       if (name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as entry name");
+       if (name_len == 0) {
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        idx = zip_name_locate(intern, name, 0);
+
        if (idx < 0) {
                RETURN_FALSE;
        }
@@ -2247,11 +2272,13 @@ PHP_METHOD(ZipArchive, setEncryptionName)
 
        ZIP_FROM_OBJECT(intern, self);
 
-       if (name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as entry name");
+       if (name_len == 0) {
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        idx = zip_name_locate(intern, name, 0);
+
        if (idx < 0) {
                RETURN_FALSE;
        }
@@ -2306,12 +2333,13 @@ PHP_METHOD(ZipArchive, getCommentName)
 
        ZIP_FROM_OBJECT(intern, self);
 
-       if (name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as entry name");
-               RETURN_FALSE;
+       if (name_len == 0) {
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        idx = zip_name_locate(intern, name, 0);
+
        if (idx < 0) {
                RETURN_FALSE;
        }
@@ -2346,7 +2374,7 @@ PHP_METHOD(ZipArchive, getCommentIndex)
 
 /* {{{ Set the compression of a file in zip, using its name */
 PHP_METHOD(ZipArchive, setCompressionName)
- {
+{
        struct zip *intern;
        zval *this = ZEND_THIS;
        size_t name_len;
@@ -2361,11 +2389,13 @@ PHP_METHOD(ZipArchive, setCompressionName)
 
        ZIP_FROM_OBJECT(intern, this);
 
-       if (name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as entry name");
+       if (name_len == 0) {
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        idx = zip_name_locate(intern, name, 0);
+
        if (idx < 0) {
                RETURN_FALSE;
        }
@@ -2404,7 +2434,7 @@ PHP_METHOD(ZipArchive, setCompressionIndex)
 #ifdef HAVE_SET_MTIME
 /* {{{ Set the modification time of a file in zip, using its name */
 PHP_METHOD(ZipArchive, setMtimeName)
- {
+{
        struct zip *intern;
        zval *this = ZEND_THIS;
        size_t name_len;
@@ -2419,11 +2449,13 @@ PHP_METHOD(ZipArchive, setMtimeName)
 
        ZIP_FROM_OBJECT(intern, this);
 
-       if (name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as entry name");
+       if (name_len == 0) {
+               zend_argument_value_error(1, "cannot be empty");
+               RETURN_THROWS();
        }
 
        idx = zip_name_locate(intern, name, 0);
+
        if (idx < 0) {
                RETURN_FALSE;
        }
@@ -2531,9 +2563,9 @@ PHP_METHOD(ZipArchive, renameIndex)
 
        ZIP_FROM_OBJECT(intern, self);
 
-       if (new_name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as new entry name");
-               RETURN_FALSE;
+       if (new_name_len == 0) {
+               zend_argument_value_error(2, "cannot be empty");
+               RETURN_THROWS();
        }
 
        if (zip_file_rename(intern, index, (const char *)new_name, 0) != 0) {
@@ -2559,9 +2591,9 @@ PHP_METHOD(ZipArchive, renameName)
 
        ZIP_FROM_OBJECT(intern, self);
 
-       if (new_name_len < 1) {
-               php_error_docref(NULL, E_NOTICE, "Empty string as new entry name");
-               RETURN_FALSE;
+       if (new_name_len == 0) {
+               zend_argument_value_error(2, "cannot be empty");
+               RETURN_THROWS();
        }
 
        PHP_ZIP_STAT_PATH(intern, name, name_len, 0, sb);
@@ -2886,19 +2918,12 @@ PHP_METHOD(ZipArchive, registerProgressCallback)
        struct zip *intern;
        zval *self = ZEND_THIS;
        double rate;
-       zval *callback;
+       zend_fcall_info fci;
+       zend_fcall_info_cache fcc;
        ze_zip_object *obj;
 
-       if (zend_parse_parameters(ZEND_NUM_ARGS(), "dz", &rate, &callback) == FAILURE) {
-               return;
-       }
-
-       /* callable? */
-       if (!zend_is_callable(callback, 0, NULL)) {
-               zend_string *callback_name = zend_get_callable_name(callback);
-               php_error_docref(NULL, E_WARNING, "Invalid callback '%s'", ZSTR_VAL(callback_name));
-               zend_string_release_ex(callback_name, 0);
-               RETURN_FALSE;
+       if (zend_parse_parameters(ZEND_NUM_ARGS(), "df", &rate, &fci, &fcc) == FAILURE) {
+               RETURN_THROWS();
        }
 
        ZIP_FROM_OBJECT(intern, self);
@@ -2909,7 +2934,7 @@ PHP_METHOD(ZipArchive, registerProgressCallback)
        _php_zip_progress_callback_free(obj);
 
        /* register */
-       ZVAL_COPY(&obj->progress_callback, callback);
+       ZVAL_COPY(&obj->progress_callback, &fci.function_name);
        if (zip_register_progress_callback_with_state(intern, rate, _php_zip_progress_callback, _php_zip_progress_callback_free, obj)) {
                RETURN_FALSE;
        }
@@ -2939,30 +2964,22 @@ PHP_METHOD(ZipArchive, registerCancelCallback)
 {
        struct zip *intern;
        zval *self = ZEND_THIS;
-       zval *callback;
+       zend_fcall_info fci;
+       zend_fcall_info_cache fcc;
        ze_zip_object *obj;
-
-       if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &callback) == FAILURE) {
-               return;
+       if (zend_parse_parameters(ZEND_NUM_ARGS(), "f", &fci, &fcc) == FAILURE) {
+               RETURN_THROWS();
        }
 
        ZIP_FROM_OBJECT(intern, self);
 
-       /* callable? */
-       if (!zend_is_callable(callback, 0, NULL)) {
-               zend_string *callback_name = zend_get_callable_name(callback);
-               php_error_docref(NULL, E_WARNING, "Invalid callback '%s'", ZSTR_VAL(callback_name));
-               zend_string_release_ex(callback_name, 0);
-               RETURN_FALSE;
-       }
-
        obj = Z_ZIP_P(self);
 
        /* free if called twice */
        _php_zip_cancel_callback_free(obj);
 
        /* register */
-       ZVAL_COPY(&obj->cancel_callback, callback);
+       ZVAL_COPY(&obj->cancel_callback, &fci.function_name);
        if (zip_register_cancel_callback_with_state(intern, _php_zip_cancel_callback, _php_zip_cancel_callback_free, obj)) {
                RETURN_FALSE;
        }
index 6f54105cc88d21e6c83feb1f6d88ec2e9b2bfa41..8687167a40e9305d828af530164909f44bbd5e9a 100644 (file)
@@ -16,16 +16,20 @@ if (!$zip->open($file)) {
 echo $zip->getArchiveComment() . "\n";
 
 $idx = $zip->locateName('foo');
-echo $zip->getCommentName('foo') . "\n";
-echo $zip->getCommentIndex($idx);
+var_dump($zip->getCommentName('foo'));
+var_dump($zip->getCommentIndex($idx));
 
-echo $zip->getCommentName('') . "\n";
+try {
+    echo $zip->getCommentName('') . "\n";
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 
 $zip->close();
 
 ?>
---EXPECTF--
+--EXPECT--
 Zip archive comment
-foo comment
-foo comment
-Notice: ZipArchive::getCommentName(): Empty string as entry name in %s on line %d
+string(11) "foo comment"
+string(11) "foo comment"
+ZipArchive::getCommentName(): Argument #1 ($name) cannot be empty
index 2dffdc0d889f7ae32e66b2a93d6c17904640279a..a7e8d308186df3c704ed2849b896556cf114f413 100644 (file)
@@ -25,7 +25,11 @@ if (!$r) {
 @unlink($dirname . 'nofile');
 
 $zip = new ZipArchive;
-$zip->open('');
+try {
+    $zip->open('');
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 
 if (!$zip->open($dirname . 'test.zip')) {
     exit("failed 1\n");
@@ -37,9 +41,8 @@ if ($zip->status == ZIPARCHIVE::ER_OK) {
     echo "failed\n";
 }
 ?>
---EXPECTF--
+--EXPECT--
 ER_OPEN: ok
 create: ok
-
-Warning: ZipArchive::open(): Empty string as source in %s on line %d
+ZipArchive::open(): Argument #1 ($filename) cannot be empty
 OK
index 78d8d3dc35602e28d966c7d06ed7e2c6cd0c70ed..74a582a6f8efe0320e275e5698cb7789d1948fcc 100644 (file)
@@ -20,21 +20,28 @@ $zip->addFromString('entry2.txt', 'entry #2');
 
 $longComment = str_repeat('a', 0x10000);
 
-var_dump($zip->setArchiveComment($longComment));
-var_dump($zip->setCommentName('entry1.txt', $longComment));
-var_dump($zip->setCommentIndex(1, $longComment));
+try {
+    var_dump($zip->setArchiveComment($longComment));
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+try {
+    var_dump($zip->setCommentName('entry1.txt', $longComment));
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+try {
+    var_dump($zip->setCommentIndex(1, $longComment));
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 
 $zip->close();
 ?>
---EXPECTF--
-Warning: ZipArchive::setArchiveComment(): Comment must not exceed 65535 bytes in %s on line %d
-bool(false)
-
-Warning: ZipArchive::setCommentName(): Comment must not exceed 65535 bytes in %s on line %d
-bool(false)
-
-Warning: ZipArchive::setCommentIndex(): Comment must not exceed 65535 bytes in %s on line %d
-bool(false)
+--EXPECT--
+ZipArchive::setArchiveComment(): Argument #1 ($comment) must be less than 65535 bytes
+ZipArchive::setCommentName(): Argument #2 ($comment) must be less than 65535 bytes
+ZipArchive::setCommentIndex(): Argument #2 ($comment) must be less than 65535 bytes
 --CLEAN--
 <?php
 @unlink(__DIR__ . '/__tmp_oo_set_comment_error.zip');
index ae52873f43036c3522edd5b2d615a704d9036e6b..1ac8bf44b222b8cc642b896ec7897b53f1b90ace 100644 (file)
@@ -10,7 +10,11 @@ if(!extension_loaded('zip')) die('skip');
 --FILE--
 <?php
 echo "Test case 1:";
-$zip = zip_open("");
+try {
+    $zip = zip_open("");
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 
 echo "Test case 2:\n";
 $zip = zip_open("/non_exisitng_directory/test_procedural.zip");
@@ -19,8 +23,7 @@ echo is_resource($zip) ? "OK" : "Failure";
 --EXPECTF--
 Test case 1:
 Deprecated: Function zip_open() is deprecated in %s on line %d
-
-Warning: zip_open(): Empty string as source in %s on line %d
+zip_open(): Argument #1 ($filename) cannot be empty
 Test case 2:
 
 Deprecated: Function zip_open() is deprecated in %s on line %d