NotFoundHttpException thrown in ParamConverterManager results in a WSOD [#1987344]

Found this thanks to #1985528: Convert filter_tips_long() to Controller, but it can occur in HEAD now.

I'll try to come up with a more tailored test, but this at least demonstrates the problem.

Steps to reproduce

Install with Standard profile
Login as user 1
Go to admin/structure/views/view/pagenotfound

Expected result

404 Page not found

Actual result

200 WSOD

Essentially, any route with upcasting that fails will not get a 404 as expected.
This is in the dblog:
Symfony\Component\HttpKernel\Exception\NotFoundHttpException: in Drupal\Core\ParamConverter\ParamConverterManager->enhance() (line 83 of /Users/tim/www/d8/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php).
But you don't see an error and it doesn't 404.

Comment	File	Size	Author
#3	menu-router-1987344-3.patch	1.96 KB	tim.plunkett
#1	menu-router-1987344-1.patch	1.78 KB	tim.plunkett
	paramconverter.patch	681 bytes	tim.plunkett

Comments

Comment #1

tim.plunkett

he/him

English

Philadelphia

commented 5 May 2013 at 23:47

Issue tags:

+Stalking Crell

Status	File	Size
new	menu-router-1987344-1.patch	1.78 KB

Okay, found the cause. It was my recent fix for _menu_translate().

Comment #2

Crell commented 5 May 2013 at 23:53

+++ b/core/includes/menu.inc
@@ -771,7 +771,13 @@ function _menu_translate(&$router_item, $map, $to_arg = FALSE) {
-    $request->attributes->add(Drupal::service('router')->matchRequest($request));
+    // Attempt to match this path to provide a fully built request to the
+    // acccess checker.
+    try {
+      $request->attributes->add(Drupal::service('router')->matchRequest($request));
+    }
+    catch (NotFoundHttpException $e) {
+    }
     $router_item['access'] = Drupal::service('access_manager')->check($route, $request);

If the request doesn't match, do we want to continue silently? Or wouldn't that mean that there's nothing there, so we always want to disallow access to the link? I'm not sure, since that would then imply links to broken pages simply never show up at all... Hm.

Either way, IF we keep the empty catch statement then it needs to be documented as to why it's empty.