Issue #2363139 by Wim Leers, dawehner, larowlan: _content controllers may only...

Issue #2363139 by Wim Leers, dawehner, larowlan: _content controllers may only return render arrays, not strings.

core/lib/Drupal/Core/Controller/HtmlControllerBase.php

@@ -49,13 +49,16 @@ public function __construct(TitleResolverInterface $title_resolver, RenderHtmlRe
   /**
    * Converts a render array into an HtmlFragment object.
    *
-   * @param array|string $page_content
+   * @param array|\Drupal\Core\Page\HtmlFragmentInterface|\Symfony\Component\HttpFoundation\Response $page_content
    *   The page content area to display.
    * @param \Symfony\Component\HttpFoundation\Request $request
    *   The request object.
    *
    * @return \Drupal\Core\Page\HtmlPage
    *   A page object.
+   *
+   * @throws \InvalidArgumentException
+   *   Thrown if the controller returns a string.
    */
   protected function createHtmlFragment($page_content, Request $request) {
     // Allow controllers to return a HtmlFragment or a Response object directly.
@@ -63,8 +66,8 @@ protected function createHtmlFragment($page_content, Request $request) {
       return $page_content;
     }
 
-    if (!is_array($page_content)) {
-      $page_content = ['#markup' => $page_content];
+    if (is_string($page_content)) {
+      throw new \InvalidArgumentException('_content controllers are not allowed to return strings. You can return a render array, a html fragment or a response object.');
     }
 
     $fragment = $this->renderHtmlRenderer->render($page_content);

core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.module

@@ -194,7 +194,7 @@ function ajax_forms_test_advanced_commands_settings_with_merging_callback($form,
 function ajax_forms_test_validation_form_callback($form, FormStateInterface $form_state) {
   drupal_set_message("ajax_forms_test_validation_form_callback invoked");
   drupal_set_message(t("Callback: drivertext=%drivertext, spare_required_field=%spare_required_field", array('%drivertext' => $form_state->getValue('drivertext'), '%spare_required_field' => $form_state->getValue('spare_required_field'))));
-  return '<div id="message_area">ajax_forms_test_validation_form_callback at ' . date('c') . '</div>';
+  return ['#markup' => '<div id="message_area">ajax_forms_test_validation_form_callback at ' . date('c') . '</div>'];
 }
 
 /**
@@ -203,7 +203,7 @@ function ajax_forms_test_validation_form_callback($form, FormStateInterface $for
 function ajax_forms_test_validation_number_form_callback($form, FormStateInterface $form_state) {
   drupal_set_message("ajax_forms_test_validation_number_form_callback invoked");
   drupal_set_message(t("Callback: drivernumber=%drivernumber, spare_required_field=%spare_required_field", array('%drivernumber' => $form_state->getValue('drivernumber'), '%spare_required_field' => $form_state->getValue('spare_required_field'))));
-  return '<div id="message_area_number">ajax_forms_test_validation_number_form_callback at ' . date('c') . '</div>';
+  return ['#markup' => '<div id="message_area_number">ajax_forms_test_validation_number_form_callback at ' . date('c') . '</div>'];
 }
 
 /**

core/modules/system/tests/modules/entity_test/src/Controller/EntityTestController.php

@@ -88,7 +88,7 @@ public function testEdit(Request $request, $entity_type_id) {
    * @see \Drupal\entity_test\Routing\EntityTestRoutes::routes()
    */
   public function testAdmin() {
-    return '';
+    return [];
   }
 
   /**

core/modules/system/tests/modules/error_test/src/Controller/ErrorTestController.php

@@ -53,7 +53,7 @@ public function generateWarnings($collect_errors = FALSE) {
     $awesomely_big = 1/0;
     // This will generate a user error.
     trigger_error("Drupal is awesome", E_USER_WARNING);
-    return "";
+    return [];
   }
 
   /**

core/modules/system/tests/modules/menu_test/menu_test.module

@@ -91,7 +91,7 @@ function menu_test_menu_local_tasks_alter(&$data, $route_name) {
  * @deprecated Use \Drupal\menu_test\Controller\MenuTestController::menuTestCallback()
  */
 function menu_test_callback() {
-  return 'This is menu_test_callback().';
+  return ['#markup' => 'This is menu_test_callback().'];
 }
 
 /**
@@ -117,7 +117,7 @@ function menu_test_theme_page_callback($inherited = FALSE) {
   if ($inherited) {
     $output .= ' Theme negotiation inheritance is being tested.';
   }
-  return $output;
+  return ['#markup' => $output];
 }
 
 /**

core/modules/system/tests/modules/menu_test/src/TestControllers.php

@@ -19,28 +19,28 @@ class TestControllers {
    * Returns page to be used as a login path.
    */
   public function testLogin() {
-    return 'This is TestControllers::testLogin.';
+    return ['#markup' => 'This is TestControllers::testLogin.'];
   }
 
   /**
    * Prints out test data.
    */
   public function test1() {
-    return 'test1';
+    return ['#markup' => 'test1'];
   }
 
   /**
    * Prints out test data.
    */
   public function test2() {
-    return 'test2';
+    return ['#markup' => 'test2'];
   }
 
   /**
    * Prints out test data.
    */
   public function testDerived() {
-    return 'testDerived';
+    return ['#markup' => 'testDerived'];
   }
 
   /**
@@ -54,10 +54,10 @@ public function testDerived() {
    */
   public function testDefaults($placeholder = NULL) {
     if ($placeholder) {
-      return String::format("Sometimes there is a placeholder: '@placeholder'.", array('@placeholder' => $placeholder));
+      return ['#markup' => String::format("Sometimes there is a placeholder: '@placeholder'.", array('@placeholder' => $placeholder))];
     }
     else {
-      return String::format('Sometimes there is no placeholder.');
+      return ['#markup' => String::format('Sometimes there is no placeholder.')];
     }
   }
 

core/modules/system/tests/modules/module_test/src/Controller/ModuleTestController.php

@@ -30,7 +30,7 @@ public function hookDynamicLoadingInvokeAll() {
    * @todo Remove module_test_class_loading().
    */
   public function testClassLoading() {
-    return module_test_class_loading();
+    return ['#markup' => module_test_class_loading()];
   }
 
 }

core/modules/system/tests/modules/paramconverter_test/src/TestControllers.php

@@ -19,14 +19,14 @@ class TestControllers {
   public function testUserNodeFoo(EntityInterface $user, NodeInterface $node, Request $request) {
     $foo = $request->attributes->get('foo');
     $foo = is_object($foo) ? $foo->label() : $foo;
-    return "user: {$user->label()}, node: {$node->label()}, foo: $foo";
+    return ['#markup' => "user: {$user->label()}, node: {$node->label()}, foo: $foo"];
   }
 
   public function testNodeSetParent(NodeInterface $node, NodeInterface $parent) {
-    return "Setting '{$parent->label()}' as parent of '{$node->label()}'.";
+    return ['#markup' => "Setting '{$parent->label()}' as parent of '{$node->label()}'."];
   }
 
   public function testEntityLanguage(NodeInterface $node) {
-    return $node->label();
+    return ['#markup' => $node->label()];
   }
 }

core/modules/system/tests/modules/router_test_directory/src/TestContent.php

@@ -43,7 +43,7 @@ public static function create(ContainerInterface $container) {
    * Provides example content for testing route enhancers.
    */
   public function test1() {
-    return 'abcde';
+    return ['#markup' => 'abcde'];
   }
 
   /**
@@ -54,13 +54,13 @@ public function test1() {
    */
   public function test11() {
     $account = $this->currentUser();
-    return $account->getUsername();
+    return ['#markup' => $account->getUsername()];
   }
 
   public function testAccount(UserInterface $user) {
     $current_user_name = $this->currentUser()->getUsername();
     $this->currentUser()->setAccount($user);
-    return $current_user_name . ':' . $user->getUsername();
+    return ['#markup' => $current_user_name . ':' . $user->getUsername()];
   }
 
   /**

core/modules/system/tests/modules/router_test_directory/src/TestControllers.php

@@ -26,19 +26,19 @@ public function test1() {
   }
 
   public function test2() {
-    return "test2";
+    return ['#markup' => "test2"];
   }
 
   public function test3($value) {
-    return $value;
+    return ['#markup' => $value];
   }
 
   public function test4($value) {
-    return $value;
+    return ['#markup' => $value];
   }
 
   public function test5() {
-    return "test5";
+    return ['#markup' => "test5"];
   }
 
   public function test6() {

core/modules/system/tests/modules/session_test/src/Controller/SessionTestController.php

@@ -24,8 +24,8 @@ class SessionTestController extends ControllerBase {
    */
   public function get() {
     return empty($_SESSION['session_test_value'])
-      ? ""
-      : $this->t('The current value of the stored session variable is: %val', array('%val' => $_SESSION['session_test_value']));
+      ? []
+      : ['#markup' => $this->t('The current value of the stored session variable is: %val', array('%val' => $_SESSION['session_test_value']))];
   }
 
   /**
@@ -41,7 +41,7 @@ public function getId() {
 
     \Drupal::service('session_manager')->save();
 
-    return 'session_id:' . session_id() . "\n";
+    return ['#markup' => 'session_id:' . session_id() . "\n"];
   }
 
   /**
@@ -54,7 +54,7 @@ public function getId() {
    *   A notification message with session ID.
    */
   public function getIdFromCookie(Request $request) {
-    return 'session_id:' . $request->cookies->get(session_name()) . "\n";
+    return ['#markup' => 'session_id:' . $request->cookies->get(session_name()) . "\n"];
   }
 
   /**
@@ -69,7 +69,7 @@ public function getIdFromCookie(Request $request) {
   public function set($test_value) {
     $_SESSION['session_test_value'] = $test_value;
 
-    return $this->t('The current value of the stored session variable has been set to %val', array('%val' => $test_value));
+    return ['#markup' => $this->t('The current value of the stored session variable has been set to %val', array('%val' => $test_value))];
   }
 
   /**
@@ -85,7 +85,7 @@ public function set($test_value) {
   public function noSet($test_value) {
     \Drupal::service('session_manager')->disable();
     $this->set($test_value);
-    return $this->t('session saving was disabled, and then %val was set', array('%val' => $test_value));
+    return ['#markup' => $this->t('session saving was disabled, and then %val was set', array('%val' => $test_value))];
   }
 
   /**
@@ -111,6 +111,7 @@ public function setMessage() {
   public function setMessageButDontSave() {
     \Drupal::service('session_manager')->disable();
     $this->setMessage();
+    return ['#markup' => ''];
   }
 
   /**
@@ -124,6 +125,7 @@ public function setNotStarted() {
     if (!drupal_session_will_start()) {
       $this->set($this->t('Session was not started'));
     }
+    return ['#markup' => ''];
   }
 
   /**
@@ -133,6 +135,6 @@ public function setNotStarted() {
    *   A notification message.
    */
   public function isLoggedIn() {
-    return $this->t('User is logged in.');
+    return ['#markup' => $this->t('User is logged in.')];
   }
 }

core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php

@@ -49,7 +49,7 @@ public static function create(ContainerInterface $container) {
    *   The text to display.
    */
   public function mainContentFallback() {
-    return $this->t('Content to test main content fallback');
+    return ['#markup' => $this->t('Content to test main content fallback')];
   }
 
   /**
@@ -65,7 +65,7 @@ public function drupalSetMessageTest() {
 
     // Remove the first.
     unset($_SESSION['messages']['status'][0]);
-    return '';
+    return [];
   }
 
   /**
@@ -93,10 +93,10 @@ public function lockExit() {
    */
   public function lockPersist($lock_name) {
     if ($this->persistentLock->acquire($lock_name)) {
-      return 'TRUE: Lock successfully acquired in SystemTestController::lockPersist()';
+      return ['#markup' => 'TRUE: Lock successfully acquired in SystemTestController::lockPersist()'];
     }
     else {
-      return 'FALSE: Lock not acquired in SystemTestController::lockPersist()';
+      return ['#markup' => 'FALSE: Lock not acquired in SystemTestController::lockPersist()'];
     }
   }
 
@@ -154,8 +154,9 @@ public function shutdownFunctions($arg1, $arg2) {
     // @see _drupal_shutdown_function()
     // @see \Drupal\system\Tests\System\ShutdownFunctionsTest
     if (function_exists('fastcgi_finish_request')) {
-      return 'The function fastcgi_finish_request exists when serving the request.';
+      return ['#markup' => 'The function fastcgi_finish_request exists when serving the request.'];
     }
+    return [];
   }
 
   /**

core/modules/system/tests/modules/system_test/system_test.module

@@ -72,10 +72,10 @@ function system_test_system_info_alter(&$info, Extension $file, $type) {
 function system_test_lock_acquire() {
   if (\Drupal::lock()->acquire('system_test_lock_acquire')) {
     \Drupal::lock()->release('system_test_lock_acquire');
-    return 'TRUE: Lock successfully acquired in system_test_lock_acquire()';
+    return ['#markup' => 'TRUE: Lock successfully acquired in system_test_lock_acquire()'];
   }
   else {
-    return 'FALSE: Lock not acquired in system_test_lock_acquire()';
+    return ['#markup' => 'FALSE: Lock not acquired in system_test_lock_acquire()'];
   }
 }
 
@@ -91,7 +91,7 @@ function system_test_lock_exit() {
     exit();
   }
   else {
-    return 'FALSE: Lock not acquired in system_test_lock_exit()';
+    return ['#markup' => 'FALSE: Lock not acquired in system_test_lock_exit()'];
   }
 }
 

core/modules/system/tests/modules/theme_test/src/ThemeTestController.php

@@ -56,7 +56,7 @@ public function testInfoStylesheets() {
    *   A render array containing a theme override.
    */
   public function testTemplate() {
-    return \Drupal::theme()->render('theme_test_template_test', array());
+    return ['#markup' => \Drupal::theme()->render('theme_test_template_test', array())];
   }
 
   /**
@@ -82,7 +82,7 @@ public function testInlineTemplate() {
    *   An HTML string containing the themed output.
    */
   public function testSuggestion() {
-    return \Drupal::theme()->render(array('theme_test__suggestion', 'theme_test'), array());
+    return ['#markup' => \Drupal::theme()->render(array('theme_test__suggestion', 'theme_test'), array())];
   }
 
   /**
@@ -92,7 +92,7 @@ public function testSuggestion() {
    *   Content in theme_test_output GLOBAL.
    */
   public function testRequestListener() {
-    return $GLOBALS['theme_test_output'];
+    return ['#markup' =>  $GLOBALS['theme_test_output']];
   }
 
   /**

core/modules/system/tests/modules/twig_theme_test/src/TwigThemeTestController.php

@@ -18,7 +18,7 @@ class TwigThemeTestController {
    * Menu callback for testing PHP variables in a Twig template.
    */
   public function phpVariablesRender() {
-    return \Drupal::theme()->render('twig_theme_test_php_variables', array());
+    return ['#markup' => \Drupal::theme()->render('twig_theme_test_php_variables', array())];
   }
 
   /**