Issue #2505931 by stefan.r, akalata, cilefen, pwolanin, joelpittet, YesCT,...

Issue #2505931 by stefan.r, akalata, cilefen, pwolanin, joelpittet, YesCT, alexpott, davidhernandez, larowlan, colbol, xjm, Wim Leers, dawehner, Cottser, catch: Remove SafeMarkup::set in ViewListBuilder

core/modules/system/css/components/item-list.theme.css

@@ -17,3 +17,23 @@
 [dir="rtl"] .item-list ul li {
   margin: 0 1.5em 0.25em 0;
 }
+ul.item-list__comma-list {
+  display: inline;
+}
+ul.item-list__comma-list li {
+  display: inline;
+  list-style-type: none;
+}
+ul.item-list__comma-list,
+ul.item-list__comma-list li,
+[dir="rtl"] ul.item-list__comma-list,
+[dir="rtl"] ul.item-list__comma-list li {
+  margin: 0;
+  padding: 0;
+}
+ul.item-list__comma-list li:after {
+  content: ", ";
+}
+ul.item-list__comma-list li:last-child:after {
+  content: "";
+}

core/modules/system/templates/item-list.html.twig

@@ -12,12 +12,17 @@
  * - attributes: HTML attributes to be applied to the list.
  * - empty: A message to display when there are no items. Allowed value is a
  *   string or render array.
+ * - context: A list of contextual data associated with the list. May contain:
+ *   - list_style: The custom list style.
  *
  * @see template_preprocess_item_list()
  *
  * @ingroup themeable
  */
 #}
+{% if context.list_style %}
+  {% set attributes = attributes.addClass('item-list__' ~ context.list_style) %}
+{% endif %}
 {%- if items or empty -%}
   {%- if title is not empty -%}
     <h3>{{ title }}</h3>

core/modules/views_ui/src/Tests/DisplayPathTest.php

@@ -35,6 +35,7 @@ class DisplayPathTest extends UITestBase {
   public function testPathUI() {
     $this->doBasicPathUITest();
     $this->doAdvancedPathsValidationTest();
+    $this->doPathXssFilterTest();
   }
 
   /**
@@ -59,6 +60,28 @@ protected function doBasicPathUITest() {
     $this->assertLink(t('View @display', array('@display' => 'Page')), 0, 'view page link found on the page.');
   }
 
+  /**
+   * Tests that View paths are properly filtered for XSS.
+   */
+  public function doPathXssFilterTest() {
+    $this->drupalGet('admin/structure/views/view/test_view');
+    $this->drupalPostForm(NULL, array(), 'Add Page');
+    $this->drupalPostForm('admin/structure/views/nojs/display/test_view/page_2/path', array('path' => '<object>malformed_path</object>'), t('Apply'));
+    $this->drupalPostForm(NULL, array(), 'Add Page');
+    $this->drupalPostForm('admin/structure/views/nojs/display/test_view/page_3/path', array('path' => '<script>alert("hello");</script>'), t('Apply'));
+    $this->drupalPostForm(NULL, array(), 'Add Page');
+    $this->drupalPostForm('admin/structure/views/nojs/display/test_view/page_4/path', array('path' => '<script>alert("hello I have placeholders %");</script>'), t('Apply'));
+    $this->drupalPostForm('admin/structure/views/view/test_view', array(), t('Save'));
+    $this->drupalGet('admin/structure/views');
+    // The anchor text should be escaped.
+    $this->assertEscaped('/<object>malformed_path</object>');
+    $this->assertEscaped('/<script>alert("hello");</script>');
+    $this->assertEscaped('/<script>alert("hello I have placeholders %");</script>');
+    // Links should be url-encoded.
+    $this->assertRaw('/%3Cobject%3Emalformed_path%3C/object%3E');
+    $this->assertRaw('/%3Cscript%3Ealert%28%22hello%22%29%3B%3C/script%3E');
+  }
+
   /**
    * Tests a couple of invalid path patterns.
    */

core/modules/views_ui/src/ViewListBuilder.php

@@ -91,12 +91,6 @@ public function load() {
    */
   public function buildRow(EntityInterface $view) {
     $row = parent::buildRow($view);
-    $display_paths = '';
-    $separator = '';
-    foreach ($this->getDisplayPaths($view) as $display_path) {
-      $display_paths .= $separator . SafeMarkup::escape($display_path);
-      $separator = ', ';
-    }
     return array(
       'data' => array(
         'view_name' => array(
@@ -113,7 +107,13 @@ public function buildRow(EntityInterface $view) {
           'class' => array('views-table-filter-text-source'),
         ),
         'tag' => $view->get('tag'),
-        'path' => SafeMarkup::set($display_paths),
+        'path' => array(
+          'data' => array(
+            '#theme' => 'item_list',
+            '#items' => $this->getDisplayPaths($view),
+            '#context' => ['list_style' => 'comma-list'],
+          ),
+        ),
         'operations' => $row['operations'],
       ),
       'title' => $this->t('Machine name: @name', array('@name' => $view->id())),

core/modules/views_ui/tests/src/Unit/ViewListBuilderTest.php

@@ -89,7 +89,10 @@ public function testBuildRowEntityList() {
     );
     $page_display->expects($this->any())
       ->method('getPath')
-      ->will($this->returnValue('test_page'));
+      ->will($this->onConsecutiveCalls(
+        $this->returnValue('test_page'),
+        $this->returnValue('<object>malformed_path</object>'),
+        $this->returnValue('<script>alert("placeholder_page/%")</script>')));
 
     $embed_display = $this->getMock('Drupal\views\Plugin\views\display\Embed', array('initDisplay'),
       array(array(), 'default', $display_manager->getDefinition('embed'))
@@ -106,6 +109,16 @@ public function testBuildRowEntityList() {
     $values['display']['page_1']['display_plugin'] = 'page';
     $values['display']['page_1']['display_options']['path'] = 'test_page';
 
+    $values['display']['page_2']['id'] = 'page_2';
+    $values['display']['page_2']['display_title'] = 'Page 2';
+    $values['display']['page_2']['display_plugin'] = 'page';
+    $values['display']['page_2']['display_options']['path'] = '<object>malformed_path</object>';
+
+    $values['display']['page_3']['id'] = 'page_3';
+    $values['display']['page_3']['display_title'] = 'Page 3';
+    $values['display']['page_3']['display_plugin'] = 'page';
+    $values['display']['page_3']['display_options']['path'] = '<script>alert("placeholder_page/%")</script>';
+
     $values['display']['embed']['id'] = 'embed';
     $values['display']['embed']['display_title'] = 'Embedded';
     $values['display']['embed']['display_plugin'] = 'embed';
@@ -115,6 +128,8 @@ public function testBuildRowEntityList() {
       ->will($this->returnValueMap(array(
         array('default', $values['display']['default'], $default_display),
         array('page', $values['display']['page_1'], $page_display),
+        array('page', $values['display']['page_2'], $page_display),
+        array('page', $values['display']['page_3'], $page_display),
         array('embed', $values['display']['embed'], $embed_display),
       )));
 
@@ -141,8 +156,16 @@ public function testBuildRowEntityList() {
 
     $row = $view_list_builder->buildRow($view);
 
-    $this->assertEquals(array('Embed admin label', 'Page admin label'), $row['data']['view_name']['data']['#displays'], 'Wrong displays got added to view list');
-    $this->assertEquals($row['data']['path'], '/test_page', 'The path of the page display is not added.');
+    $expected_displays = array(
+      'Embed admin label',
+      'Page admin label',
+      'Page admin label',
+      'Page admin label',
+    );
+    $this->assertEquals($expected_displays, $row['data']['view_name']['data']['#displays']);
+
+    $display_paths = $row['data']['path']['data']['#items'];
+    $this->assertEquals('/test_page, /&lt;object&gt;malformed_path&lt;/object&gt;, /&lt;script&gt;alert(&quot;placeholder_page/%&quot;)&lt;/script&gt;', implode(', ', $display_paths));
   }
 
 }

core/themes/bartik/css/components/item-list.css

@@ -10,3 +10,10 @@
 [dir="rtl"] .item-list ul li {
   padding: 0.2em 0 0 0.5em;
 }
+.item-list .item-list__comma-list,
+.item-list .item-list__comma-list li,
+[dir="rtl"] .item-list .item-list__comma-list,
+[dir="rtl"] .item-list .item-list__comma-list li {
+  margin: 0;
+  padding: 0;
+}

core/themes/classy/templates/dataset/item-list.html.twig

@@ -12,10 +12,15 @@
  * - attributes: HTML attributes to be applied to the list.
  * - empty: A message to display when there are no items. Allowed value is a
  *   string or render array.
+ * - context: A list of contextual data associated with the list. May contain:
+ *   - list_style: The custom list style.
  *
  * @see template_preprocess_item_list()
  */
 #}
+{% if context.list_style %}
+  {% set attributes = attributes.addClass('item-list__' ~ context.list_style) %}
+{% endif %}
 {%- if items or empty -%}
   <div class="item-list">
     {%- if title is not empty -%}

core/themes/seven/css/components/menus-and-lists.css

@@ -38,3 +38,6 @@ ul.inline li {
 ul.inline li {
   display: inline;
 }
+[dir="rtl"] ul.item-list__comma-list {
+  margin: 0;
+}