diff --git a/src/Controller/HandlePdfController.php b/src/Controller/HandlePdfController.php index de76bae17cd5f42a2044681b3c0720bdb3d91d4d..dd32bd1966d4598f8f89d8d48b1b1703d4b27cf5 100644 --- a/src/Controller/HandlePdfController.php +++ b/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. diff --git a/src/FillPdfAccessController.php b/src/FillPdfAccessController.php index e1985847c052f5d06be24a586355aa6fd88aa6f9..effa746a0d167b60ce1011d403be167cd099c226 100644 --- a/src/FillPdfAccessController.php +++ b/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); diff --git a/src/Service/FillPdfLinkManipulator.php b/src/Service/FillPdfLinkManipulator.php index d8980971159bb9052f70ae5a3c7b37805c3c0b69..970b9517c63f1c1e7eca30e4634565152cc143c3 100644 --- a/src/Service/FillPdfLinkManipulator.php +++ b/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; diff --git a/tests/src/Functional/LinkManipulatorTest.php b/tests/src/Functional/LinkManipulatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..9bb148da1cf8d60a183b48c437962aa110af33be --- /dev/null +++ b/tests/src/Functional/LinkManipulatorTest.php @@ -0,0 +1,50 @@ +<?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."); + } + +}