Split handling of old and new style subrequests to avoid empty or inceptioned pages.

core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php

@@ -36,11 +36,18 @@ class LegacyControllerSubscriber implements EventSubscriberInterface {
    *   The Event to process.
    */
   public function onKernelControllerLegacy(FilterControllerEvent $event) {
-    $router_item = $event->getRequest()->attributes->get('drupal_menu_item');
+    $request = $event->getRequest();
+    $router_item = $request->attributes->get('drupal_menu_item');
     $controller = $event->getController();
 
     // This BC logic applies only to functions. Otherwise, skip it.
     if (is_string($controller) && function_exists($controller)) {
+      // Flag this as a legacy request.  We need to use this for subrequest
+      // handling so that we can treat older page callbacks and new routes
+      // differently.
+      // @todo Remove this line as soon as possible.
+      $request->attributes->set('_legacy', TRUE);
+
       $new_controller = function() use ($router_item) {
         return call_user_func_array($router_item['page_callback'], $router_item['page_arguments']);
       };

core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php

@@ -45,14 +45,14 @@ public function __construct(ContentNegotiation $negotiation) {
    */
   public function onView(GetResponseForControllerResultEvent $event) {
 
+    $request = $event->getRequest();
+
     // For a master request, we process the result and wrap it as needed.
     // For a subrequest, all we want is the string value.  We assume that
     // is just an HTML string from a controller, so wrap that into a response
     // object.  The subrequest's response will get dissected and placed into
     // the larger page as needed.
     if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) {
-      $request = $event->getRequest();
-
       $method = 'on' . $this->negotiation->getContentType($request);
 
       if (method_exists($this, $method)) {
@@ -62,7 +62,9 @@ public function onView(GetResponseForControllerResultEvent $event) {
         $event->setResponse(new Response('Unsupported Media Type', 415));
       }
     }
-    else {
+    elseif ($request->attributes->get('_legacy')) {
+      // This is an old hook_menu-based subrequest, which means we assume
+      // the body is supposed to be the complete page.
       $page_result = $event->getControllerResult();
       if (!is_array($page_result)) {
         $page_result = array(
@@ -71,6 +73,18 @@ public function onView(GetResponseForControllerResultEvent $event) {
       }
       $event->setResponse(new Response(drupal_render_page($page_result)));
     }
+    else {
+      // This is a new-style Symfony-esque subrequest, which means we assume
+      // the body is not supposed to be a complete page but just a page
+      // fragment.
+      $page_result = $event->getControllerResult();
+      if (!is_array($page_result)) {
+        $page_result = array(
+          '#markup' => $page_result,
+        );
+      }
+      $event->setResponse(new Response(drupal_render($page_result)));
+    }
   }
 
   public function onJson(GetResponseForControllerResultEvent $event) {

core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php

@@ -43,7 +43,14 @@ public function testCanRoute() {
   public function testDefaultController() {
     $this->drupalGet('router_test/test2');
     $this->assertRaw('test2', 'The correct string was returned because the route was successful.');
+
+    // Confirm that the page wrapping is being added, so we're not getting a
+    // raw body returned.
     $this->assertRaw('</html>', 'Page markup was found.');
+
+    // In some instances, the subrequest handling may get confused and render
+    // a page inception style.  This test verifies that is not happening.
+    $this->assertNoPattern('#</body>.*</body>#s', 'There was no double-page effect from a misrendered subrequest.');
   }
 
   /**
@@ -53,7 +60,14 @@ public function testControllerPlaceholders() {
     $value = $this->randomName();
     $this->drupalGet('router_test/test3/' . $value);
     $this->assertRaw($value, 'The correct string was returned because the route was successful.');
+
+    // Confirm that the page wrapping is being added, so we're not getting a
+    // raw body returned.
     $this->assertRaw('</html>', 'Page markup was found.');
+
+    // In some instances, the subrequest handling may get confused and render
+    // a page inception style.  This test verifies that is not happening.
+    $this->assertNoPattern('#</body>.*</body>#s', 'There was no double-page effect from a misrendered subrequest.');
   }
 
   /**
@@ -62,7 +76,14 @@ public function testControllerPlaceholders() {
   public function testControllerPlaceholdersDefaultValues() {
     $this->drupalGet('router_test/test4');
     $this->assertRaw('narf', 'The correct string was returned because the route was successful.');
+
+    // Confirm that the page wrapping is being added, so we're not getting a
+    // raw body returned.
     $this->assertRaw('</html>', 'Page markup was found.');
+
+    // In some instances, the subrequest handling may get confused and render
+    // a page inception style.  This test verifies that is not happening.
+    $this->assertNoPattern('#</body>.*</body>#s', 'There was no double-page effect from a misrendered subrequest.');
   }
 
 }