Fixed bug #75902
authorNikita Popov <nikita.ppv@gmail.com>
Tue, 10 Mar 2020 15:49:17 +0000 (16:49 +0100)
committerNikita Popov <nikita.ppv@gmail.com>
Tue, 10 Mar 2020 15:49:17 +0000 (16:49 +0100)
Don't special-case nested arrays/objects in str_replace(), instead
perform a string cast on them as well. For arrays, this will always
result in the usual conversion warning.

This behavior is consistent with preg_replace(). If we didn't want
to cast the array to string here, we should instead perform the
replacement recursively. Silently copying it is just confusing.

NEWS
ext/standard/string.c
ext/standard/tests/strings/bug25671.phpt
ext/standard/tests/strings/bug71969.phpt
ext/standard/tests/strings/str_replace_variation1.phpt

diff --git a/NEWS b/NEWS
index 2c30043c71d1f61ef8b6441918d8584ec95d8c9b..6a871ab37269aeec3de7a68d87430034f16dede4 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -119,6 +119,8 @@ PHP                                                                        NEWS
     filter). (kkopachev)
   . Fixed bug #78385 (parse_url() does not include 'query' when question mark
     is the last char). (Islam Israfilov)
+  . Fixed bug #75902 (str_replace should warn when misused with nested arrays).
+    (Nikita)
   . Made quoting of cmd execution functions consistent. (cmb)
 
 - tidy:
index ec397fe97c216ba205310092eeff8bf9661ee0bb..363117fce86a43ef501826bcf5ffb0748d01ecc0 100644 (file)
@@ -4326,15 +4326,12 @@ static void php_str_replace_common(INTERNAL_FUNCTION_PARAMETERS, int case_sensit
                /* For each subject entry, convert it to string, then perform replacement
                   and add the result to the return_value array. */
                ZEND_HASH_FOREACH_KEY_VAL(subject_ht, num_key, string_key, subject_entry) {
+                       zend_string *tmp_subject_str;
                        ZVAL_DEREF(subject_entry);
-                       if (Z_TYPE_P(subject_entry) != IS_ARRAY && Z_TYPE_P(subject_entry) != IS_OBJECT) {
-                               zend_string *tmp_subject_str;
-                               subject_str = zval_get_tmp_string(subject_entry, &tmp_subject_str);
-                               count += php_str_replace_in_subject(search, replace, subject_str, &result, case_sensitivity);
-                               zend_tmp_string_release(tmp_subject_str);
-                       } else {
-                               ZVAL_COPY(&result, subject_entry);
-                       }
+                       subject_str = zval_get_tmp_string(subject_entry, &tmp_subject_str);
+                       count += php_str_replace_in_subject(search, replace, subject_str, &result, case_sensitivity);
+                       zend_tmp_string_release(tmp_subject_str);
+
                        /* Add to return array */
                        if (string_key) {
                                zend_hash_add_new(Z_ARRVAL_P(return_value), string_key, &result);
index cd6dec6641e283de6d1417e53427333913efa9b9..c564fac2c0a507a23a676ce6bba2ef211cfdaaeb 100644 (file)
@@ -15,8 +15,15 @@ Bug #25671 (subarrays not copied correctly)
   echo serialize(str_replace(" ", "", $arr)) . "\n";
   echo serialize(str_replace(" ", "", $arr)) . "\n";
 ?>
---EXPECT--
-a:4:{i:0;s:19:"This is strung one.";i:1;s:19:"This is strung two.";i:2;a:2:{i:0;s:23:"This is another string.";i:1;s:22:"This is a last string.";}i:3;s:22:"This is a last strung.";}
-a:4:{i:0;s:19:"This is strung one.";i:1;s:19:"This is strung two.";i:2;a:2:{i:0;s:23:"This is another string.";i:1;s:22:"This is a last string.";}i:3;s:22:"This is a last strung.";}
-a:4:{i:0;s:16:"Thisisstringone.";i:1;s:16:"Thisisstringtwo.";i:2;a:2:{i:0;s:23:"This is another string.";i:1;s:22:"This is a last string.";}i:3;s:18:"Thisisalaststring.";}
-a:4:{i:0;s:16:"Thisisstringone.";i:1;s:16:"Thisisstringtwo.";i:2;a:2:{i:0;s:23:"This is another string.";i:1;s:22:"This is a last string.";}i:3;s:18:"Thisisalaststring.";}
+--EXPECTF--
+Warning: Array to string conversion in %s on line %d
+a:4:{i:0;s:19:"This is strung one.";i:1;s:19:"This is strung two.";i:2;s:5:"Array";i:3;s:22:"This is a last strung.";}
+
+Warning: Array to string conversion in %s on line %d
+a:4:{i:0;s:19:"This is strung one.";i:1;s:19:"This is strung two.";i:2;s:5:"Array";i:3;s:22:"This is a last strung.";}
+
+Warning: Array to string conversion in %s on line %d
+a:4:{i:0;s:16:"Thisisstringone.";i:1;s:16:"Thisisstringtwo.";i:2;s:5:"Array";i:3;s:18:"Thisisalaststring.";}
+
+Warning: Array to string conversion in %s on line %d
+a:4:{i:0;s:16:"Thisisstringone.";i:1;s:16:"Thisisstringtwo.";i:2;s:5:"Array";i:3;s:18:"Thisisalaststring.";}
index 9795186e15272939e3e114858151ae2c2a4bb9ea..5d2153c39320e735fa0d0c2f287f76576dd32128 100644 (file)
@@ -13,16 +13,9 @@ foreach($a as &$record)
 }
 var_dump(str_replace("2", "3", $a));
 ?>
---EXPECT--
+--EXPECTF--
+Warning: Array to string conversion in %s on line %d
 array(1) {
   [0]=>
-  array(1) {
-    ["one"]=>
-    array(2) {
-      ["a"]=>
-      string(4) "2222"
-      ["b"]=>
-      string(4) "1111"
-    }
-  }
+  string(5) "Array"
 }
index 2e6dd18b94fe56e771ea5903d4a58187096006c3..78b483ad934c663fca7039ce7490d56436c3999e 100644 (file)
@@ -27,9 +27,11 @@ foreach( $search_arr as $value ) {
 }
 
 ?>
---EXPECT--
+--EXPECTF--
 *** Testing str_replace() with various search values ***
 -- Iteration 0 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(5) "FOUND"
@@ -50,8 +52,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -60,6 +61,8 @@ array(12) {
 int(5)
 
 -- Iteration 1 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -80,8 +83,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -90,6 +92,8 @@ array(12) {
 int(0)
 
 -- Iteration 2 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(5) "FOUND"
@@ -110,8 +114,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -120,6 +123,8 @@ array(12) {
 int(5)
 
 -- Iteration 3 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -140,8 +145,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -150,6 +154,8 @@ array(12) {
 int(2)
 
 -- Iteration 4 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -170,8 +176,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -180,6 +185,8 @@ array(12) {
 int(2)
 
 -- Iteration 5 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(5) "FOUND"
@@ -200,8 +207,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -210,6 +216,8 @@ array(12) {
 int(5)
 
 -- Iteration 6 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -230,8 +238,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -240,6 +247,8 @@ array(12) {
 int(2)
 
 -- Iteration 7 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -260,8 +269,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -270,6 +278,8 @@ array(12) {
 int(2)
 
 -- Iteration 8 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -290,8 +300,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -300,6 +309,8 @@ array(12) {
 int(0)
 
 -- Iteration 9 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -320,8 +331,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>
@@ -330,6 +340,8 @@ array(12) {
 int(0)
 
 -- Iteration 10 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -350,8 +362,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(5) "FOUND"
   [11]=>
@@ -360,6 +371,8 @@ array(12) {
 int(1)
 
 -- Iteration 11 --
+
+Warning: Array to string conversion in %s on line %d
 array(12) {
   [0]=>
   string(1) "1"
@@ -380,8 +393,7 @@ array(12) {
   [8]=>
   string(0) ""
   [9]=>
-  array(0) {
-  }
+  string(5) "Array"
   [10]=>
   string(3) "php"
   [11]=>