From e4672fd1ffd1ce59bafc15c89ca069acaedaac71 Mon Sep 17 00:00:00 2001 From: David Gebler Date: Wed, 28 Apr 2021 08:19:16 +0100 Subject: [PATCH 1/8] Check parameters on compact() and throw TypeError --- ext/standard/array.c | 9 ++++++--- ext/standard/tests/array/compact.phpt | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 54c08b3c97080..7586af5206204 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2447,7 +2447,7 @@ PHP_FUNCTION(extract) } /* }}} */ -static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_value, zval *entry) /* {{{ */ +static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_value, zval *entry, int pos) /* {{{ */ { zval *value_ptr, data; @@ -2475,11 +2475,14 @@ static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_valu Z_PROTECT_RECURSION_P(entry); } ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(entry), value_ptr) { - php_compact_var(eg_active_symbol_table, return_value, value_ptr); + php_compact_var(eg_active_symbol_table, return_value, value_ptr, pos); } ZEND_HASH_FOREACH_END(); if (Z_REFCOUNTED_P(entry)) { Z_UNPROTECT_RECURSION_P(entry); } + } else { + zend_throw_error(zend_ce_type_error, "Parameter %d in compact() must be string or array of strings", pos); + return; } } /* }}} */ @@ -2512,7 +2515,7 @@ PHP_FUNCTION(compact) } for (i = 0; i < num_args; i++) { - php_compact_var(symbol_table, return_value, &args[i]); + php_compact_var(symbol_table, return_value, &args[i], i + 1); } } /* }}} */ diff --git a/ext/standard/tests/array/compact.phpt b/ext/standard/tests/array/compact.phpt index 978120e822cea..23f7072655728 100644 --- a/ext/standard/tests/array/compact.phpt +++ b/ext/standard/tests/array/compact.phpt @@ -11,6 +11,20 @@ $location_vars = array("c\\u0327ity", "state"); $result = compact("event", $location_vars); var_dump($result); + +try { + $result = compact(true); +} catch (TypeError $e) { + echo "TypeError: ".$e->getMessage()."\n"; +} + +try { + $foo = 'bar'; + $bar = 'baz'; + $result = compact($foo, [true]); +} catch (TypeError $e) { + echo "TypeError: ".$e->getMessage(); +} ?> --EXPECTF-- Warning: compact(): Undefined variable $c\u0327ity in %s on line %d @@ -20,3 +34,5 @@ array(2) { ["state"]=> string(2) "CA" } +TypeError: Parameter 1 in compact() must be string or array of strings +TypeError: Parameter 2 in compact() must be string or array of strings \ No newline at end of file From f35c7e0acf2346bc23d39d0e079ca7317afcbc38 Mon Sep 17 00:00:00 2001 From: David Gebler Date: Wed, 28 Apr 2021 08:55:49 +0100 Subject: [PATCH 2/8] remove old test cases which no longer yield empty output --- ext/standard/tests/array/compact_basic.phpt | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/ext/standard/tests/array/compact_basic.phpt b/ext/standard/tests/array/compact_basic.phpt index da8844d38b276..d26a03ba0c350 100644 --- a/ext/standard/tests/array/compact_basic.phpt +++ b/ext/standard/tests/array/compact_basic.phpt @@ -21,11 +21,6 @@ var_dump (compact(array("a", "b", "c", "d", "e", "f"))); var_dump (compact("a", "b", "c", "d", "e", "f")); var_dump (compact(array("keyval"=>"a", "b"=>"b", "c"=>1))); -// cases which should not yield any output. -var_dump (compact(array(10, 0.3, true, array(20), NULL))); -var_dump (compact(10, 0.3, true, array(20), NULL)); -var_dump (compact(array("g"))); - echo "Done"; ?> --EXPECTF-- @@ -70,12 +65,4 @@ array(2) { ["b"]=> float(0.2) } -array(0) { -} -array(0) { -} - -Warning: compact(): Undefined variable $g in %s on line %d -array(0) { -} Done From da2118c975c97051ed39671dcd24a1f6510018ca Mon Sep 17 00:00:00 2001 From: David Gebler Date: Wed, 28 Apr 2021 09:33:06 +0100 Subject: [PATCH 3/8] remove integer val from array test --- ext/standard/tests/array/compact_basic.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/tests/array/compact_basic.phpt b/ext/standard/tests/array/compact_basic.phpt index d26a03ba0c350..681f2638cb054 100644 --- a/ext/standard/tests/array/compact_basic.phpt +++ b/ext/standard/tests/array/compact_basic.phpt @@ -19,7 +19,7 @@ $f="string"; var_dump (compact(array("a", "b", "c", "d", "e", "f"))); // simple parameter test var_dump (compact("a", "b", "c", "d", "e", "f")); -var_dump (compact(array("keyval"=>"a", "b"=>"b", "c"=>1))); +var_dump (compact(array("keyval"=>"a", "b"=>"b"))); echo "Done"; ?> From aada56e5517cd98af8587b80f12249e89417cdd1 Mon Sep 17 00:00:00 2001 From: David Gebler Date: Wed, 28 Apr 2021 11:44:49 +0100 Subject: [PATCH 4/8] address gpb's review comments --- ext/standard/array.c | 2 +- ext/standard/tests/array/compact.phpt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 7586af5206204..bc8b0020a4a82 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2447,7 +2447,7 @@ PHP_FUNCTION(extract) } /* }}} */ -static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_value, zval *entry, int pos) /* {{{ */ +static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_value, zval *entry, uint32_t pos) /* {{{ */ { zval *value_ptr, data; diff --git a/ext/standard/tests/array/compact.phpt b/ext/standard/tests/array/compact.phpt index 23f7072655728..05fb377393676 100644 --- a/ext/standard/tests/array/compact.phpt +++ b/ext/standard/tests/array/compact.phpt @@ -35,4 +35,4 @@ array(2) { string(2) "CA" } TypeError: Parameter 1 in compact() must be string or array of strings -TypeError: Parameter 2 in compact() must be string or array of strings \ No newline at end of file +TypeError: Parameter 2 in compact() must be string or array of strings From 97b22e4793e8747903c89d98a2b0becbc408c9f8 Mon Sep 17 00:00:00 2001 From: David Gebler Date: Wed, 28 Apr 2021 17:56:00 +0100 Subject: [PATCH 5/8] change typeerror message and corresponding tests --- ext/standard/array.c | 4 ++-- ext/standard/tests/array/compact.phpt | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index bc8b0020a4a82..4ef9adf73836c 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2481,8 +2481,8 @@ static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_valu Z_UNPROTECT_RECURSION_P(entry); } } else { - zend_throw_error(zend_ce_type_error, "Parameter %d in compact() must be string or array of strings", pos); - return; + zend_argument_type_error(pos, "must be string or array of strings, %s given", zend_zval_type_name(entry)); + RETURN_THROWS(); } } /* }}} */ diff --git a/ext/standard/tests/array/compact.phpt b/ext/standard/tests/array/compact.phpt index 05fb377393676..b22bdc4b0ddb9 100644 --- a/ext/standard/tests/array/compact.phpt +++ b/ext/standard/tests/array/compact.phpt @@ -21,7 +21,7 @@ try { try { $foo = 'bar'; $bar = 'baz'; - $result = compact($foo, [true]); + $result = compact($foo, [42]); } catch (TypeError $e) { echo "TypeError: ".$e->getMessage(); } @@ -34,5 +34,5 @@ array(2) { ["state"]=> string(2) "CA" } -TypeError: Parameter 1 in compact() must be string or array of strings -TypeError: Parameter 2 in compact() must be string or array of strings +TypeError: compact(): Argument #1 ($var_name) must be string or array of strings, bool given +TypeError: compact(): Argument #2 must be string or array of strings, int given From 1f62ace8eee9be82cce018af27e7df9677e1ad70 Mon Sep 17 00:00:00 2001 From: David Gebler Date: Sat, 8 May 2021 01:01:09 +0100 Subject: [PATCH 6/8] revert to warning on invalid parameter --- ext/standard/array.c | 4 ++-- ext/standard/tests/array/compact.phpt | 22 ++++++++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 4ef9adf73836c..02c0bccc6f07f 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2481,8 +2481,8 @@ static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_valu Z_UNPROTECT_RECURSION_P(entry); } } else { - zend_argument_type_error(pos, "must be string or array of strings, %s given", zend_zval_type_name(entry)); - RETURN_THROWS(); + php_error_docref(NULL, E_WARNING, "Argument #%d must be string or array of strings, %s given", pos, zend_zval_type_name(entry)); + return; } } /* }}} */ diff --git a/ext/standard/tests/array/compact.phpt b/ext/standard/tests/array/compact.phpt index b22bdc4b0ddb9..cef94cc588f97 100644 --- a/ext/standard/tests/array/compact.phpt +++ b/ext/standard/tests/array/compact.phpt @@ -12,19 +12,11 @@ $location_vars = array("c\\u0327ity", "state"); $result = compact("event", $location_vars); var_dump($result); -try { - $result = compact(true); -} catch (TypeError $e) { - echo "TypeError: ".$e->getMessage()."\n"; -} +$result = compact(true); +$foo = 'bar'; +$bar = 'baz'; +$result = compact($foo, [42]); -try { - $foo = 'bar'; - $bar = 'baz'; - $result = compact($foo, [42]); -} catch (TypeError $e) { - echo "TypeError: ".$e->getMessage(); -} ?> --EXPECTF-- Warning: compact(): Undefined variable $c\u0327ity in %s on line %d @@ -34,5 +26,7 @@ array(2) { ["state"]=> string(2) "CA" } -TypeError: compact(): Argument #1 ($var_name) must be string or array of strings, bool given -TypeError: compact(): Argument #2 must be string or array of strings, int given + +Warning: compact(): Argument #1 must be string or array of strings, bool given in %s on line %d + +Warning: compact(): Argument #2 must be string or array of strings, int given in %s on line %d From 3655199db237ee4763e5ee3d0cb5c82933a6e388 Mon Sep 17 00:00:00 2001 From: David Gebler Date: Sat, 8 May 2021 12:49:13 +0100 Subject: [PATCH 7/8] restore undefined variable test --- ext/standard/tests/array/compact_basic.phpt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ext/standard/tests/array/compact_basic.phpt b/ext/standard/tests/array/compact_basic.phpt index 681f2638cb054..ec9935b56acdc 100644 --- a/ext/standard/tests/array/compact_basic.phpt +++ b/ext/standard/tests/array/compact_basic.phpt @@ -20,6 +20,7 @@ var_dump (compact(array("a", "b", "c", "d", "e", "f"))); // simple parameter test var_dump (compact("a", "b", "c", "d", "e", "f")); var_dump (compact(array("keyval"=>"a", "b"=>"b"))); +var_dump(compact(array("g"))); echo "Done"; ?> @@ -65,4 +66,8 @@ array(2) { ["b"]=> float(0.2) } + +Warning: compact(): Undefined variable $g in %s on line %d +array(0) { +} Done From 23610026fb2d73b446ac057cf270d8dbfebf8b2c Mon Sep 17 00:00:00 2001 From: David Gebler Date: Mon, 10 May 2021 17:51:23 +0100 Subject: [PATCH 8/8] add var dump to test --- ext/standard/tests/array/compact.phpt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ext/standard/tests/array/compact.phpt b/ext/standard/tests/array/compact.phpt index cef94cc588f97..d4c6236c03e47 100644 --- a/ext/standard/tests/array/compact.phpt +++ b/ext/standard/tests/array/compact.phpt @@ -16,6 +16,7 @@ $result = compact(true); $foo = 'bar'; $bar = 'baz'; $result = compact($foo, [42]); +var_dump($result); ?> --EXPECTF-- @@ -30,3 +31,7 @@ array(2) { Warning: compact(): Argument #1 must be string or array of strings, bool given in %s on line %d Warning: compact(): Argument #2 must be string or array of strings, int given in %s on line %d +array(1) { + ["bar"]=> + string(3) "baz" +} \ No newline at end of file