php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #35104 __set() and read-only property prevents proper Class inheritence
Submitted: 2005-11-04 11:49 UTC Modified: 2005-12-01 13:19 UTC
From: php at tjworld dot net Assigned: dmitry (profile)
Status: Not a bug Package: Class/Object related
PHP Version: 5.1.0RC5 OS: Windows 2003
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: php at tjworld dot net
New email:
PHP Version: OS:

 

 [2005-11-04 11:49 UTC] php at tjworld dot net
Description:
------------
When extending a class that has a visible (public or protected) read-only dynamic property by virtue of __set(), the sub-class is prevented from modifying (or "overloading") the property as if it were merely a public consumer of an instance-object of super, rather than an extension of the class definition itself.

This shows up in particular in the DOM classes DOMNode and DOMDocument, where it causes:

Fatal error: extDOMElement::__construct() [function.--construct]: Cannot write property in...

If you extend both classes, and try to create a new derived extDOMNode object using a custom  extDOMDocument->createElement(), it is impossible to set any of the new extDOMNode's dynamic properties from within extDOMNode's constructor (especially ownerDocument) because DOMNodes without an owner are forced to be read-only.

The extDOMNode class definition should be able to modify the publically visible ownerDocument property.

Since real properties accessible from a sub-class can't be private (if they're to be accessible from other objects), it follows that the same rule should apply to dynamic properties. If this were so the dynamic properties made visible by __set() would be inherited as protected or public  and this issue wouldn't arise.

Reproduce code:
---------------
class ReadOnly {
 protected $realProperty;
 private $dynamicProperty = array();
 function __construct() {
  $realProperty = 12;
  $this->members['test'] = 'read-only';
 }
 public function __set($name, $value) {
  if($name=='test') {
   if(isset($this->dynamicProperty[$name]))
    throw new Exception("Can't overwrite $name");  

   $props[$name] = $value;
  }
 }
}
class Writeable extends ReadOnly {
 function __construct($value) {
  parent::__construct();
  $this->realProperty = 25; // ok
  $this->test = $value; // causes Fatal Error
 }
}

$test = new Writeable('write to me');

Expected result:
----------------
The extended class should be able to inherit and modify protected or public properties of the super class.

Actual result:
--------------
For built-in classes causes a Fatal Error. For user defined classes causes a user-defined read-only result. In the example causes an Exception.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2005-11-04 12:38 UTC] sniper@php.net
Please try using this CVS snapshot:

  http://snaps.php.net/php5-latest.tar.gz
 
For Windows:
 
  http://snaps.php.net/win32/php5-win32-latest.zip


 [2005-11-04 13:02 UTC] php at tjworld dot net
C:\PHP\5.1.0RC5-dev>php.exe test.php
testing...
Fatal error: Uncaught exception 'Exception' with message 'Can't overwrite test' in C:\test.php:12
Stack trace:
#0 C:\test.php(23): ReadOnly::__set('test', 'write to me')
#1 C:\test.php(30): Writeable->__construct('write to me')
#2 {main}
  thrown in C:\test.php on line 12


---------------test.php--------------
<?php
class ReadOnly {
 protected $realProperty;
 private $dynamicProperty = array();
 function __construct() {
  $realProperty = 12;
  $this->dynamicProperty['test'] = 'read-only';
 }
 public function __set($name, $value) {
  if($name=='test') {
   if(isset($this->dynamicProperty[$name]))
    throw new Exception("Can't overwrite $name");

   $dynamicProperty[$name] = $value;
  }
 }
 public function __get($name) { return $this->dynamicProperty[$name]; }
}
class Writeable extends ReadOnly {
 function __construct($value) {
  parent::__construct();
  $this->realProperty = 25; // ok
  $this->test = $value; // causes Fatal Error
 }
 public function getReal() {return $this->realProperty; }
 public function getDynamic() {return $this->test; }
}

echo "testing...\r\n";
$test = new Writeable('write to me');
echo 'real: '.$test->getReal()."\r\n";
echo 'dynamic: '.$test->getDynamic()."\r\n";
?>
 [2005-11-04 13:27 UTC] php at tjworld dot net
Further test using DOMDocument/DOMElement...

C:\PHP\5.1.0RC5-dev>php.exe dom.php

Fatal error: extDOMElement::__construct(): Cannot write property in C:\dom.php on line 14

----------dom.php-------------
<?php
class extDOMDocument extends DOMDocument {
 public function createElement($name, $value=null) {
  $ret = new extDOMElement($name, $value, $this); // create the new element with this Document as owner
  return $ret;
 }
}

class extDOMElement extends DOMElement {
 function __construct($name, $value='', $owner=null, $namespaceURI=null) {
  if(!$owner instanceof extDOMDocument)
   throw new DOMException(DOM_NOT_FOUND_ERR); // illegal owner
  parent::__construct($name, $value, $namespaceURI);
  $this->ownerDocument = $owner; //** this line causes a Fatal Error
 }
  //  ... more class definition here
}

$doc = new extDOMDocument('test');
$el = $doc->createElement('tagname');
?>
 [2005-11-04 16:26 UTC] sniper@php.net
Dmitry, any insight on this?

 [2005-11-04 17:02 UTC] rrichards@php.net
I can't speak for the user class example, but DOM properties CANNOT be overriden.
 [2005-11-04 21:39 UTC] php at tjworld dot net
"but DOM properties CANNOT be overriden."

Does this occur anywhere else in the PHP classes or is it unique to DOM? It's the first time I've met this situation in OO since the 80's.

It pretty much makes having the DOM object-oriented pointless, when the base class (DOMNode) of the other significant DOM classes prevents useful extension.

A simple solution would be to provide a courtesy:

DOMNode->__construct($ownerDocument = null);

But that'd be available to the public of course. Alternatively, 

protected DOMNode function _setOwnerDocument(DOMDocument ownerDocument);

But thats a bit arbitary. Alternatively, solve the practical loss of functionality by fixing the bug in importNode() so it returns an object of the class passed in:

DOMNode DOMDocument->importNode(DOMNode $node, bool deep);

Currently it *casts* the passed $node to one of the DOM base classes it inherited from *and* discards all their extended properties and methods, which is surely not OO behaviour because in the following scenario, the cases listed at the end are inconsistent:

class inChild extends DOMNode {}
class inGrandChild extends DOMElement()
class inGreatGrandChild extends inGrandChild()

$node = new DOMNode();
$element = new DOMElement();
$child = new inChild();
$grandChild = new inGrandChild();
$greatGrandChild = new inGreatGrandChild();

1. DOMDocument->importNode($node, true) instanceof DOMNode
2. DOMDocument->importNode($element, true) instanceof DOMElement
3. DOMDocument->importNode($child, true) instance of DOMNode
4. DOMDocument->importNode($grandChild, true) instanceof DOMElement (not inGrandChild)
5. DOMDocument->importNode($greatGrandChild, true) instanceof DOMElement (not inGreatGrandChild)

So importNode() doesn't even cast to a consistent DOMNode, but to the 'highest* level in the built-in classes.

Usually in OO although the cast is to a super-class (to guarantee portability) the extended methods and properties aren't discarded.

If importNode() were fixed to return the same class as passed in the following code would solve the ownerDocument issue:

<?php
class extDOMDocument extends DOMDocument {
 public function createElement($name, $value=null) {
  $orphan = new extDOMElement($name, $value);
  $adopt = $this->importNode($ret, true); // adopt it
  // now $adopt satisfies $this->isSameNode($adopt->ownerDocument) && $adopt instanceof extDOMelement 
  return $adopt;
 }
}

class extDOMElement extends DOMElement {
 function __construct($name, $value=null, $namespace=null) {
  parent::__construct($name, $value, $namespaceURI);
 }
  //  ... more class definition here
}

(excuse any typo's - working weird hours!)
 [2005-11-04 22:54 UTC] rrichards@php.net
Cant say about other internal classes, but DOM is written specifically not to allow overriding properties (properties provide direct access to libxml2 functionality which cant be done in userland). As far as casting goes... already working on a way to allow returning extended classes from all functions in dom (for PHP 6).

Current workaround for your issue create your extended class using new keyword, and append it to a DOMDocumentFragment (if you dont want it linked into the document tree) that was created using createDocumentFragment. The appending updates the libxml2 pointers and the extended object is now linked to the DOMDocument correctly.

Was this report specific to dom or is it to remain open for the extneded uses object example?
 [2005-11-04 23:32 UTC] php at tjworld dot net
Thanks for the DOM-specific recommendations, I'll have a play about with it.

The case for dynamic properties is a big issue in itself, since when extending a class the developer doesn't always have control of the internal design on the super-class.

This could lead to some frustrating situations where the developer has every reasonable expectation they can extend the super-class, but in use trying to modify a protected or public property causes this Fatal Error.

It could also lead to intermitent results if the statement  attempting the modification is in a little-used method of the sub-class, and only occurs on rare occasions.

Without compile-time access-checking (unlike a strongly typed pre-compiled environment) these kind of issues could occur easily, especially for the more general developers who aren't so au-fait with the technicalities of PHP OO dynamic properties.

I was wondering if the properties affected by __set() should be marked final, which would prevent inconsistencies but reduce functionality to a great extent.

Maybe its a case of stressing in the documentation *not* to rely on __set() to produce read-only properties *unless* the __set() method first checks that the calling class-instance is an instanceof the super-class - in other words the read-only functionality is conditional on the method being called on an instance of the super-class itself, not a sub-class.

This puts the ball back in the developer's court, but you'd need some good strong documentation across the platform to ensure developers use this design template.
 [2005-11-05 00:02 UTC] php at tjworld dot net
Following on from my suggestion to provide a strong design template for read-only properties and inheritence, I've put together the following example.

It provides for inheritence of a read-only property so the property can be modified from sub-classes.

<?php
class ReadOnly {
 const CLASS_READ_ONLY_PROPERTY_ERR = 1;
 protected $realProperty;
 private $dynamicProperty = array();
 function __construct() {
  $this->realProperty = 12;
  $this->test = 'read-only';
 }
 public function __set($name, $value) {
  if($name=='test') {
   if(get_class($this)==__CLASS__ && isset($this->dynamicProperty[$name]))
    throw new Exception(self::CLASS_READ_ONLY_PROPERTY_ERR);
   else
    $this->dynamicProperty[$name] = $value;
  }
 }
 public function __get($name) { return $this->dynamicProperty[$name]; }

 public function getReal() {return $this->realProperty; }
 public function getDynamic() {return $this->test; }
}

class Writeable extends ReadOnly {
 function __construct($value) {
  parent::__construct();
  $this->realProperty = 25; // ok
  $this->test = $value; // causes Fatal Error
 }
}

echo "Testing Writeable...\r\n";
$test = new Writeable('write to me');
echo 'real: '.$test->getReal()."\r\n";
echo 'dynamic: '.$test->getDynamic()."\r\n";

echo "Testing ReadOnly...\r\n";
$test = new ReadOnly();
echo 'real: '.$test->getReal()."\r\n";
echo 'dynamic: '.$test->getDynamic()."\r\n";
try {
  $test->test = "can't change me";
}
catch(Exception $e) {
  if ($e->getMessage() == ReadOnly::CLASS_READ_ONLY_PROPERTY_ERR) echo "Read-only Property Exception";
}
?>

Thanks for your prompt and considered attention to this issue. Hopefully this provides a solution that elegantly solves the issue for all concerned.

TJ.
Nottingham, UK
 [2005-11-05 01:00 UTC] php at tjworld dot net
And finally... for completeness here's a worked example that solves the DOM case of setting the ownerDocument property.

<?php
class extDOMDocument extends DOMDocument {
 public function createElement($name, $value=null) {
  $orphan = new extDOMElement($name, $value); // new  sub-class object
  $docFragment = $this->createDocumentFragment(); // lightweight container maintains "ownerDocument"
  $docFragment->appendChild($orphan); // attach
  
  return $docFragment;
 }
 // .. more class definition
}

class extDOMElement extends DOMElement {
 function __construct($name, $value='', $namespaceURI=null) {
  parent::__construct($name, $value, $namespaceURI);
 }
  //  ... more class definition here
}

$doc = new extDOMDocument('test');
$el = $doc->createElement('tagname');

// append discards the DOMDocumentFragment and just adds its child nodes, but ownerDocument is maintained.
$doc->appendChild($el); 
echo $doc->saveXML();
?>

TJ.
 [2005-11-05 02:56 UTC] php at tjworld dot net
Based on real-world experience the previous example is not sufficent - the new Element is destroyed along with the DOMDocumentFragment when the method exits.

The fix is to remove the new element from the DOMDocumentFragment before returning it to the caller.

Here's an updated, tested example.

<?php
class extDOMDocument extends DOMDocument {
 public function createElement($name, $value=null) {
  $orphan = new extDOMElement($name, $value); // new  sub-class object
  $docFragment = $this->createDocumentFragment(); // lightweight
container maintains "ownerDocument"
  $docFragment->appendChild($orphan); // attach
  $ret = $docFragment->removeChild($orphan); // remove
  return $ret; // ownerDocument set; won't be destroyed on  method exit
 }
 // .. more class definition
}

class extDOMElement extends DOMElement {
 function __construct($name, $value='', $namespaceURI=null) {
  parent::__construct($name, $value, $namespaceURI);
 }
  //  ... more class definition here
}

$doc = new extDOMDocument('test');
$el = $doc->createElement('tagname');

// append discards the DOMDocumentFragment and just adds its child
nodes, but ownerDocument is maintained.
$doc->appendChild($el); 
echo $doc->saveXML();
?>
 [2005-11-05 03:03 UTC] php at tjworld dot net
... and I should have added that returning a DOMDocumentFragment is not practical because adding attributes and nodes to the new element would then require new indirect code in the application - you can't add them to the returned DOMDocumentFragment.

So returning the new sub-class object requires it be removed from the DOMDocumentFragment *and* more importantly have a variable-reference to it so that its reference counter isn't zero when the method exits - otherwise it is destroyed and the caller can't do anything with it, and will get a:

Warning: DOMElement::setAttribute(): Couldn't fetch extDOMElement in...

trying to set an attribute on the new sub-classed object.
 [2005-12-01 13:19 UTC] dmitry@php.net
This is not a bug, but ext/dom limitation.

Nothing to fix.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Dec 06 17:01:27 2024 UTC