#PAreview: security

โšก๏ธ Live updates comments, jobs, and issues, tagged with #PAreview: security will update issues and activities on this page.

Issues

The last 100 updated issues.

Activities

The last 7 days of comments and CI jobs.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    webt.module

    /**
     * Perform scheduled translations for entity to be loaded.
     *
     * @param \Drupal\Core\Entity\Entity $e
     *   Entity.
     */
    function webt_entity_load($e) {
      $entity = \Drupal::state()->get("webt_update_entity");
      if ($entity) {
        \Drupal::state()->delete("webt_update_entity");
        $translation_manager = \Drupal::service('webt.translation_manager');
        $translation_manager->entityUpdateAllTranslations($entity);
      }
    }
    

    Hook implementations use a different documentation comment.
    Entity hooks like this one needs to use the entity passed as parameter, not an entity stored somewhere. If an entity must the flagged for translation, a dynamic property needs to be set for that entity. (The ContentEntityBase class implements the __get() and __set() magic methods.

      if (method_exists($entity, 'isTranslatable') && $entity->isTranslatable() && array_key_exists('default_langcode', $entity->toArray()) && $entity->default_langcode &&
          $entity->default_langcode[0]->value && \Drupal::service('webt.translation_manager')->entityTranslatableContentChanged($entity)) {
        \Drupal::state()->set("webt_update_entity", $entity);
      }
    

    Instead of verifying a method exists, the code needs to verify the object implements Drupal\Core\TypedData\TranslatableInterface.

    src/Controller/AboutTabController.php

            'configure_content' => [
              '#type' => 'html_tag',
              '#tag' => 'p',
              '#value' => Markup::create($this->t('See how to <a target="_blank" href=":link">configure and use this plugin</a>.', [
                ':link' => 'https://website-translation.language-tools.ec.europa.eu/solutions/drupal_en',
              ])),
    

    The Drupal\Core\Render\Markup class is internal and must not be used by contributed modules. In this case, it is not even necessary to use that class, since $this->t() already returns an object that implements \Drupal\Component\Render\MarkupInterface.

      /**
       * The request stack service.
       *
       * @var \Symfony\Component\HttpFoundation\RequestStack
       */
      protected $requestStack;
    

    There is not need to add request_stack as dependency and use that property, since:

    Additionally, if a parameter is typed to one of the following special classes the system will pass those values as well.

    • \Symfony\Component\HttpFoundation\Request: The raw Symfony request object. It is generally only useful if the controller needs access to the query parameters of the request. By convention, this parameter is usually named $request.
    • \Psr\Http\Message\ServerRequestInterface: The raw request, represented using the PSR-7 ServerRequest format. This object is derived as necessary from the Symfony request, so if either will suffice the Symfony request will be slightly more performant. By convention this parameter is usually named $request.
    • \Drupal\Core\Routing\RouteMatchInterface: The "route match" data from this request. This object contains various standard data derived from the request and routing process. Consult the interface for details.

    (Routing API / Route controllers for simple routes)

      /**
       * The cache service.
       *
       * @var \Drupal\Core\Cache\CacheBackendInterface
       */
      protected $cache;
    

    There is no need to use that property, since the parent class has cache().

      /**
       * The language manager service.
       *
       * @var \Drupal\Core\Language\LanguageManagerInterface
       */
      protected $languageManager;
    

    There is no need to use that property, since the parent class has languageManager().

      /**
       * The logger service.
       *
       * @var \Psr\Log\LoggerInterface
       */
      protected $logger;
    
      /**
       * Entity type manager service.
       *
       * @var \Drupal\Core\Entity\EntityTypeManagerInterface
       */
      protected $entityTypeManager;
    

    Similarly, there is no need to define those properties, for the same reason. Check which methods must be called to initialize the parent properties.

    src/Controller/PretranslationProgressController.php

      use LoggerChannelTrait;
    

    That trait is already used from the parent class, together other traits.

        use AutowireTrait;
        use LoggerChannelTrait;
        use MessengerTrait;
        use RedirectDestinationTrait;
        use StringTranslationTrait;
    

    None of those traits needs to be used from ControllerBase child classes.

      /**
       * Messenger.
       *
       * @var Drupal\Core\Messenger\MessengerInterface
       */
      protected $messenger;
    

    The parent class has methods to use the messenger. Check also which methods must be called to initialize the parent properties.

        $logs = Link::fromTextAndUrl('logs', Url::fromRoute('dblog.overview'));
        $this->messenger->addError($this->t("Backend translation process was aborted! Check @logs for errors or try increasing website's PHP max_execution_time.", ['@logs' => $logs->toString()]));
        return new Response();
    

    The correct code to add a link in a translatable string is similar to the following code, which uses the url_generator service (which exists also in Drupal 11).

    $message = $this->t('Operating in maintenance mode. <a href=":url">Go online.</a>', [':url' => $this->urlGenerator->generate('system.site_maintenance_mode')]);
    
    $this->messenger->addMessage($message, 'status', FALSE);
    

    src/Form/PretranslationForm.php

    The class defines properties that are not necessary because the parent class already define the necessary methods; it also include methods that child classes need to call to initialize the parent properties.
    For inherited methods, the documentation comments are different from the documentation comments this class uses.

      public function submitForm(array &$form, FormStateInterface $form_state) {
        $this->messenger->deleteAll();
        $config = $this->configFactory->getEditable('webt.settings');
        $types  = StringType::getAllTypes();
        foreach ($types as $type) {
          $selected = $form_state->getValue('advanced')[$type];
          $config->set('translate_' . $type, $selected);
        }
        $config->save();
        $this->messenger->addStatus($this->t('Affected content type selection saved'));
      }
    

    Submission form handlers do not need to remove messages from the messenger.

Production build 0.69.0 2024