Issue #3048344 by Pancho: Fatal error when specified FillPdfForm doesn't exist

src/Controller/HandlePdfController.php

@@ -112,6 +112,10 @@ class HandlePdfController extends ControllerBase {
    *
    * @return \Symfony\Component\HttpFoundation\Response
    *   The action plugin's response object.
+   *
+   * @throws \InvalidArgumentException
+   *   If one of the passed arguments is missing or does not pass the
+   *   validation.
    */
   public function populatePdf() {
     $context = $this->linkManipulator->parseRequest($this->requestStack->getCurrentRequest());
@@ -125,13 +129,7 @@ class HandlePdfController extends ControllerBase {
 
     // @todo: Emit event (or call alter hook?) before populating PDF.
     // Rename fillpdf_merge_fields_alter() to fillpdf_populate_fields_alter().
-    /** @var \Drupal\fillpdf\FillPdfFormInterface $fillpdf_form */
     $fillpdf_form = FillPdfForm::load($context['fid']);
-    if (!$fillpdf_form) {
-      $this->messenger->addError($this->t('FillPDF Form (fid) not found in the system. Please check the value in your FillPDF Link.'));
-      return new RedirectResponse(Url::fromRoute('<front>')->toString());
-    }
-
     $fields = $fillpdf_form->getFormFields();
 
     // Populate entities array based on what user passed in.

src/FillPdfAccessController.php

@@ -2,13 +2,19 @@
 
 namespace Drupal\fillpdf;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
+use Drupal\Core\Logger\LoggerChannelTrait;
+use Drupal\Core\Messenger\MessengerTrait;
 use Drupal\Core\Session\AccountInterface;
 
 class FillPdfAccessController implements ContainerInjectionInterface {
 
+  use MessengerTrait;
+  use LoggerChannelTrait;
+
   /** @var \Drupal\fillpdf\FillPdfAccessHelperInterface */
   protected $accessHelper;
 
@@ -40,7 +46,17 @@ class FillPdfAccessController implements ContainerInjectionInterface {
   }
 
   public function checkLink() {
-    $context = $this->linkManipulator->parseRequest($this->requestStack->getCurrentRequest());
+    try {
+      $context = $this->linkManipulator->parseRequest($this->requestStack->getCurrentRequest());
+    }
+    catch (\InvalidArgumentException $exception) {
+      $message = $exception->getMessage();
+      $is_admin = $this->currentUser->hasPermission('administer pdfs');
+      $this->messenger()->addError($is_admin ? $message : t('An error occurred. Please notify the administrator.'));
+      $this->getLogger('fillpdf')->error($message);
+      return AccessResult::forbidden();
+    }
+
     $account = $this->currentUser;
 
     return $this->accessHelper->canGeneratePdfFromContext($context, $account);

src/Service/FillPdfLinkManipulator.php

@@ -54,8 +54,7 @@ class FillPdfLinkManipulator implements FillPdfLinkManipulatorInterface {
     $query = $link->getOption('query');
 
     if (!$query) {
-      throw new \InvalidArgumentException('The \Drupal\Core\Url you pass in must
-      have its \'query\' option set.');
+      throw new \InvalidArgumentException("This link doesn't specify a query string, so failing.");
     }
 
     $request_context = [
@@ -78,8 +77,7 @@ class FillPdfLinkManipulator implements FillPdfLinkManipulatorInterface {
       $request_context['fid'] = $query['fid'];
     }
     else {
-      throw new \InvalidArgumentException('fid parameter missing from query
-      string; cannot determine how to proceed, so failing.');
+      throw new \InvalidArgumentException('No FillPdfForm was specified in the query string, so failing.');
     }
 
     if (!empty($query['entity_type'])) {
@@ -116,6 +114,11 @@ class FillPdfLinkManipulator implements FillPdfLinkManipulatorInterface {
     else {
       // Populate defaults.
       $fillpdf_form = FillPdfForm::load($request_context['fid']);
+
+      if (!$fillpdf_form) {
+        throw new \InvalidArgumentException("The requested FillPdfForm doesn't exist, so failing.");
+      }
+
       $default_entity_id = $fillpdf_form->default_entity_id->value;
       if ($default_entity_id) {
         $default_entity_type = $fillpdf_form->default_entity_type->value;

tests/src/Functional/LinkManipulatorTest.php

+<?php
+
+namespace Drupal\Tests\fillpdf\Functional;
+
+use Drupal\Core\Url;
+
+/**
+ * @coversDefaultClass \Drupal\fillpdf\Service\FillPdfLinkManipulator
+ *
+ * @group fillpdf
+ *
+ * @todo Convert into a unit test.
+ */
+class LinkManipulatorTest extends FillPdfUploadTestBase {
+
+  /**
+   * Tests handling of a non-existing FillPdfForm ID.
+   */
+  public function testLinkExceptions() {
+    // Hit the generation route with no query string set.
+    $fillpdf_route = Url::fromRoute('fillpdf.populate_pdf', [], []);
+    $this->drupalGet($fillpdf_route);
+    // Ensure the exception is converted to an error and access is denied.
+    $this->assertSession()->statusCodeEquals(403);
+    $this->assertSession()->pageTextContains("This link doesn't specify a query string, so failing.");
+
+    // Hit the generation route with no fid set.
+    $fillpdf_route = Url::fromRoute('fillpdf.populate_pdf', [], [
+      'query' => [
+        'sample' => 1,
+      ],
+    ]);
+    $this->drupalGet($fillpdf_route);
+    // Ensure the exception is converted to an error and access is denied.
+    $this->assertSession()->statusCodeEquals(403);
+    $this->assertSession()->pageTextContains("No FillPdfForm was specified in the query string, so failing.");
+
+    // Hit the generation route with a non-existing fid set.
+    $fillpdf_route = Url::fromRoute('fillpdf.populate_pdf', [], [
+      'query' => [
+        'fid' => 1234,
+      ],
+    ]);
+    $this->drupalGet($fillpdf_route);
+    // Ensure the exception is converted to an error and access is denied.
+    $this->assertSession()->statusCodeEquals(403);
+    $this->assertSession()->pageTextContains("The requested FillPdfForm doesn't exist, so failing.");
+  }
+
+}