From 20978076def6908dc85f07ac751e54aa7034ce53 Mon Sep 17 00:00:00 2001
From: Bernd Oliver Suenderhauf <bos@suenderhauf.de>
Date: Tue, 16 Apr 2019 02:06:29 +0200
Subject: [PATCH] Issue #3048344 by Pancho: Fatal error when specified
FillPdfForm doesn't exist
---
src/Controller/HandlePdfController.php | 10 ++--
src/FillPdfAccessController.php | 18 ++++++-
src/Service/FillPdfLinkManipulator.php | 11 +++--
tests/src/Functional/LinkManipulatorTest.php | 50 ++++++++++++++++++++
4 files changed, 78 insertions(+), 11 deletions(-)
create mode 100644 tests/src/Functional/LinkManipulatorTest.php
diff --git a/src/Controller/HandlePdfController.php b/src/Controller/HandlePdfController.php
index de76bae..dd32bd1 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 e198584..effa746 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 d898097..970b951 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 0000000..9bb148d
--- /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.");
+ }
+
+}
--
GitLab