Skip to content

RenderCallbackRule: Check if method exists in trustedCallbacks #562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
8 changes: 8 additions & 0 deletions src/Drupal/DrupalAutoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,14 @@ protected function addCoreTestNamespaces(): void
$this->namespaces['Drupal\\TestSite'] = $core_tests_dir . '/TestSite';
$this->namespaces['Drupal\\TestTools'] = $core_tests_dir . '/TestTools';
$this->namespaces['Drupal\\Tests\\TestSuites'] = $this->drupalRoot . '/core/tests/TestSuites';

$classMap = [
'\\Drupal\\Tests\\Core\\Render\\BubblingTest' => $this->drupalRoot . '/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php',
'\\Drupal\\Tests\\Core\\Render\\PlaceholdersTest' => $this->drupalRoot . '/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php',
'\\Drupal\\Tests\\Core\\Render\\TestAccessClass' => $this->drupalRoot . '/core/tests/Drupal/Tests/Core/Render/RendererTest.php',
'\\Drupal\\Tests\\Core\\Render\\TestCallables' => $this->drupalRoot . '/core/tests/Drupal/Tests/Core/Render/RendererTest.php',
];
$this->autoloader->addClassMap($classMap);
}
Comment on lines +257 to 264
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't fix the internal error:

Error: Internal error: Internal error: Method of class Drupal\Tests\Core\Render\BubblingTest cannot be used as the class does not exist in file /home/runner/work/_temp/drupal/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php

I don't know what to do.


protected function addModuleNamespaces(): void
Expand Down
55 changes: 54 additions & 1 deletion src/Rules/Drupal/RenderCallbackRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,28 @@ private function doProcessNode(Node\Expr $node, Scope $scope, string $keyChecked
)->line($errorLine)->build();
} elseif (!$trustedCallbackType->isSuperTypeOf(new ObjectType($matches[1]))->yes()) {
$errors[] = RuleErrorBuilder::message(
sprintf("%s callback class %s at key '%s' does not implement Drupal\Core\Security\TrustedCallbackInterface.", $keyChecked, $constantStringType->describe(VerbosityLevel::value()), $pos)
sprintf(
"%s callback class %s at key '%s' does not implement Drupal\Core\Security\TrustedCallbackInterface.",
$keyChecked,
$constantStringType->describe(VerbosityLevel::value()),
$pos
)
)->line($errorLine)->tip('Change record: https://www.drupal.org/node/2966725.')->build();
} else {
$object = (new ObjectType($matches[1]));
if ($object->hasMethod('trustedCallbacks')->yes()) {
$allowedMethods = $this->getTrustedCallbackValues($object);
if (!in_array($matches[2], $allowedMethods, true)) {
$errors[] = RuleErrorBuilder::message(
sprintf(
"%s callback method '%s' is not present in 'trustedCallbacks' at key '%s'.",
$keyChecked,
$matches[2],
$pos
)
)->line($errorLine)->tip('Change record: https://www.drupal.org/node/2966725.')->build();
}
}
}
}
}
Expand Down Expand Up @@ -237,6 +257,21 @@ function (ClassReflection $reflection) use ($typeAndMethodName) {
)
)->line($errorLine)->tip('Change record: https://www.drupal.org/node/2966725.')->build();
}
} elseif ($isTrustedCallbackInterfaceType) {
$object = $typeAndMethodName->getType();
if ($object->hasMethod('trustedCallbacks')->yes()) {
$allowedMethods = $this->getTrustedCallbackValues($typeAndMethodName->getType());
if (!in_array($typeAndMethodName->getMethod(), $allowedMethods, true)) {
$errors[] = RuleErrorBuilder::message(
sprintf(
"%s callback method '%s' is not present in 'trustedCallbacks' at key '%s'.",
$keyChecked,
$typeAndMethodName->getMethod(),
$pos
)
)->line($errorLine)->tip('Change record: https://www.drupal.org/node/2966725.')->build();
}
}
}
}
}
Expand Down Expand Up @@ -314,4 +349,22 @@ private function getType(Node\Expr $node, Scope $scope): Type
}
return $type;
}

/**
* @return string[]
*/
private function getTrustedCallbackValues(Type $type): array
{
$values = [];
foreach ($type->getObjectClassReflections() as $classReflection) {
if (!$classReflection->hasMethod('trustedCallbacks')) {
continue;
}
$values[] = $classReflection
->getNativeReflection()
->getMethod('trustedCallbacks')
->invoke(null);
}
return array_merge(...$values);
}
}
13 changes: 12 additions & 1 deletion tests/src/Rules/RenderCallbackRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,18 @@ public static function fileData(): \Generator
}
yield [
__DIR__ . '/data/bug-543.php',
[]
[
[
"#access_callback callback method 'accessResultForbiddenNotInTrustedCallbacks' is not present in 'trustedCallbacks' at key '0'.",
58,
"Change record: https://www.drupal.org/node/2966725."
],
[
"#access_callback callback method 'accessResultForbiddenNotInTrustedCallbacks' is not present in 'trustedCallbacks' at key '0'.",
103,
"Change record: https://www.drupal.org/node/2966725."
],
],
];
yield [
__DIR__ . '/data/bug-554.php',
Expand Down
36 changes: 36 additions & 0 deletions tests/src/Rules/data/bug-543.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ public function providerAccessValues() {
[TRUE],
[AccessResult::forbidden()],
[AccessResult::allowed()],
['accessResultForbiddenNotInTrustedCallbacks'],
];
}

/**
* Tests callbacks with the method names in a variable.
*
* @dataProvider providerAccessValues
*/
public function testRenderWithAccessControllerResolved($access) {
Expand All @@ -45,38 +48,67 @@ public function testRenderWithAccessControllerResolved($access) {
case TRUE:
$method = 'accessTrue';
break;

case 'accessResultForbiddenNotInTrustedCallbacks':
$method = 'accessResultForbiddenNotInTrustedCallbacks';
break;
}

$build = [
'#access_callback' => TestAccessClass::class . '::' . $method,
];
}

/**
* Tests callback with the actual method name.
*/
public function bug543AccessResultAllowed(): void {
$build = [
'#access_callback' => TestAccessClass::class . '::accessResultAllowed',
];
}

/**
* Tests callback with the actual method name.
*/
public function bug543AccessResultForbidden(): void {
$build = [
'#access_callback' => TestAccessClass::class . '::accessResultForbidden',
];
}

/**
* Tests callback with the actual method name.
*/
public function bug543AccessFalse(): void {
$build = [
'#access_callback' => TestAccessClass::class . '::accessFalse',
];
}

/**
* Tests callback with the actual method name.
*/
public function bug543AccessTrue(): void {
$build = [
'#access_callback' => TestAccessClass::class . '::accessTrue',
];
}

/**
* Tests callback with the actual method name.
*/
public function bug543AccessResultForbiddenNotInTrustedCallbacks(): void {
$build = [
'#access_callback' => TestAccessClass::class . '::accessResultForbiddenNotInTrustedCallbacks',
];
}

}

/**
* Test class with callbacks.
*/
class TestAccessClass implements TrustedCallbackInterface {

public static function accessTrue() {
Expand All @@ -95,6 +127,10 @@ public static function accessResultForbidden() {
return AccessResult::forbidden();
}

public static function accessResultForbiddenNotInTrustedCallbacks() {
return AccessResult::forbidden();
}

/**
* {@inheritdoc}
*/
Expand Down