Skip to content

Commit

Permalink
FIX Don't try to access private properties/methods
Browse files Browse the repository at this point in the history
  • Loading branch information
GuySartorelli committed Jan 30, 2023
1 parent 0f40146 commit b199638
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 19 deletions.
8 changes: 7 additions & 1 deletion src/Core/CustomMethods.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,13 @@ protected function registerExtraMethodCallback($name, $callback)
*/
public function hasMethod($method)
{
return method_exists($this, $method ?? '') || $this->getExtraMethodConfig($method);
return (method_exists($this, $method ?? '') && !$this->isPrivateMethod($method)) || $this->getExtraMethodConfig($method);
}

private function isPrivateMethod(string $method): bool
{
$reflectionMethod = new ReflectionMethod($this, $method);
return $reflectionMethod->isPrivate();
}

/**
Expand Down
21 changes: 11 additions & 10 deletions src/View/ViewableData.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
use InvalidArgumentException;
use IteratorAggregate;
use LogicException;
use ReflectionMethod;
use ReflectionObject;
use ReflectionProperty;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Config\Configurable;
Expand Down Expand Up @@ -111,7 +113,7 @@ public function __construct()
public function __isset($property)
{
// getField() isn't a field-specific getter and shouldn't be treated as such
if (strtolower($property ?? '') !== 'field' && $this->hasMethod($method = "get$property")) {
if (strtolower($property ?? '') !== 'field' && $this->hasMethod("get$property")) {
return true;
}
if ($this->hasField($property)) {
Expand Down Expand Up @@ -159,20 +161,13 @@ public function __set($property, $value)
$this->objCacheClear();
$method = "set$property";

if ($this->hasMethod($method) && !$this->isPrivate($this, $method)) {
if ($this->hasMethod($method)) {
$this->$method($value);
} else {
$this->setField($property, $value);
}
}

private function isPrivate(object $class, string $method): bool
{
$class = new ReflectionObject($class);

return $class->getMethod($method)->isPrivate();
}

/**
* Set a failover object to attempt to get data from if it is not present on this object.
*
Expand Down Expand Up @@ -207,7 +202,13 @@ public function getFailover()
*/
public function hasField($field)
{
return property_exists($this, $field) || isset($this->data[$field]);
return (property_exists($this, $field) && !$this->isPrivateProperty($field)) || isset($this->data[$field]);
}

private function isPrivateProperty(string $property): bool
{
$reflectionProperty = new ReflectionProperty($this, $property);
return $reflectionProperty->isPrivate();
}

/**
Expand Down
35 changes: 27 additions & 8 deletions tests/php/View/ViewableDataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -208,16 +208,35 @@ public function testSetFailover()
$this->assertFalse($container->hasMethod('testMethod'), 'testMethod() incorrectly reported as existing');
}

public function testIsPrivate()
public function testIsPrivateMethod()
{
$reflectionMethod = new ReflectionMethod(ViewableData::class, 'isPrivate');
$reflectionMethod = new ReflectionMethod(ViewableData::class, 'isPrivateMethod');
$reflectionMethod->setAccessible(true);
$object = new ViewableDataTestObject();

$output = $reflectionMethod->invokeArgs($object, [$object, 'privateMethod']);
$this->assertTrue($output, 'Method is not private');

$output = $reflectionMethod->invokeArgs($object, [$object, 'publicMethod']);
$this->assertFalse($output, 'Method is private');

$output = $reflectionMethod->invokeArgs($object, ['privateMethod']);
$this->assertTrue($output, 'Method should be private');

$output = $reflectionMethod->invokeArgs($object, ['protectedMethod']);
$this->assertFalse($output, 'Method should not be private');

$output = $reflectionMethod->invokeArgs($object, ['publicMethod']);
$this->assertFalse($output, 'Method should not be private');
}

public function testIsPrivateProperty()
{
$reflectionMethod = new ReflectionMethod(ViewableData::class, 'isPrivateProperty');
$reflectionMethod->setAccessible(true);
$object = new ViewableDataTestObject();

$output = $reflectionMethod->invokeArgs($object, ['privateProperty']);
$this->assertTrue($output, 'Property should be private');

$output = $reflectionMethod->invokeArgs($object, ['protectedProperty']);
$this->assertFalse($output, 'Property should not be private');

$output = $reflectionMethod->invokeArgs($object, ['publicProperty']);
$this->assertFalse($output, 'Property should not be private');
}
}
11 changes: 11 additions & 0 deletions tests/php/View/ViewableDataTestObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,22 @@

class ViewableDataTestObject extends DataObject implements TestOnly
{
private string $privateProperty = 'private property';

protected string $protectedProperty = 'protected property';

public string $publicProperty = 'public property';

private function privateMethod(): string
{
return 'Private function';
}

protected function protectedMethod(): string
{
return 'Protected function';
}

public function publicMethod(): string
{
return 'Public function';
Expand Down

0 comments on commit b199638

Please sign in to comment.