Problem/Motivation
In https://www.drupal.org/project/salesforce/issues/3269666 the code expected all routes to provide a RouteObject and 'parameters' in the options array.
We are using Webform Scheduled Task in both Drupal 10 and a forked version for Drupal 11 as they have not released a Drupal 11 compatible version of their module yet.
In both 10.4 and 11.2 it causes a php warning in PHP 8.3. It is possible it was also happening in earlier versions of Drupal/PHP and has not been noticed before.
Warning: foreach() argument must be of type array|object, null given in Drupal\salesforce_mapping_ui\Controller\MappedObjectController->access() (line 52 of modules/contrib/salesforce/modules/salesforce_mapping_ui/src/Controller/MappedObjectController.php).
Stepping through the code does show that the code is expecting things that may not always be true. There are two issues with the following line:
$parameter_info = $this->route->getRouteObject()->getOption('parameters');
1. getRouteObject() can return NULL.
From \Drupal\Core\Routing\RouteMatchInterface
/**
* Returns the route object.
*
* @return \Symfony\Component\Routing\Route|null
* The route object. NULL if no route is matched.
*/
public function getRouteObject();
2. It is assuming that the route always contains an iterable object called 'parameters' in the route options.
In the case of the Scheduled Tasks admin page for a form (e.g. /admin/structure/webform/manage/competition/scheduled-tasks) there is no 'parameters' option in the route object.
Steps to reproduce
Install Webform Scheduled Tasks and visit the scheduled tasks admin page for a Webform.
Proposed resolution
Make the code more defensive by checking a route object is returned and there are paratmers in the options:
$parameter_info = $this->route->getRouteObject()?->getOption('parameters');
if (!is_iterable($parameter_info)) {
return AccessResult::forbidden();
}
However, there is what I think is a much cleaner way of doing this.
Currently the code does this:
$param = null;
$parameter_info = $this->route->getRouteObject()->getOption('parameters');
foreach ($parameter_info as $name => $options) {
if (isset($options['type']) && str_starts_with($options['type'], 'entity:')) {
$param = $this->route->getParameter($name);
continue 1;
}
}
if (!is_object($param)) {
return AccessResult::forbidden();
}
$implements = class_implements($param);
if (empty($implements['Drupal\Core\Entity\EntityInterface'])) {
return AccessResult::forbidden();
}
But two of the conditions can be simply collapsed into one by getting the parameters directly from the route var and finding the entity (if one exists) directly from that array of objects. It also fixes the warning as while the old way returned NULL the same page getting the parameters this way returns an array.
$param = null;
$parameters = $this->route->getParameters();
foreach ($parameters as $name => $parameter) {
if ($parameter instanceof EntityInterface) {
$param = $parameter;
continue 1;
}
}
if (is_null($param)) {
return AccessResult::forbidden();
}
Issue fork salesforce-3533938
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
altcom_neil commentedComment #3
jessey commentedComment #7
aaronbaumanFixed in 5.1.x and 6.0.x branches.
Thanks for the patches, and your patience.