Skip to content

Fixing incorrect error message when passing null to join/implode's array parameter #12683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Dec 1, 2023
Merged

Fixing incorrect error message when passing null to join/implode's array parameter #12683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
469981b
Fixes #12682
CViniciusSDias Nov 15, 2023
af47cdf
Adding correct check and error message
CViniciusSDias Nov 15, 2023
b2494f1
Splitting paths as suggested by @iluuu1994
CViniciusSDias Nov 15, 2023
0581bf1
Fixing parameters parsing
CViniciusSDias Nov 16, 2023
d3524bd
Simplifying the parameter check conditions
CViniciusSDias Nov 16, 2023
737d829
Tests fixed and code approach reverted to the original one
CViniciusSDias Nov 16, 2023
63648cc
Removing duplicated test
CViniciusSDias Nov 16, 2023
48a080e
Rewriting the error message and moving test conditions
CViniciusSDias Nov 17, 2023
b56c486
Removing deprecation test
CViniciusSDias Nov 17, 2023
5ac3431
Fixing tests with the new error message
CViniciusSDias Nov 17, 2023
f5b351e
Using the new error message always with null
CViniciusSDias Nov 20, 2023
2193972
Adding closing tag to test and removing unreacheable statement
CViniciusSDias Nov 21, 2023
e86f567
Reverting checks to original structure, changing only the error messages
CViniciusSDias Nov 22, 2023
95b8db9
Making tests PSR-12 compliant
CViniciusSDias Nov 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ext/standard/string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,11 @@ PHP_FUNCTION(implode)

if (pieces == NULL) {
if (arg1_array == NULL) {
zend_type_error("%s(): Argument #1 ($array) must be of type array, string given", get_active_function_name());
zend_type_error(
"%s(): If argument #1 ($separator) is of type string, "
"argument #2 ($array) must be of type array, null given",
get_active_function_name()
);
RETURN_THROWS();
}

Expand Down
0 ext/standard/tests/strings/implode.phpt → ...standard/tests/strings/implode_basic.phpt
View file
Open in desktop
File renamed without changes.
29 changes: 29 additions & 0 deletions ext/standard/tests/strings/implode_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
Test implode() function: error conditions
--FILE--
<?php
/* only glue */
try {
var_dump(implode("glue"));
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

/* NULL as pieces */
try {
var_dump(implode("glue", NULL));
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

/* integer as glue */
try {
var_dump(implode(12, "pieces"));
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}
?>
--EXPECT--
implode(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given
implode(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given
implode(): Argument #2 ($array) must be of type ?array, string given
78 changes: 15 additions & 63 deletions ext/standard/tests/strings/implode1.phpt → ...dard/tests/strings/implode_variation.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,24 @@ Test implode() function
--FILE--
<?php
echo "*** Testing implode() for basic operations ***\n";
$arrays = array (
$arrays = array(
array(1,2),
array(1.1,2.2),
array(array(2),array(1)),
array(false,true),
array(),
array("a","aaaa","b","bbbb","c","ccccccccccccccccccccc")
array("a","aaaa","b","bbbb","c","ccccccccccccccccccccc"),
array(NULL),
);
/* loop to output string with ', ' as $glue, using implode() */
foreach ($arrays as $array) {
var_dump( implode(', ', $array) );
var_dump(implode(', ', $array));
var_dump($array);
}

echo "\n*** Testing implode() with variations of glue ***\n";
/* checking possible variations */
$pieces = array (
$pieces = array(
2,
0,
-639,
Expand All @@ -30,7 +31,7 @@ $pieces = array (
" ",
"string\x00with\x00...\0"
);
$glues = array (
$glues = array(
"TRUE",
true,
false,
Expand Down Expand Up @@ -72,7 +73,7 @@ try {
echo $exception->getMessage() . "\n";
}
try {
var_dump( implode(2, $sub_array) );
var_dump(implode(2, $sub_array));
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}
Expand All @@ -90,7 +91,7 @@ $obj = new foo(); //creating new object
$arr = array();
$arr[0] = &$obj;
$arr[1] = &$obj;
var_dump( implode(",", $arr) );
var_dump(implode(",", $arr));
var_dump($arr);

/* Checking on resource types */
Expand All @@ -104,51 +105,7 @@ $dir_handle = opendir( __DIR__ );
/* store resources in array for comparison */
$resources = array($file_handle, $dir_handle);

var_dump( implode("::", $resources) );

echo "\n*** Testing error conditions ***\n";

/* only glue */
try {
var_dump( implode("glue") );
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

/* int as pieces */
try {
var_dump( implode("glue",1234) );
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

/* NULL as pieces */
try {
var_dump( implode("glue", NULL) );
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

/* pieces as NULL array */
try {
var_dump( implode(",", array(NULL)) );
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

/* integer as glue */
try {
var_dump( implode(12, "pieces") );
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

/* NULL as glue */
try {
var_dump( implode(NULL, "abcd") );
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}
var_dump(implode("::", $resources));

/* closing resource handles */
fclose($file_handle);
Expand Down Expand Up @@ -214,6 +171,11 @@ array(6) {
[5]=>
string(21) "ccccccccccccccccccccc"
}
string(0) ""
array(1) {
[0]=>
NULL
}

*** Testing implode() with variations of glue ***
-- Iteration 1 --
Expand All @@ -236,7 +198,7 @@ string(35) "2000-639010PHP000 0string%0with%0...%0"
string(43) "2\00\0-639\01\0PHP\0\0\0 \0string%0with%0...%0"

*** Testing implode() on empty string ***
implode(): Argument #1 ($array) must be of type array, string given
implode(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given

*** Testing implode() on sub-arrays ***

Expand Down Expand Up @@ -264,14 +226,4 @@ array(2) {

*** Testing end() on resource type ***
string(%d) "Resource id #%d::Resource id #%d"

*** Testing error conditions ***
implode(): Argument #1 ($array) must be of type array, string given
implode(): Argument #2 ($array) must be of type ?array, int given
implode(): Argument #1 ($array) must be of type array, string given
string(0) ""
implode(): Argument #2 ($array) must be of type ?array, string given

Deprecated: implode(): Passing null to parameter #1 ($separator) of type array|string is deprecated in %s on line %d
implode(): Argument #2 ($array) must be of type ?array, string given
Done
2 changes: 1 addition & 1 deletion ext/standard/tests/strings/join_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ echo "Done\n";
*** Testing join() : error conditions ***

-- Testing join() with less than expected no. of arguments --
join(): Argument #1 ($array) must be of type array, string given
join(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given
Done
11 changes: 5 additions & 6 deletions ...andard/tests/strings/join_variation2.phpt → ext/standard/tests/strings/join_error1.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class test

// array with different values
$values = array (

// integer values
0,
1,
Expand Down Expand Up @@ -84,7 +83,7 @@ for($index = 0; $index < count($values); $index ++) {
$pieces = $values [$index];

try {
var_dump( join($glue, $pieces) );
var_dump(join($glue, $pieces));
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}
Expand Down Expand Up @@ -138,13 +137,13 @@ join(): Argument #2 ($array) must be of type ?array, string given
-- Iteration 18 --
join(): Argument #2 ($array) must be of type ?array, string given
-- Iteration 19 --
join(): Argument #1 ($array) must be of type array, string given
join(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given
-- Iteration 20 --
join(): Argument #1 ($array) must be of type array, string given
join(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given
-- Iteration 21 --
join(): Argument #2 ($array) must be of type ?array, resource given
-- Iteration 22 --
join(): Argument #1 ($array) must be of type array, string given
join(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given
-- Iteration 23 --
join(): Argument #1 ($array) must be of type array, string given
join(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given
Done