I was playing around with Bug #28817 and seem to have run into an issue when overriding the read_property handler with an extended object (this case a DomDocument object).
In order to support standard properties correctly, such as $obj->myprop['a'] = 'b', the internal object needs to support get_property_ptr_ptr. As the internal object here overrides the read_property handler, it would also need to override the get_property_ptr_ptr handler to make that standard property work correctly, which is where I have run into some problems.
At first, I implemented a get_property_ptr_ptr handler, but found that for the base internal class properties, this method needs to return NULL so that cases like zend_pre_incdec_property would fall back to the read_property/write_property handling. Next was to return NULL for all properties of the base internal class and return the result of the standard get_property_ptr_ptr for non base class properties. This fixed the standard properties, but returning NULL for the internal base properties breaks calls like:
$document->documentElement->ownerDocument;
because zend_fetch_property_address_inner will not fall back to the read_property handler if NULL is returned get_property_ptr_ptr.
Rather the error "Cannot access undefined property for object with overloaded property access" is returned.
Is this is a pitfall of the engine or is there something I am overlooking which would allow this to work across all cases?
Thanks,
Rob
Hello Rob,
Tuesday, July 6, 2004, 12:15:31 PM, you wrote:
I was playing around with Bug #28817 and seem to have run into an issue
when overriding the read_property handler with an extended object (this
case a DomDocument object).
In order to support standard properties correctly, such as
$obj->myprop['a'] = 'b', the internal object needs to support
get_property_ptr_ptr. As the internal object here overrides the
read_property handler, it would also need to override the
get_property_ptr_ptr handler to make that standard property work
correctly, which is where I have run into some problems.
At first, I implemented a get_property_ptr_ptr handler, but found that
for the base internal class properties, this method needs to returnNULL
so that cases like zend_pre_incdec_property would fall back to the
read_property/write_property handling. Next was to returnNULLfor all
properties of the base internal class and return the result of the
standard get_property_ptr_ptr for non base class properties. This fixed
the standard properties, but returningNULLfor the internal base
properties breaks calls like:
$document->documentElement->ownerDocument;
because zend_fetch_property_address_inner will not fall back to the
read_property handler ifNULLis returned get_property_ptr_ptr.
Rather the error "Cannot access undefined property for object with
overloaded property access" is returned.
Is this is a pitfall of the engine or is there something I am
overlooking which would allow this to work across all cases?
Sounds more like small oversight. Could you try to make a patch for that?
Best regards,
Marcus mailto:helly@php.net
Attached is a patch for zend_execute.c which will fall to read_property if
get_property_ptr_ptr returns NULL in zend_fetch_property_address_inner.
Hopefully this is the correct fix as it didnt break any tests and resolves
the issue I hit.
Rob
From: Marcus Boerger
Sounds more like small oversight. Could you try to make a patch for that?
At 16:32 07/07/2004, Rob Richards wrote:
Attached is a patch for zend_execute.c which will fall to read_property if
get_property_ptr_ptr returnsNULLin zend_fetch_property_address_inner.
Hopefully this is the correct fix as it didnt break any tests and resolves
the issue I hit.
It's not a correct fix :I get_ptr_ptr must return the address of an
allocated zval *, one which can later be separated,
etc. T(result->u.var).var.ptr doesn't fall into that category - it can be
gone in the next opcode...
The only safe way to return a zval ** is for the underlying object model to
implement get_ptr_ptr...
Zeev
From: Zeev Suraski
It's not a correct fix :I get_ptr_ptr must return the address of an
allocated zval *, one which can later be separated,
etc. T(result->u.var).var.ptr doesn't fall into that category - it can be
gone in the next opcode...The only safe way to return a zval ** is for the underlying object model
to
implement get_ptr_ptr...
What is this then a catch 22? If get_ptr_ptr is implemented and returns an
allocated zval **, then you can run into problems using pre/post_incdec if
you need to rely on the read_property and write_property of those functions,
which are only hit if you return NULL from get_ptr_ptr, which then brings
one back full circle to getting an error in
zend_fetch_property_address_inner when returning a NULL zval **.
So, my question boils down to is this a problem in the engine, the extension
or a combination of both.
Thanks,
Rob
At 22:27 07/07/2004, Rob Richards wrote:
From: Zeev Suraski
It's not a correct fix :I get_ptr_ptr must return the address of an
allocated zval *, one which can later be separated,
etc. T(result->u.var).var.ptr doesn't fall into that category - it can be
gone in the next opcode...The only safe way to return a zval ** is for the underlying object model
to
implement get_ptr_ptr...What is this then a catch 22? If get_ptr_ptr is implemented and returns an
allocated zval **, then you can run into problems using pre/post_incdec if
you need to rely on the read_property and write_property of those functions,
which are only hit if you returnNULLfrom get_ptr_ptr, which then brings
one back full circle to getting an error in
zend_fetch_property_address_inner when returning aNULLzval **.
I guess so, yes, it's a bit of a catch 22. get_ptr_ptr is used in very
rare cases - assign-by-reference is one of them, and pre/post incdec is
another. The assumption is that if you provide get_ptr_ptr, the engine can
perform incdec on its own, without calling read_/write_property. It's
basically an optimization for standard PHP objects. So if you want to
support assign-by-ref and also override incdec, I guess it cannot be done
right now.
The correct solution, I think, would be adding a boolean function (or even
just a bit) to the handlers structure, that instructs the engine whether it
should use get_ptr_ptr for incdec or not, since it's an
optimization. Maybe we can just check whether we're dealing with PHP
objects and perform this optimization only then. I need to look into it.
Zeev
Zeev Suraski <zeev <at> zend.com> writes:
The correct solution, I think, would be adding a boolean function (or even
just a bit) to the handlers structure, that instructs the engine whether it
should use get_ptr_ptr for incdec or not, since it's an
optimization. Maybe we can just check whether we're dealing with PHP
objects and perform this optimization only then. I need to look into it.Zeev
I just wanted to point you to bug http://bugs.php.net/bug.php?id=28444 which
would get fixed with this patch as well, so only fixing this for post/incdec is
probably a bad idea :)
I think this should be targeted to be fixed for the 5.0 release because i ran
across this behaviour very quick after using __get & __set and it could produce
a bit confusion
From: Zeev Suraski
The correct solution, I think, would be adding a boolean function (or even
just a bit) to the handlers structure, that instructs the engine whether
it
should use get_ptr_ptr for incdec or not, since it's an
optimization. Maybe we can just check whether we're dealing with PHP
objects and perform this optimization only then. I need to look into it.
I dont see the problem as being with incdec. In my particular case at least,
I dont want to implement get_ptr_ptr. The only reason it was needed was to
support properties on an extended object which are not from the base class.
All the properties of the base class want/need to be handled with the
read/write_property handlers. If a property is not of the base class, then
get_ptr_ptr is needed so that the standard get_ptr_ptr handler can be called
for those properties, while all base class properties would return NULL from
the get_ptr_ptr handler (thus in most cases defaulting to calling the
read_property handler).
zend_fetch_property_address_inner is the only function that does not fall
back on the read_property if NULL is returned from get_ptr_ptr. Looking at
the other bug mentioned (28444), standard objects also fall into a similar
problem I hit when returning NULL from get_ptr_ptr.
zend_std_get_property_ptr_ptr assumes that if it returns NULL, then it
should try with the normal __get handler (which also errors out since
zend_fetch_property_address_inner doesnt fall back to the other handler).
If there is a problem with always forcing zend_fetch_property_address_inner
to use the read_property handler when get_ptr_ptr returns NULL, it should at
least allow it to fall back to the read_property handler when the object is
not a standard object. In this case the developer knows they are returning
NULL or a zval ** for a reason and are handling things in the read_property
handler if needed, though it still leaves open the issue with standard
objects.
Rob