php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #36814 ArrayObject is leaking. Seems that OffsetGet should use OffsetExists
Submitted: 2006-03-21 15:50 UTC Modified: 2006-03-22 11:40 UTC
From: raphaelpereira at gmail dot com Assigned: helly (profile)
Status: Not a bug Package: SPL related
PHP Version: * OS: *
Private report: No CVE-ID: None
 [2006-03-21 15:50 UTC] raphaelpereira at gmail dot com
Description:
------------
My code is very complex and I could not reproduce the bug in another code, but the issue is that it seems that ArrayObject::offsetGet doesn't check if the key exists to return it and in some very specific case this returns invalid results.

The problem is in my query_constraints class. On its constructor I declare:

	class query_constraints
	{

		protected $_dados;

		public function __construct ($params=null)
		{
			$this->_dados = new ArrayObject();

			$this->_dados['in']    = new ArrayObject();
			$this->_dados['eq']    = new ArrayObject();
...

Later on I have:

		public function equal($campo, $valor)
		{
			if (!$this->_dados['eq'][$campo] && !$this->_dados['in'][$campo])
			{
...


Both tests returns false on the first call to this method just after object construction.


To workaroud the problem I substituted all references to ArrayObject in this class with the following class:

	class ArrayObject1 extends ArrayObject
	{
		public function offsetget($key)
		{
			if ($this->offsetexists($key))
				return parent::offsetget($key);

			return null;
		}
	}

This solved my problem.


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2006-03-21 16:50 UTC] helly@php.net
Thank you for taking the time to write to us, but this is not
a bug. Please double-check the documentation available at
http://www.php.net/manual/ and the instructions on how to report
a bug at http://bugs.php.net/how-to-report.php

There is no need for offsetGet to first call offsetExists from an engine point of view. If there is a need for this it is either a problem in the way you either overload ArrayObject/ArrayIterator or use them.

Unless you can provide code that demonstrates an issue in ArrayObject itself i assume there is a problem in usage.

Maybe you should note that offsetExists indeed always checks for pure existance rather than working like empty() or isset() do. Even if invoked by them.
 [2006-03-21 17:55 UTC] raphaelpereira at gmail dot com
So, you are telling me (and everybody that uses PHP) that in the following code:

$a = new ArrayObject();
$a['some'] = new ArrayObject();

if ($a['some']['not_set'])
   echo 'Crash!';
else
   echo 'Ok';


I cannot expect the answer 'Ok'? 

Because my code is doing exactly that, but with more complex code before the check, none of them touching the ArrayObject in question.

I reopened the bug because this IS a bug. Even if I'm not using the code correctly, memory is leaking and PHP should not leak, I suppose.
 [2006-03-21 19:38 UTC] helly@php.net
Thank you for this bug report. To properly diagnose the problem, we
need a short but complete example script to be able to reproduce
this bug ourselves. 

A proper reproducing script starts with <?php and ends with ?>,
is max. 10-20 lines long and does not require any external 
resources such as databases, etc.

If possible, make the script source available online and provide
an URL to it here. Try to avoid embedding huge scripts into the report.

Given your code i cannot reproduce any leak or failure in the code with 5.1.3RC2-dev nor CVS, not even using memcheck.

But internally i uses a few things that touch code we have changed a slight bit. So maybe you hit an edgcase by your real code here.

Besides that you are of course correct to expect Ok and so does the reproducing code.
 [2006-03-21 23:45 UTC] raphaelpereira at gmail dot com
I think I could isolate the bug. Check the following code:

<?php

	class crash
	{
		protected $array;
		protected $mem;
		
		function nothing()
		{
			$this->array = new ArrayObject();
			$this->mem = new ArrayObject();
			$this->mem['empty'] = new ArrayObject();
		}

		function __get($field)
		{
			$field = strtolower($field);
			return $this->array[$field];
		}

		function test()
		{
			if ($this->mem['empty']['crash'])
				return true;

			return false;
		}
	}

	$crash = new crash();

	$crash->break_mem['here0'] = 'This is to break.';
	$crash->break_mem['here1'] = 'This is to break.';
	$crash->break_mem['here2'] = 'This is to break.';
	$crash->break_mem['here3'] = 'This is to break.';
	$crash->break_mem['here4'] = 'This is to break.';
	
	if ($crash->test())
		echo 'BUUUMMMMM!';

?>

This code should show nothing, but it showed BUUUMMMM! here. If you cannot make it show BUM, increase the number of calls to $crash->break_mem['here4'] = 'This is to break.';

Hope that is enough
 [2006-03-22 11:40 UTC] tony2001@php.net
The issue isn't related to SPL actually.
Duplicate of bug #36484.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 21 14:01:32 2024 UTC