When trying to edit an effect with an invalid (inexistent or deleted) effect ID in URL, Drupal returns 500. It should return 401.

Files: 
CommentFileSizeAuthor
#17 no-effect-2063403-17.patch7.21 KBclaudiu.cristea
PASSED: [[SimpleTest]]: [MySQL] 59,094 pass(es).
[ View ]
#15 no-effect-2063403-15.patch5.97 KBclaudiu.cristea
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
#15 interdiff.txt810 bytesclaudiu.cristea
#11 no-effect-2063403-11.patch7.32 KBclaudiu.cristea
PASSED: [[SimpleTest]]: [MySQL] 59,562 pass(es).
[ View ]
#11 interdiff.txt2.5 KBclaudiu.cristea
#8 no-effect-2063403-8.patch7.35 KBclaudiu.cristea
PASSED: [[SimpleTest]]: [MySQL] 58,137 pass(es).
[ View ]
#8 interdiff.txt6.7 KBclaudiu.cristea
#3 no-effect-2063403-3.patch5.76 KBclaudiu.cristea
FAILED: [[SimpleTest]]: [MySQL] 57,986 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#1 2063403-1.patch1.98 KBclaudiu.cristea
FAILED: [[SimpleTest]]: [MySQL] 58,128 pass(es), 1 fail(s), and 1 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.98 KB
FAILED: [[SimpleTest]]: [MySQL] 58,128 pass(es), 1 fail(s), and 1 exception(s).
[ View ]

Here's a patch showing this bug.

Status:Needs review» Needs work

The last submitted patch, 2063403-1.patch, failed testing.

StatusFileSize
new5.76 KB
FAILED: [[SimpleTest]]: [MySQL] 57,986 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Here's the fix.

Status:Needs work» Needs review

Title:Crash when edit inexistent effectFatal when editing a nonexistent image effect

@@ -65,7 +67,13 @@ public function has($instance_id) {
-      $this->initializePlugin($instance_id);
+      try {
+        $this->initializePlugin($instance_id);
+      }

This should not catch the exception, its up to an individual plugin bag.

@@ -65,7 +67,13 @@ public function has($instance_id) {
+      catch (PluginException $e) {
+        $null = NULL;
+        return $null;

This looks wrong

@@ -344,6 +346,11 @@ function testEditEffect() {
+    // Try to edit an inexisting effect.

nonexistent

Status:Needs review» Needs work

The last submitted patch, no-effect-2063403-3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.7 KB
new7.35 KB
PASSED: [[SimpleTest]]: [MySQL] 58,137 pass(es).
[ View ]

Just finished patch when you posted :)

This needs reviews or RTBC

  1. +++ b/core/lib/Drupal/Component/Plugin/Exception/UnknownPluginException.php
    @@ -0,0 +1,31 @@
    + * Definition of Drupal\Component\Plugin\Exception\UnknownPluginException.

    Should be @file\nContains ...

  2. +++ b/core/lib/Drupal/Component/Plugin/Exception/UnknownPluginException.php
    @@ -0,0 +1,31 @@
    +  public function __construct($instance_id, $message = '', $code = 0, \Exception $previous = NULL) {

    It is a bit odd that the message is not the first parameter of the exception.

  3. +++ b/core/modules/image/lib/Drupal/image/Form/ImageEffectFormBase.php
    @@ -57,7 +58,12 @@ public function getFormID() {
    +    catch (UnknownPluginException $e) {
    +      throw new NotFoundHttpException();
    +    }

    It would be great to display that the image effect could not be created/does not exist.

StatusFileSize
new2.5 KB
new7.32 KB
PASSED: [[SimpleTest]]: [MySQL] 59,562 pass(es).
[ View ]

@dawehner,

It is a bit odd that the message is not the first parameter of the exception.

I know but I need a way to pass the plugin id without a message by default. It's not against any rule.

Fixed others, see interdiff.

Assigned:Unassigned» claudiu.cristea

Yay, passed.

Status:Needs review» Reviewed & tested by the community

As #10 is addressed and #1 contains test only patch. So I think it is ready for RTBC.

Status:Reviewed & tested by the community» Needs work

Needs work since #1969572: Make Uuid a service has landed

Status:Needs work» Needs review
StatusFileSize
new810 bytes
new5.97 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

Here with uuid as service.

Status:Needs review» Needs work

The last submitted patch, no-effect-2063403-15.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.21 KB
PASSED: [[SimpleTest]]: [MySQL] 59,094 pass(es).
[ View ]

Oh, missed Drupal\Component\Plugin\Exception\PluginException.

Status:Needs review» Reviewed & tested by the community

Thanks @claudiu.cristea for fixing this. Green with Uuid service so back to RTBC.

#17: no-effect-2063403-17.patch queued for re-testing.

Status:Reviewed & tested by the community» Fixed

Committed a6d19bf and pushed to 8.x. Thanks!

Removed unnecessary use statements during commit.

diff --git a/core/modules/image/lib/Drupal/image/Form/ImageEffectFormBase.php b/core/modules/image/lib/Drupal/image/Form/ImageEffectFormBase.php
index fc0f547..0df59dd 100644
--- a/core/modules/image/lib/Drupal/image/Form/ImageEffectFormBase.php
+++ b/core/modules/image/lib/Drupal/image/Form/ImageEffectFormBase.php
@@ -12,8 +12,6 @@
use Drupal\image\ImageStyleInterface;
use Drupal\Component\Plugin\Exception\UnknownPluginException;
use Drupal\Component\Utility\String;
-use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
/**

Status:Fixed» Closed (fixed)

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