Issue #2296885 by mikeker, dawehner: Remove format_xml_elements()

core/includes/common.inc

@@ -295,53 +295,6 @@ function check_url($uri) {
  * Functions to format numbers, strings, dates, etc.
  */
 
-/**
- * Formats XML elements.
- *
- * Note: It is the caller's responsibility to sanitize any input parameters.
- * This function does not perform sanitization.
- *
- * @param $array
- *   An array where each item represents an element and is either a:
- *   - (key => value) pair (<key>value</key>)
- *   - Associative array with fields:
- *     - 'key': The element name. Element names are not sanitized, so do not
- *       pass user input.
- *     - 'value': element contents
- *     - 'attributes': associative array of element attributes
- *
- * In both cases, 'value' can be a simple string, or it can be another array
- * with the same format as $array itself for nesting.
- */
-function format_xml_elements($array) {
-  $output = '';
-  foreach ($array as $key => $value) {
-    if (is_numeric($key)) {
-      if ($value['key']) {
-        $output .= ' <' . $value['key'];
-        if (isset($value['attributes']) && is_array($value['attributes'])) {
-          $output .= new Attribute($value['attributes']);
-        }
-
-        if (isset($value['value']) && $value['value'] != '') {
-          $output .= '>' . (is_array($value['value']) ? format_xml_elements($value['value']) : SafeMarkup::checkPlain($value['value'])) . '</' . $value['key'] . ">\n";
-        }
-        else {
-          $output .= " />\n";
-        }
-      }
-    }
-    else {
-      $output .= ' <' . $key . '>' . (is_array($value) ? format_xml_elements($value) : SafeMarkup::checkPlain($value)) . "</$key>\n";
-    }
-  }
-  // @todo This is marking the output string as safe HTML, but we have only
-  //   sanitized the attributes and tag values, not the tag names, and we
-  //   cannot guarantee the assembled markup is safe. Consider a fix in:
-  //   https://www.drupal.org/node/2296885
-  return SafeMarkup::set($output);
-}
-
 /**
  * Generates a string representation for the given byte count.
  *

core/modules/file/src/Tests/FileFieldRSSContentTest.php

@@ -68,15 +68,12 @@ class FileFieldRSSContentTest extends FileFieldTestBase {
     // Check that the RSS enclosure appears in the RSS feed.
     $this->drupalGet('rss.xml');
     $uploaded_filename = str_replace('public://', '', $node_file->getFileUri());
-    $test_element = array(
-      'key' => 'enclosure',
-      'value' => "",
-      'attributes' => array(
-        'url' => file_create_url("public://$uploaded_filename", array('absolute' => TRUE)),
-        'length' => $node_file->getSize(),
-        'type' => $node_file->getMimeType()
-      ),
+    $test_element = sprintf(
+      '<enclosure url="%s" length="%s" type="%s" />',
+      file_create_url("public://$uploaded_filename", array('absolute' => TRUE)),
+      $node_file->getSize(),
+      $node_file->getMimeType()
     );
-    $this->assertRaw(format_xml_elements(array($test_element)), 'File field RSS enclosure is displayed when viewing the RSS feed.');
+    $this->assertRaw($test_element, 'File field RSS enclosure is displayed when viewing the RSS feed.');
   }
 }

core/modules/node/src/Tests/NodeRSSContentTest.php

@@ -54,12 +54,9 @@ class NodeRSSContentTest extends NodeTestBase {
     $this->assertNoText($non_rss_content, 'Node content not designed for RSS does not appear in RSS feed.');
 
     // Check that extra RSS elements and namespaces are added to RSS feed.
-    $test_element = array(
-      'key' => 'testElement',
-      'value' => t('Value of testElement RSS element for node !nid.', array('!nid' => $node->id())),
-    );
+    $test_element = '<testElement>' . t('Value of testElement RSS element for node !nid.', array('!nid' => $node->id())) . '</testElement>';
     $test_ns = 'xmlns:drupaltest="http://example.com/test-namespace"';
-    $this->assertRaw(format_xml_elements(array($test_element)), 'Extra RSS elements appear in RSS feed.');
+    $this->assertRaw($test_element, 'Extra RSS elements appear in RSS feed.');
     $this->assertRaw($test_ns, 'Extra namespaces appear in RSS feed.');
 
     // Check that content added in 'rss' view mode doesn't appear when

core/modules/taxonomy/src/Tests/RssTest.php

@@ -97,14 +97,12 @@ class RssTest extends TaxonomyTestBase {
 
     // Check that the term is displayed when the RSS feed is viewed.
     $this->drupalGet('rss.xml');
-    $test_element = array(
-      'key' => 'category',
-      'value' => $term1->getName(),
-      'attributes' => array(
-        'domain' => $term1->url('canonical', array('absolute' => TRUE)),
-      ),
+    $test_element = sprintf(
+      '<category %s>%s</category>',
+      'domain="' . $term1->url('canonical', array('absolute' => TRUE)) . '"',
+      $term1->getName()
     );
-    $this->assertRaw(format_xml_elements(array($test_element)), 'Term is displayed when viewing the rss feed.');
+    $this->assertRaw($test_element, 'Term is displayed when viewing the rss feed.');
 
     // Test that the feed page exists for the term.
     $this->drupalGet("taxonomy/term/{$term1->id()}/feed");
@@ -121,12 +119,9 @@ class RssTest extends TaxonomyTestBase {
     $view->storage->save();
     // Check the article is shown in the feed.
     $node = $this->drupalGetNodeByTitle($edit['title[0][value]']);
-    $raw_xml = format_xml_elements([[
-      'key' => 'title',
-      'value' => $node->label(),
-    ]]);
+    $raw_xml = '<title>' . $node->label() . '</title>';
     $this->drupalGet('taxonomy/term/all/feed');
-    $this->assertRaw($raw_xml);
+    $this->assertRaw($raw_xml, "Raw text '$raw_xml' is found.");
     // Unpublish the article and check that it is not shown in the feed.
     $node->setPublished(FALSE)->save();
     $this->drupalGet('taxonomy/term/all/feed');

core/modules/views/src/Plugin/views/style/Rss.php

@@ -83,7 +83,7 @@ class Rss extends StylePluginBase {
    * Return an array of additional XHTML elements to add to the channel.
    *
    * @return
-   *   An array that can be passed to format_xml_elements().
+   *   A render array.
    */
   protected function getChannelElements() {
     return array();

core/modules/views/src/Tests/Plugin/DisplayFeedTest.php

@@ -45,7 +45,12 @@ class DisplayFeedTest extends PluginTestBase {
    */
   public function testFeedOutput() {
     $this->drupalCreateContentType(['type' => 'page']);
-    $node = $this->drupalCreateNode();
+
+    // Verify a title with HTML entities is properly escaped.
+    $node_title = 'This "cool" & "neat" article\'s title';
+    $node = $this->drupalCreateNode(array(
+      'title' => $node_title
+    ));
 
     // Test the site name setting.
     $site_name = $this->randomMachineName();
@@ -54,6 +59,7 @@ class DisplayFeedTest extends PluginTestBase {
     $this->drupalGet('test-feed-display.xml');
     $result = $this->xpath('//title');
     $this->assertEqual($result[0], $site_name, 'The site title is used for the feed title.');
+    $this->assertEqual($result[1], $node_title, 'Node title with HTML entities displays correctly.');
 
     $view = $this->container->get('entity.manager')->getStorage('view')->load('test_display_feed');
     $display = &$view->getDisplay('feed_1');
@@ -86,6 +92,23 @@ class DisplayFeedTest extends PluginTestBase {
     $this->assertEqual($feed_link, $page_url, 'The channel link was found.');
   }
 
+  /**
+   * Tests the rendered output for fields display.
+   */
+  public function testFeedFieldOutput() {
+    $this->drupalCreateContentType(['type' => 'page']);
+
+    // Verify a title with HTML entities is properly escaped.
+    $node_title = 'This "cool" & "neat" article\'s title';
+    $this->drupalCreateNode(array(
+      'title' => $node_title
+    ));
+
+    $this->drupalGet('test-feed-display-fields.xml');
+    $result = $this->xpath('//title/a');
+    $this->assertEqual($result[0], $node_title, 'Node title with HTML entities displays correctly.');
+  }
+
   /**
    * Tests that nothing is output when the feed display is disabled.
    */
@@ -118,4 +141,5 @@ class DisplayFeedTest extends PluginTestBase {
     $this->drupalGet('/test-attached-disabled.xml');
     $this->assertResponse(404);
   }
+
 }

core/modules/views/templates/views-view-row-rss.html.twig

@@ -7,16 +7,24 @@
  * - title: RSS item title.
  * - link: RSS item link.
  * - description: RSS body text.
- * - item_elements: RSS item elements rendered as XML (pubDate, creator, guid).
+ * - item_elements: RSS item elements to be rendered as XML (pubDate, creator,
+ *   guid).
  *
  * @see template_preprocess_views_view_row_rss()
  *
  * @ingroup themeable
  */
 #}
-  <item>
-    <title>{{ title }}</title>
-    <link>{{ link }}</link>
-    <description>{{ description }}</description>
-    {{ item_elements }}
-  </item>
+<item>
+  <title>{{ title }}</title>
+  <link>{{ link }}</link>
+  <description>{{ description }}</description>
+  {% for item in item_elements -%}
+    <{{ item.key }}{{ item.attributes -}}
+    {% if item.value -%}
+      >{{ item.value }}</{{ item.key }}>
+    {% else -%}
+      {{ ' />' }}
+    {% endif %}
+  {%- endfor %}
+</item>

core/modules/views/tests/modules/views_test_config/test_views/views.view.test_display_feed.yml

@@ -94,6 +94,31 @@ display:
     display_title: Feed
     id: feed_1
     position: 0
+  feed_2:
+    display_options:
+      displays: {  }
+      pager:
+        type: some
+      path: test-feed-display-fields.xml
+      row:
+        type: rss_fields
+        options:
+          title_field: title
+          link_field: title
+          description_field: title
+          creator_field: title
+          date_field: title
+          guid_field_options:
+            guid_field: title
+            guid_field_is_permalink: true
+      style:
+        type: rss
+      sitename_title: true
+      display_description: ''
+    display_plugin: feed
+    display_title: 'Feed with Fields'
+    id: feed_2
+    position: 0
   page:
     display_options:
       path: test-feed-display

core/modules/views/views.theme.inc

@@ -895,7 +895,7 @@ function template_preprocess_views_view_rss(&$variables) {
   $variables['langcode'] = SafeMarkup::checkPlain(\Drupal::languageManager()->getCurrentLanguage()->getId());
   $variables['namespaces'] = new Attribute($style->namespaces);
   $variables['items'] = $items;
-  $variables['channel_elements'] = format_xml_elements($style->channel_elements);
+  $variables['channel_elements'] = \Drupal::service('renderer')->render($style->channel_elements);
 
   // During live preview we don't want to output the header since the contents
   // of the feed are being displayed inside a normal HTML page.
@@ -915,11 +915,16 @@ function template_preprocess_views_view_rss(&$variables) {
  */
 function template_preprocess_views_view_row_rss(&$variables) {
   $item = $variables['row'];
-
-  $variables['title'] = SafeMarkup::checkPlain($item->title);
+  $variables['title'] = $item->title;
   $variables['link'] = $item->link;
-  $variables['description'] = SafeMarkup::checkPlain($item->description);
-  $variables['item_elements'] = empty($item->elements) ? '' : format_xml_elements($item->elements);
+  $variables['description'] = $item->description;
+  $variables['item_elements'] = array();
+  foreach ($item->elements as $element) {
+    if (isset($element['attributes']) && is_array($element['attributes'])) {
+      $element['attributes'] = new Attribute($element['attributes']);
+    }
+    $variables['item_elements'][] = $element;
+  }
 }
 
 /**

core/themes/classy/templates/views/views-view-row-rss.html.twig

@@ -7,14 +7,24 @@
  * - title: RSS item title.
  * - link: RSS item link.
  * - description: RSS body text.
- * - item_elements: RSS item elements rendered as XML (pubDate, creator, guid).
+ * - item_elements: RSS item elements to be rendered as XML (pubDate, creator,
+ *   guid).
  *
  * @see template_preprocess_views_view_row_rss()
+ *
+ * @ingroup themeable
  */
 #}
-  <item>
-    <title>{{ title }}</title>
-    <link>{{ link }}</link>
-    <description>{{ description }}</description>
-    {{ item_elements }}
-  </item>
+<item>
+  <title>{{ title }}</title>
+  <link>{{ link }}</link>
+  <description>{{ description }}</description>
+  {% for item in item_elements -%}
+  <{{ item.key }}{{ item.attributes -}}
+  {% if item.value -%}
+  >{{ item.value }}</{{ item.key }}>
+    {% else -%}
+  {{ ' />' }}
+    {% endif %}
+  {%- endfor %}
+</item>