php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #23690 Config_Container::toArray() Bug + FIX :-)
Submitted: 2003-05-18 21:38 UTC Modified: 2003-06-14 20:04 UTC
From: info at rhalff dot com Assigned: mansion (profile)
Status: Closed Package: PEAR related
PHP Version: 4.3.2RC3 OS: All
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: info at rhalff dot com
New email:
PHP Version: OS:

 

 [2003-05-18 21:38 UTC] 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

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2003-05-18 21:46 UTC] 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-19 04:38 UTC] 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-19 09:35 UTC] 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 10:01 UTC] mansion@php.net
> 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.
 [2003-05-19 10:11 UTC] info at rhalff dot com
Ok.. my fix doesn't work either :|
 [2003-05-19 10:37 UTC] info at rhalff dot com
Ok your right, with the test code I gave for reproduction of this bug the fix works. But if I change the type from 'inicommented' to 'inifile' the array will only contain 1 'extension'.

Is this expected behaviour or a bug ?
 [2003-05-19 12:16 UTC] mansion@php.net
Inicommented uses my own parser. Inifile uses the php function parse_ini_file() which is very fast but not as complete.

My parser is quite slow but keeps track of comments, blank lines, duplicates...

I don't think parse_ini_file() allows duplicate entries.

 [2003-05-19 13:30 UTC] info at rhalff dot com
Ok, so that's kind of excpected behaviour then.

To me te fix seems to work, there's only one unlogical thing left in there.

  if (isset($array[$this->name][$key])) {
    // duplicate name/type
    .......
  } elseif (isset($array[$this->name][$key][0])) {
    
The elseif will never be reached, since isset($array[$this->name][$key]) will always be true first.
 [2003-06-14 20:04 UTC] mansion@php.net
This bug has been fixed in CVS.

In case this was a PHP problem, snapshots of the sources are packaged
every three hours; this change will be in the next snapshot. You can
grab the snapshot at http://snaps.php.net/.
 
In case this was a documentation problem, the fix will show up soon at
http://www.php.net/manual/.

In case this was a PHP.net website problem, the change will show
up on the PHP.net site and on the mirror sites in short time.
 
Thank you for the report, and for helping us make PHP better.


 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Mar 28 23:01:26 2024 UTC