From 0ea8230787338b89abaeb6f01f2e8fe717da96dc Mon Sep 17 00:00:00 2001
From: Larry Garfield <larry@garfieldtech.com>
Date: Sat, 22 Sep 2012 14:16:28 -0500
Subject: [PATCH] Split handling of old and new style subrequests to avoid
empty or inceptioned pages.
---
.../LegacyControllerSubscriber.php | 9 +++++++-
.../Core/EventSubscriber/ViewSubscriber.php | 20 +++++++++++++++---
.../system/Tests/Routing/RouterTest.php | 21 +++++++++++++++++++
3 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php
index f088f7ef2e2e..7009eff9154e 100644
--- a/core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php
+++ b/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']);
};
diff --git a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php
index cb3aeeaaa6eb..e88525e7ed4f 100644
--- a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php
+++ b/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) {
diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php
index f5b4a1893f4d..413737516ac3 100644
--- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php
+++ b/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.');
}
}
--
GitLab