From e52fd28ea1a1817cc410050d86370c6732a2a702 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 21 Aug 2019 02:52:20 +0200 Subject: [PATCH 1/4] Promote warnings to errors in extract() --- ext/standard/array.c | 7 +++-- ext/standard/tests/array/extract_error.phpt | 32 +++++++++++++-------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 5f3830d89984f..3628d76a76ec5 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2452,23 +2452,24 @@ PHP_FUNCTION(extract) extract_type &= 0xff; if (extract_type < EXTR_OVERWRITE || extract_type > EXTR_IF_EXISTS) { - php_error_docref(NULL, E_WARNING, "Invalid extract type"); + zend_throw_error(NULL, "Invalid extract type"); return; } if (extract_type > EXTR_SKIP && extract_type <= EXTR_PREFIX_IF_EXISTS && ZEND_NUM_ARGS() < 3) { - php_error_docref(NULL, E_WARNING, "specified extract type requires the prefix parameter"); + zend_throw_error(NULL, "specified extract type requires the prefix parameter"); return; } if (prefix) { if (ZSTR_LEN(prefix) && !php_valid_var_name(ZSTR_VAL(prefix), ZSTR_LEN(prefix))) { - php_error_docref(NULL, E_WARNING, "prefix is not a valid identifier"); + zend_throw_error(NULL, "prefix is not a valid identifier"); return; } } if (zend_forbid_dynamic_call("extract()") == FAILURE) { + /* TODO Elevate to exception ? */ return; } diff --git a/ext/standard/tests/array/extract_error.phpt b/ext/standard/tests/array/extract_error.phpt index 08e1824b0fb30..c03212973ebe4 100644 --- a/ext/standard/tests/array/extract_error.phpt +++ b/ext/standard/tests/array/extract_error.phpt @@ -8,11 +8,25 @@ echo "*** Testing Error Conditions ***\n"; /* Invalid second argument ( only 0-6 is valid) */ $arr = array(1); -var_dump( extract($arr, -1 . "wddr") ); -var_dump( extract($arr, 7 , "wddr") ); + +try { + var_dump( extract($arr, -1 . "wddr") ); +} catch (\Error $e) { + echo $e->getMessage() . "\n"; +} + +try { + var_dump( extract($arr, 7 , "wddr") ); +} catch (\Error $e) { + echo $e->getMessage() . "\n"; +} /* Two Arguments, second as prefix but without prefix string as third argument */ -var_dump( extract($arr,EXTR_PREFIX_IF_EXISTS) ); +try { + var_dump( extract($arr,EXTR_PREFIX_IF_EXISTS) ); +} catch (\Error $e) { + echo $e->getMessage() . "\n"; +} echo "Done\n"; ?> @@ -20,13 +34,7 @@ echo "Done\n"; *** Testing Error Conditions *** Notice: A non well formed numeric value encountered in %s on line %d - -Warning: extract(): Invalid extract type in %s on line %d -NULL - -Warning: extract(): Invalid extract type in %s on line %d -NULL - -Warning: extract(): specified extract type requires the prefix parameter in %s on line %d -NULL +Invalid extract type +Invalid extract type +specified extract type requires the prefix parameter Done From 5531e5a2332e86887e0ca5ddd1c7c4805b67fe80 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 21 Aug 2019 18:44:38 +0200 Subject: [PATCH 2/4] Implement review --- ext/standard/array.c | 5 ++--- ext/standard/tests/array/extract_error.phpt | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 3628d76a76ec5..1a4c5eac7fc89 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2457,19 +2457,18 @@ PHP_FUNCTION(extract) } if (extract_type > EXTR_SKIP && extract_type <= EXTR_PREFIX_IF_EXISTS && ZEND_NUM_ARGS() < 3) { - zend_throw_error(NULL, "specified extract type requires the prefix parameter"); + zend_throw_error(NULL, "Specified extract type requires the prefix parameter"); return; } if (prefix) { if (ZSTR_LEN(prefix) && !php_valid_var_name(ZSTR_VAL(prefix), ZSTR_LEN(prefix))) { - zend_throw_error(NULL, "prefix is not a valid identifier"); + zend_throw_error(NULL, "Prefix is not a valid identifier"); return; } } if (zend_forbid_dynamic_call("extract()") == FAILURE) { - /* TODO Elevate to exception ? */ return; } diff --git a/ext/standard/tests/array/extract_error.phpt b/ext/standard/tests/array/extract_error.phpt index c03212973ebe4..2103a1b9a4819 100644 --- a/ext/standard/tests/array/extract_error.phpt +++ b/ext/standard/tests/array/extract_error.phpt @@ -36,5 +36,5 @@ echo "Done\n"; Notice: A non well formed numeric value encountered in %s on line %d Invalid extract type Invalid extract type -specified extract type requires the prefix parameter +Specified extract type requires the prefix parameter Done From d86262a302ebd0b02b59d77948559e27b5cf6803 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 28 Aug 2019 00:53:48 +0200 Subject: [PATCH 3/4] Add tests for extract error conditions --- .../tests/array/extract_error_variation1.phpt | 15 +++++++++++++++ .../tests/array/extract_error_variation2.phpt | 15 +++++++++++++++ .../tests/array/extract_error_variation3.phpt | 14 ++++++++++++++ 3 files changed, 44 insertions(+) create mode 100644 ext/standard/tests/array/extract_error_variation1.phpt create mode 100644 ext/standard/tests/array/extract_error_variation2.phpt create mode 100644 ext/standard/tests/array/extract_error_variation3.phpt diff --git a/ext/standard/tests/array/extract_error_variation1.phpt b/ext/standard/tests/array/extract_error_variation1.phpt new file mode 100644 index 0000000000000..31fac72bedb86 --- /dev/null +++ b/ext/standard/tests/array/extract_error_variation1.phpt @@ -0,0 +1,15 @@ +--TEST-- +Test extract() function - error condition - Invalid flag argument. +--FILE-- + "one", "2" => "two", "3" => "three", "4" => "four", "5" => "five"]; + +try { + extract($a, 10000); +} catch (\Error $e) { + echo $e->getMessage(); +} + +?> +--EXPECT-- +Invalid extract type diff --git a/ext/standard/tests/array/extract_error_variation2.phpt b/ext/standard/tests/array/extract_error_variation2.phpt new file mode 100644 index 0000000000000..c1ac6b7af4565 --- /dev/null +++ b/ext/standard/tests/array/extract_error_variation2.phpt @@ -0,0 +1,15 @@ +--TEST-- +Test extract() function - error condition - Prefix flag without supplying prefix. +--FILE-- + "one", "2" => "two", "3" => "three", "4" => "four", "5" => "five"]; + +try { + extract($a, EXTR_PREFIX_ALL); +} catch (\Error $e) { + echo $e->getMessage(); +} + +?> +--EXPECT-- +Specified extract type requires the prefix parameter diff --git a/ext/standard/tests/array/extract_error_variation3.phpt b/ext/standard/tests/array/extract_error_variation3.phpt new file mode 100644 index 0000000000000..ec3078a0b9713 --- /dev/null +++ b/ext/standard/tests/array/extract_error_variation3.phpt @@ -0,0 +1,14 @@ +--TEST-- +Test extract() function - error condition - Invalid prefix. +--FILE-- + "one", "2" => "two", "3" => "three", "4" => "four", "5" => "five"]; + +try { + extract($a, EXTR_PREFIX_ALL, '85bogus'); +} catch (\Error $e) { + echo $e->getMessage(); +} +?> +--EXPECT-- +Prefix is not a valid identifier From f6371181f6b1ad89a7cb1c51fad3a52affe35cb1 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 28 Aug 2019 00:59:58 +0200 Subject: [PATCH 4/4] extract() cannot return null anymore --- ext/standard/array.c | 2 +- ext/standard/basic_functions.stub.php | 2 +- ext/standard/basic_functions_arginfo.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 1a4c5eac7fc89..3492f942a2942 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2427,7 +2427,7 @@ static zend_long php_extract_skip(zend_array *arr, zend_array *symbol_table) /* } /* }}} */ -/* {{{ proto int|null extract(array var_array [, int extract_type [, string prefix]]) +/* {{{ proto int extract(array var_array [, int extract_type [, string prefix]]) Imports variables into symbol table from an array */ PHP_FUNCTION(extract) { diff --git a/ext/standard/basic_functions.stub.php b/ext/standard/basic_functions.stub.php index e110e5f2db4ab..0922d39115674 100755 --- a/ext/standard/basic_functions.stub.php +++ b/ext/standard/basic_functions.stub.php @@ -139,7 +139,7 @@ function in_array($needle, array $haystack, bool $strict = false): bool {} function array_search($needle, array $haystack, bool $strict = false) {} /** @prefer-ref $arg */ -function extract(array &$arg, int $extract_type = EXTR_OVERWRITE, string $prefix = ""): ?int {} +function extract(array &$arg, int $extract_type = EXTR_OVERWRITE, string $prefix = ""): int {} function compact($var_name, ...$var_names): array {} diff --git a/ext/standard/basic_functions_arginfo.h b/ext/standard/basic_functions_arginfo.h index 6663c4131f8ee..5a45b55b03443 100755 --- a/ext/standard/basic_functions_arginfo.h +++ b/ext/standard/basic_functions_arginfo.h @@ -150,7 +150,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_array_search, 0, 0, 2) ZEND_ARG_TYPE_INFO(0, strict, _IS_BOOL, 0) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_extract, 0, 1, IS_LONG, 1) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_extract, 0, 1, IS_LONG, 0) ZEND_ARG_TYPE_INFO(ZEND_SEND_PREFER_REF, arg, IS_ARRAY, 0) ZEND_ARG_TYPE_INFO(0, extract_type, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, prefix, IS_STRING, 0)