#23690 [Asn]: Config_Container::toArray() Bug + FIX :-)
| From: | mansion@php.net | Date: | Mon, 19 May 2003 15:01:18 +0000 |
| Subject: | #23690 [Asn]: Config_Container::toArray() Bug + FIX :-) | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-16469@lists.php.net to get a copy of this message | ||
ID: 23690
Updated by: mansion@php.net
Reported By: info at rhalff dot com
Status: Assigned
Bug Type: PEAR related
Operating System: All
PHP Version: 4.3.2RC3
Assigned To: mansion
New Comment:
> Hmm your fix will cause:
> [PHP]
> extension = bcmath.so
> extension = bz2.so
> extension = calendar.so
> extension = cdpf.so
> extension = crack.so
> extension = curl.so
Array
(
[root] => Array
(
[PHP] => Array
(
[extension] => curl.so
)
)
)
No it won't, you should test it.
But I will wait to see if you can come up with a better solution before
I commit anything. Let me know,
Thanks.
Previous Comments:
------------------------------------------------------------------------
[2003-05-19 09:35:49] info at rhalff dot com
Hmm your fix will cause:
[PHP]
extension = bcmath.so
extension = bz2.so
extension = calendar.so
extension = cdpf.so
extension = crack.so
extension = curl.so
Array
(
[root] => Array
(
[PHP] => Array
(
[extension] => curl.so
)
)
)
To me it seems the usage of [0] in the code is wrong.
Since it can both mean the first character of a string as
the first item in an array.
For example try:
$string = "Hi there";
echo $string[0];
$string[0] = "other value";
echo $string;
I think the code should be something like this:
650: foreach ($newArr as $key => $value) {
651: if (isset($array[$this->name][$key])) {
652: // duplicate name/type
if(!is_array($array[$this->name][$key])) {
$old = $array[$this->name][$key];
unset($array[$this->name][$key]);
$array[$this->name][$key][0] = $old;
}
$array[$this->name][$key][] = $value;
661: } else {
662: $array[$this->name][$key] = $value;
663: }
664: }
I was wrong with the else part in my earlier code fix.
Im not very sure my fix is good enough, so I will try
to find if there is a better sollution.
------------------------------------------------------------------------
[2003-05-19 04:38:30] mansion@php.net
Hi Rob,
The problem with your solution is that it breaks this:
require_once('Config.php');
$array['a']['b'][0]['foo']="value1";
$array['a']['b'][1]['foo']="value2";
$phpIni = new Config();
$root =& $phpIni->parseConfig($array, 'phparray');
echo '<pre>';
var_dump($root->toArray());
echo '</pre>';
Here is the solution I suggest instead:
foreach ($newArr as $key => $value) {
if (isset($array[$this->name][$key])) {
// duplicate name/type
if (!is_array($array[$this->name][$key]) ||
!isset($array[$this->name][$key][0])) {
$old = $array[$this->name][$key];
unset($array[$this->name][$key]);
$array[$this->name][$key][0] = $old;
}
$array[$this->name][$key][] = $value;
} elseif (isset($array[$this->name][$key][0])) {
$array[$this->name][$key][] = $value;
} else {
$array[$this->name][$key] = $value;
}
}
Please let me know if this would make it for you.
Thanks for the bug report.
Bertrand Mansion
Mamasam
------------------------------------------------------------------------
[2003-05-18 21:46:43] info at rhalff dot com
Somewhat more readable code:
Solution:
Original Code:
650: foreach ($newArr as $key => $value) {
651: if (isset($array[$this->name][$key])) {
652: // duplicate name/type
653: if (!isset($array[$this->name][$key][0])) {
654: $old = $array[$this->name][$key];
655: unset($array[$this->name][$key]);
656: $array[$this->name][$key][0] = $old;
657: }
658: $array[$this->name][$key][] = $value;
659: } elseif (isset($array[$this->name][$key][0])) {
660: $array[$this->name][$key][] = $value;
661: } else {
662: $array[$this->name][$key] = $value;
663: }
664: }
Fixed Code:
650: foreach ($newArr as $key => $value) {
651: if (isset($array[$this->name][$key])) {
652: // duplicate name/type
if(!is_array($array[$this->name][$key])) {
$old = $array[$this->name][$key];
unset($array[$this->name][$key]);
$array[$this->name][$key][0] = $old;
} else {
$array[$this->name][$key][] = $value;
}
661: } else {
662: $array[$this->name][$key] = $value;
663: }
664: }
------------------------------------------------------------------------
[2003-05-18 21:38:08] info at rhalff dot com
Reproduction of this bug:
1. Create the two files below:
-------------------------
php_snippet.ini:
[PHP]
extension = bcmath.so
extension = bz2.so
extension = calendar.so
extension = cdpf.so
extension = crack.so
extension = curl.so
-------------------------
<?php
require_once('Config.php');
$datasrc = 'php_snippet.ini';
$phpIni = new Config();
$root =& $phpIni->parseConfig($datasrc, 'inicommented');
if (PEAR::isError($root)) {
die($root->getMessage());
}
print_r($root->toArray());
?>
------------------------
2. Run test.php
The result will be:
Fatal error: [] operator not supported for strings in
/usr/lib/php/Config/Container.php on line 658
Solution:
Original Code:
650: foreach ($newArr as $key => $value) {
651: if
(isset($array[$this->name][$key])) {
652: // duplicate name/type
653: if
(!isset($array[$this->name][$key][0])) {
654: $old =
$array[$this->name][$key];
655:
unset($array[$this->name][$key]);
656:
$array[$this->name][$key][0] = $old;
657: }
658: $array[$this->name][$key][] =
$value;
659: } elseif
(isset($array[$this->name][$key][0])) {
660: $array[$this->name][$key][] =
$value;
661: } else {
662: $array[$this->name][$key] =
$value;
663: }
664: }
Fixed Code:
650: foreach ($newArr as $key => $value) {
651: if
(isset($array[$this->name][$key])) {
652: // duplicate name/type
if(!is_array($array[$this->name][$key])) {
$old =
$array[$this->name][$key];
unset($array[$this->name][$key]);
$array[$this->name][$key][0] =
$old;
} else {
$array[$this->name][$key][] =
$value;
}
661: } else {
662: $array[$this->name][$key] =
$value;
663: }
664: }
Explaination:
The original code checks if $array[$this->name][$key][0] is set
and falsly assumes that if [0] exists it will be an array.
In the example first it will set $array['PHP']['extension'] =
bcmath.so
The second time around it will detect that $array['PHP']['extension']
allready exists.
But $array['PHP']['extension'][0] will be 'b' as
$array['PHP']['extension'][1] will be 'c' and
$array['PHP']['extension'][2] will be 'm' and
$array['PHP']['extension'][3] will be 'a' and
$array['PHP']['extension'][4] will be 't' and
$array['PHP']['extension'][5] will be 'h' etc..
So when the code reaches $array[$this->name][$key][] = $value;
$array[$this->name][$key] will still be a string and this explains the
error message.
That's why the check should be like:
!is_array($array[$this->name][$key])
The rest of the changes in the Fixed code should be obvious.
Greetings,
Rob Halff
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/?id=23690&edit=1