Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Commit

Permalink
Do not validate callables
Browse files Browse the repository at this point in the history
- If a callback is_callable, then we should allow it.
- Developers will get an E_STRICT when invoking non-static methods statically,
  and an E_FATAL when using $this in such situations; this is up to the
  developer to debug and handle, not the framework.
  • Loading branch information
weierophinney committed Jan 12, 2012
1 parent 32a8259 commit 132d5b6
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 106 deletions.
103 changes: 1 addition & 102 deletions src/CallbackHandler.php
Expand Up @@ -46,12 +46,6 @@ class CallbackHandler
*/
protected $callback;

/**
* Until callback has been validated, mark as invalid
* @var bool
*/
protected $isValidCallback = false;

/**
* Callback metadata, if any
* @var array
Expand Down Expand Up @@ -80,8 +74,7 @@ public function __construct($callback, array $metadata = array())
*
* If a callback is a functor, or an array callback composing an object
* instance, this method will pass the object to a WeakRef instance prior
* to registering the callback. See {@link isValid()} for more information
* on how this affects execution.
* to registering the callback.
*
* @param callback $callback
* @return void
Expand All @@ -94,7 +87,6 @@ protected function registerCallback($callback)

// If pecl/weakref is not installed, simply store the callback and return
if (!class_exists('WeakRef')) {
$this->validateCallback($callback);
$this->callback = $callback;
return;
}
Expand All @@ -119,7 +111,6 @@ protected function registerCallback($callback)
// If we have an array callback, and the first argument is not an
// object, register as-is
if (!is_object($target)) {
$this->validateCallback($callback);
$this->callback = $callback;
return;
}
Expand All @@ -134,7 +125,6 @@ protected function registerCallback($callback)
* Retrieve registered callback
*
* @return Callback
* @throws Exception\InvalidCallbackException If callback is invalid
*/
public function getCallback()
{
Expand Down Expand Up @@ -244,95 +234,4 @@ public function getMetadatum($name)
}
return null;
}

/**
* Validate a callback
*
* Attempts to ensure that the provided callback is actually callable.
*
* Interestingly, given a class "Foo" with a non-static method "bar",
* is_callable() still returns true for the following callbacks:
*
* - array('Foo', 'bar')
* - 'Foo::bar'
*
* even though passing these to call_user_func*() will raise an error.
*
* @param string|array|object $callback
* @return bool
* @throws Exception\InvalidCallbackException
*/
protected function validateCallback($callback)
{
if (is_array($callback)) {
return $this->validateArrayCallback($callback);
}
if (is_string($callback)) {
return $this->validateStringCallback($callback);
}
return true;
}

/**
* Validate an array callback
*
* If the first argument of the array is an object, simply returns true.
* Otherwise, passes the array arguments off to validateStaticMethod().
*
* @param array $callback
* @return true
* @throws Exception\InvalidCallbackException
*/
protected function validateArrayCallback($callback)
{
list($target, $method) = $callback;
if (is_object($target)) {
return true;
}

return $this->validateStaticMethod($target, $method);
}

/**
* Validate a string callback
*
* If the callback is a function name, simply returns true. If it refers
* to a static method, proxies to validateStaticMethod().
*
* @param string $callback
* @return true
* @throws Exception\InvalidCallbackException
*/
protected function validateStringCallback($callback)
{
if (!strstr($callback, '::')) {
return true;
}

list($target, $method) = explode('::', $callback, 2);
return $this->validateStaticMethod($target, $method);
}

/**
* Validates that a static callback actually exists
*
* @param string $target
* @param string $method
* @return bool
* @throws Exception\InvalidCallbackException
*/
protected function validateStaticMethod($target, $method)
{
$r = new ReflectionClass($target);
if (!$r->hasMethod($method)) {
throw new Exception\InvalidCallbackException('Invalid callback; method does not exist');
}

$m = $r->getMethod($method);
if (!$m->isStatic()) {
throw new Exception\InvalidCallbackException('Invalid callback; method is not static');
}

return true;
}
}
18 changes: 14 additions & 4 deletions test/CallbackHandlerTest.php
Expand Up @@ -96,18 +96,28 @@ public function testStringCallbackReferringToClassWithoutDefinedInvokeShouldRais
$handler = new CallbackHandler('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod');
}

public function testCallbackConsistingOfStringContextWithNonStaticMethodShouldRaiseException()
public function testCallbackConsistingOfStringContextWithNonStaticMethodShouldNotRaiseExceptionButWillRaiseEStrict()
{
$this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException');
$handler = new CallbackHandler(array('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod', 'handler'));
$error = false;
set_error_handler(function ($errno, $errstr) use (&$error) {
$error = true;
}, E_STRICT);
$handler->call();
restore_error_handler();
$this->assertTrue($error);
}

public function testStringCallbackConsistingOfNonStaticMethodShouldRaiseException()
public function testStringCallbackConsistingOfNonStaticMethodShouldNotRaiseExceptionButWillRaiseEStrict()
{
$this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException');
$handler = new CallbackHandler('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod::handler');
$error = false;
set_error_handler(function ($errno, $errstr) use (&$error) {
$error = true;
}, E_STRICT);
$handler->call();
restore_error_handler();
$this->assertTrue($error);
}

public function testCallbackToClassImplementingOverloadingButNotInvocableShouldRaiseException()
Expand Down

0 comments on commit 132d5b6

Please sign in to comment.