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

Command icon 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

altcom_neil created an issue. See original summary.

altcom_neil’s picture

jessey’s picture

aaronbauman made their first commit to this issue’s fork.

  • aaronbauman committed 1f9b53e6 on 6.0.x
    Issue #3533938 - Protect against routes not setting parameters in their...
aaronbauman’s picture

Version: 5.1.2 » 6.0.x-dev
Status: Active » Fixed

Fixed in 5.1.x and 6.0.x branches.
Thanks for the patches, and your patience.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • aaronbauman committed ed0579fd on 5.1.x
    Issue #3533938 - Protect against routes not setting parameters in their...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.