#715142: remove the confusing check_url().

From: Damien Tournoud <damien@tournoud.net>


---
 common.inc                      |   33 +++++++++------------------------
 form.inc                        |    4 ++--
 install.core.inc                |    2 +-
 theme.inc                       |   12 ++++++------
 theme.maintenance.inc           |    2 +-
 aggregator/aggregator.module    |    2 +-
 aggregator/aggregator.pages.inc |   12 ++++++------
 comment/comment.tokens.inc      |    2 +-
 filter/filter.module            |    2 +-
 profile/profile.module          |    2 +-
 search/search.pages.inc         |    2 +-
 simpletest/tests/common.test    |    2 +-
 update.php                      |    2 +-
 13 files changed, 32 insertions(+), 47 deletions(-)

diff --git includes/common.inc includes/common.inc
index 49d5921..efcd34e 100644
--- includes/common.inc
+++ includes/common.inc
@@ -1180,13 +1180,6 @@ function flood_is_allowed($name, $threshold, $window = 3600, $identifier = NULL)
  */
 
 /**
- * Prepare a URL for use in an HTML attribute. Strips harmful protocols.
- */
-function check_url($uri) {
-  return filter_xss_bad_protocol($uri, FALSE);
-}
-
-/**
  * Very permissive XSS/HTML filter for admin-only use.
  *
  * Use only for fields where it is impractical to use the
@@ -1362,7 +1355,7 @@ function _filter_xss_attributes($attr) {
       case 2:
         // Attribute value, a URL after href= for instance
         if (preg_match('/^"([^"]*)"(\s+|$)/', $attr, $match)) {
-          $thisval = filter_xss_bad_protocol($match[1]);
+          $thisval = check_plain(filter_xss_bad_protocol(decode_entities($match[1])));
 
           if (!$skip) {
             $attrarr[] = "$attrname=\"$thisval\"";
@@ -1374,7 +1367,7 @@ function _filter_xss_attributes($attr) {
         }
 
         if (preg_match("/^'([^']*)'(\s+|$)/", $attr, $match)) {
-          $thisval = filter_xss_bad_protocol($match[1]);
+          $thisval = check_plain(filter_xss_bad_protocol(decode_entities($match[1])));
 
           if (!$skip) {
             $attrarr[] = "$attrname='$thisval'";
@@ -1385,7 +1378,7 @@ function _filter_xss_attributes($attr) {
         }
 
         if (preg_match("%^([^\s\"']+)(\s+|$)%", $attr, $match)) {
-          $thisval = filter_xss_bad_protocol($match[1]);
+          $thisval = check_plain(filter_xss_bad_protocol(decode_entities($match[1])));
 
           if (!$skip) {
             $attrarr[] = "$attrname=\"$thisval\"";
@@ -1425,24 +1418,16 @@ function _filter_xss_attributes($attr) {
  *
  * @param $string
  *   The string with the attribute value.
- * @param $decode
- *   Whether to decode entities in the $string. Set to FALSE if the $string
- *   is in plain text, TRUE otherwise. Defaults to TRUE.
  * @return
  *   Cleaned up and HTML-escaped version of $string.
  */
-function filter_xss_bad_protocol($string, $decode = TRUE) {
+function filter_xss_bad_protocol($string) {
   static $allowed_protocols;
 
   if (!isset($allowed_protocols)) {
     $allowed_protocols = array_flip(variable_get('filter_allowed_protocols', array('ftp', 'http', 'https', 'irc', 'mailto', 'news', 'nntp', 'rtsp', 'sftp', 'ssh', 'telnet', 'webcal')));
   }
 
-  // Get the plain text representation of the attribute value (i.e. its meaning).
-  if ($decode) {
-    $string = decode_entities($string);
-  }
-
   // Iteratively remove any invalid protocol found.
   do {
     $before = $string;
@@ -1464,7 +1449,7 @@ function filter_xss_bad_protocol($string, $decode = TRUE) {
     }
   } while ($before != $string);
 
-  return check_plain($string);
+  return $string;
 }
 
 /**
@@ -1488,7 +1473,7 @@ function format_rss_channel($title, $link, $description, $items, $langcode = NUL
 
   $output = "<channel>\n";
   $output .= ' <title>' . check_plain($title) . "</title>\n";
-  $output .= ' <link>' . check_url($link) . "</link>\n";
+  $output .= ' <link>' . check_plain(filter_xss_bad_protocol($link)) . "</link>\n";
 
   // The RSS 2.0 "spec" doesn't indicate HTML can be used in the description.
   // We strip all HTML tags, but need to prevent double encoding from properly
@@ -1510,7 +1495,7 @@ function format_rss_channel($title, $link, $description, $items, $langcode = NUL
 function format_rss_item($title, $link, $description, $args = array()) {
   $output = "<item>\n";
   $output .= ' <title>' . check_plain($title) . "</title>\n";
-  $output .= ' <link>' . check_url($link) . "</link>\n";
+  $output .= ' <link>' . check_plain(filter_xss_bad_protocol($link)) . "</link>\n";
   $output .= ' <description>' . check_plain($description) . "</description>\n";
   $output .= format_xml_elements($args);
   $output .= "</item>\n";
@@ -1953,7 +1938,7 @@ function url($path = NULL, array $options = array()) {
     // Note: we could use url_is_external($path) here, but that would
     // require another function call, and performance inside url() is critical.
     $colonpos = strpos($path, ':');
-    $options['external'] = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path));
+    $options['external'] = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path) == $path);
   }
 
   // Preserve the original path before altering or aliasing.
@@ -2076,7 +2061,7 @@ function url_is_external($path) {
   $colonpos = strpos($path, ':');
   // Only call the slow filter_xss_bad_protocol if $path contains a ':'
   // before any / ? or #.
-  return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path);
+  return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path) == $path;
 }
 
 /**
diff --git includes/form.inc includes/form.inc
index 2e390b6..3ba0ab3 100644
--- includes/form.inc
+++ includes/form.inc
@@ -2831,7 +2831,7 @@ function theme_textfield($variables) {
   if ($element['#autocomplete_path'] && drupal_valid_path($element['#autocomplete_path'])) {
     drupal_add_js('misc/autocomplete.js');
     $class[] = 'form-autocomplete';
-    $extra =  '<input class="autocomplete" type="hidden" id="' . $element['#id'] . '-autocomplete" value="' . check_url(url($element['#autocomplete_path'], array('absolute' => TRUE))) . '" disabled="disabled" />';
+    $extra =  '<input class="autocomplete" type="hidden" id="' . $element['#id'] . '-autocomplete" value="' . check_plain(url($element['#autocomplete_path'], array('absolute' => TRUE))) . '" disabled="disabled" />';
   }
   _form_set_class($element, $class);
 
@@ -2856,7 +2856,7 @@ function theme_textfield($variables) {
 function theme_form($variables) {
   $element = $variables['element'];
   // Anonymous div to satisfy XHTML compliance.
-  $action = $element['#action'] ? 'action="' . check_url($element['#action']) . '" ' : '';
+  $action = $element['#action'] ? 'action="' . check_plain(filter_xss_bad_protocol($element['#action'])) . '" ' : '';
   return '<form ' . $action . ' accept-charset="UTF-8" method="' . $element['#method'] . '" id="' . $element['#id'] . '"' . drupal_attributes($element['#attributes']) . ">\n<div>" . $element['#children'] . "\n</div></form>\n";
 }
 
diff --git includes/install.core.inc includes/install.core.inc
index dda7ded..97433e1 100644
--- includes/install.core.inc
+++ includes/install.core.inc
@@ -723,7 +723,7 @@ function install_verify_requirements(&$install_state) {
     if ($install_state['interactive']) {
       drupal_set_title(st('Requirements problem'));
       $status_report = theme('status_report', array('requirements' => $requirements));
-      $status_report .= st('Check the error messages and <a href="!url">proceed with the installation</a>.', array('!url' => check_url(request_uri())));
+      $status_report .= st('Check the error messages and <a href="!url">proceed with the installation</a>.', array('!url' => check_plain(filter_xss_bad_protocol(request_uri()))));
       return $status_report;
     }
     else {
diff --git includes/theme.inc includes/theme.inc
index 5e47764..aa32a39 100644
--- includes/theme.inc
+++ includes/theme.inc
@@ -1527,7 +1527,7 @@ function theme_image($variables) {
   if (!$getsize || (is_file($path) && (list($width, $height, $type, $image_attributes) = @getimagesize($path)))) {
     $attributes = drupal_attributes($attributes);
     $url = file_create_url($path);
-    return '<img src="' . check_url($url) . '" alt="' . check_plain($alt) . '" title="' . check_plain($title) . '" ' . (isset($image_attributes) ? $image_attributes : '') . $attributes . ' />';
+    return '<img src="' . check_plain(filter_xss_bad_protocol($url)) . '" alt="' . check_plain($alt) . '" title="' . check_plain($title) . '" ' . (isset($image_attributes) ? $image_attributes : '') . $attributes . ' />';
   }
 }
 
@@ -1880,7 +1880,7 @@ function theme_item_list($variables) {
  * Returns code that emits the 'more help'-link.
  */
 function theme_more_help_link($variables) {
-  return '<div class="more-help-link">' . t('<a href="@link">More help</a>', array('@link' => check_url($variables['url']))) . '</div>';
+  return '<div class="more-help-link">' . t('<a href="@link">More help</a>', array('@link' => check_plain(filter_xss_bad_protocol($variables['url'])))) . '</div>';
 }
 
 /**
@@ -1894,7 +1894,7 @@ function theme_more_help_link($variables) {
 function theme_feed_icon($variables) {
   $text = t('Subscribe to @feed-title', array('@feed-title' => $variables['title']));
   if ($image = theme('image', array('path' => 'misc/feed.png', 'alt' => $text))) {
-    return '<a href="' . check_url($variables['url']) . '" title="' . $text . '" class="feed-icon">' . $image . '</a>';
+    return '<a href="' . check_plain(filter_xss_bad_protocol($variables['url']) ). '" title="' . $text . '" class="feed-icon">' . $image . '</a>';
   }
 }
 
@@ -1947,7 +1947,7 @@ function theme_html_tag($variables) {
  *   - title: A descriptive verb for the link, like 'Read more'
  */
 function theme_more_link($variables) {
-  return '<div class="more-link">' . t('<a href="@link" title="@title">More</a>', array('@link' => check_url($variables['url']), '@title' => $variables['title'])) . '</div>';
+  return '<div class="more-link">' . t('<a href="@link" title="@title">More</a>', array('@link' => check_plain(filter_xss_bad_protocol($variables['url'])), '@title' => $variables['title'])) . '</div>';
 }
 
 /**
@@ -2279,7 +2279,7 @@ function template_preprocess_html(&$variables) {
   if (theme_get_setting('toggle_favicon')) {
     $favicon = theme_get_setting('favicon');
     $type = theme_get_setting('favicon_mimetype');
-    drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_url($favicon), 'type' => $type));
+    drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_plain(filter_xss_bad_protocol($favicon)), 'type' => $type));
   }
 
   // Construct page title.
@@ -2463,7 +2463,7 @@ function template_preprocess_maintenance_page(&$variables) {
   if (theme_get_setting('toggle_favicon')) {
     $favicon = theme_get_setting('favicon');
     $type = theme_get_setting('favicon_mimetype');
-    drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_url($favicon), 'type' => $type));
+    drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_plain(filter_xss_bad_protocol($favicon)), 'type' => $type));
   }
 
   global $theme;
diff --git includes/theme.maintenance.inc includes/theme.maintenance.inc
index b12eaaf..749d5ca 100644
--- includes/theme.maintenance.inc
+++ includes/theme.maintenance.inc
@@ -145,7 +145,7 @@ function theme_install_page($variables) {
     $title = count($messages['error']) > 1 ? st('The following errors must be resolved before you can continue the installation process') : st('The following error must be resolved before you can continue the installation process');
     $variables['messages'] .= '<h3>' . $title . ':</h3>';
     $variables['messages'] .= theme('status_messages', array('display' => 'error'));
-    $variables['content'] .= '<p>' . st('Check the error messages and <a href="!url">try again</a>.', array('!url' => check_url(request_uri()))) . '</p>';
+    $variables['content'] .= '<p>' . st('Check the error messages and <a href="!url">try again</a>.', array('!url' => check_plain(filter_xss_bad_protocol(request_uri())))) . '</p>';
   }
 
   // Special handling of warning messages
diff --git modules/aggregator/aggregator.module modules/aggregator/aggregator.module
index a57e52f..7fe1e7c 100644
--- modules/aggregator/aggregator.module
+++ modules/aggregator/aggregator.module
@@ -716,7 +716,7 @@ function aggregator_category_load($cid) {
  */
 function theme_aggregator_block_item($variables) {
   // Display the external link to the item.
-  return '<a href="' . check_url($variables['item']->link) . '">' . check_plain($variables['item']->title) . "</a>\n";
+  return '<a href="' . check_plain(filter_xss_bad_protocol($variables['item']->link)) . '">' . check_plain($variables['item']->title) . "</a>\n";
 }
 
 /**
diff --git modules/aggregator/aggregator.pages.inc modules/aggregator/aggregator.pages.inc
index 4b58067..fcf67f3 100644
--- modules/aggregator/aggregator.pages.inc
+++ modules/aggregator/aggregator.pages.inc
@@ -281,7 +281,7 @@ function template_preprocess_aggregator_wrapper(&$variables) {
 function template_preprocess_aggregator_item(&$variables) {
   $item = $variables['item'];
 
-  $variables['feed_url'] = check_url($item->link);
+  $variables['feed_url'] = check_plain(filter_xss_bad_protocol($item->link));
   $variables['feed_title'] = check_plain($item->title);
   $variables['content'] = aggregator_filter_xss($item->description);
 
@@ -393,7 +393,7 @@ function theme_aggregator_page_rss($variables) {
       case 'teaser':
         $summary = text_summary($feed->description, NULL, variable_get('aggregator_teaser_length', 600));
         if ($summary != $feed->description) {
-          $summary .= '<p><a href="' . check_url($feed->link) . '">' . t('read more') . "</a></p>\n";
+          $summary .= '<p><a href="' . check_plain(filter_xss_bad_protocol($feed->link)) . '">' . t('read more') . "</a></p>\n";
         }
         $feed->description = $summary;
         break;
@@ -458,7 +458,7 @@ function theme_aggregator_page_opml($variables) {
   $output .= "</head>\n";
   $output .= "<body>\n";
   foreach ($feeds as $feed) {
-    $output .= '<outline text="' . check_plain($feed->title) . '" xmlUrl="' . check_url($feed->url) . "\" />\n";
+    $output .= '<outline text="' . check_plain($feed->title) . '" xmlUrl="' . check_plain(filter_xss_bad_protocol($feed->url)) . "\" />\n";
   }
   $output .= "</body>\n";
   $output .= "</opml>\n";
@@ -485,14 +485,14 @@ function template_preprocess_aggregator_summary_items(&$variables) {
 function template_preprocess_aggregator_summary_item(&$variables) {
   $item = $variables['item'];
 
-  $variables['feed_url'] = check_url($item->link);
+  $variables['feed_url'] = check_plain(filter_xss_bad_protocol($item->link));
   $variables['feed_title'] = check_plain($item->title);
   $variables['feed_age'] = t('%age old', array('%age' => format_interval(REQUEST_TIME - $item->timestamp)));
 
   $variables['source_url'] = '';
   $variables['source_title'] = '';
   if (!empty($item->feed_link)) {
-    $variables['source_url'] = check_url($item->feed_link);
+    $variables['source_url'] = check_plain(filter_xss_bad_protocol($item->feed_link));
     $variables['source_title'] = check_plain($item->feed_title);
   }
 }
@@ -508,7 +508,7 @@ function template_preprocess_aggregator_feed_source(&$variables) {
   $variables['source_icon'] = theme('feed_icon', array('url' => $feed->url, 'title' => t('!title feed', array('!title' => $feed->title))));
   $variables['source_image'] = $feed->image;
   $variables['source_description'] = aggregator_filter_xss($feed->description);
-  $variables['source_url'] = check_url(url($feed->link, array('absolute' => TRUE)));
+  $variables['source_url'] = check_plain(filter_xss_bad_protocol(url($feed->link, array('absolute' => TRUE))));
 
   if ($feed->checked) {
     $variables['last_checked'] = t('@time ago', array('@time' => format_interval(REQUEST_TIME - $feed->checked)));
diff --git modules/comment/comment.tokens.inc modules/comment/comment.tokens.inc
index 848ec0f..305db13 100644
--- modules/comment/comment.tokens.inc
+++ modules/comment/comment.tokens.inc
@@ -167,7 +167,7 @@ function comment_tokens($type, $tokens, array $data = array(), array $options =
           break;
 
         case 'homepage':
-          $replacements[$original] = $sanitize ? filter_xss_bad_protocol($comment->homepage) : $comment->homepage;
+          $replacements[$original] = $sanitize ? check_plain(filter_xss_bad_protocol($comment->homepage)) : $comment->homepage;
           break;
 
         case 'title':
diff --git modules/filter/filter.module modules/filter/filter.module
index ff1d132..54f19af 100644
--- modules/filter/filter.module
+++ modules/filter/filter.module
@@ -1317,7 +1317,7 @@ function _filter_url($text, $filter) {
 function _filter_url_parse_full_links($match) {
   $match[2] = decode_entities($match[2]);
   $caption = check_plain(_filter_url_trim($match[2]));
-  $match[2] = check_url($match[2]);
+  $match[2] = check_plain(filter_xss_bad_protocol($match[2]));
   return $match[1] . '<a href="' . $match[2] . '" title="' . $match[2] . '">' . $caption . '</a>' . $match[5];
 }
 
diff --git modules/profile/profile.module modules/profile/profile.module
index 542799a..c7b01a1 100644
--- modules/profile/profile.module
+++ modules/profile/profile.module
@@ -296,7 +296,7 @@ function profile_view_field($account, $field) {
       case 'checkbox':
         return $browse ? l($field->title, 'profile/' . $field->name) : check_plain($field->title);
       case 'url':
-        return '<a href="' . check_url($value) . '">' . check_plain($value) . '</a>';
+        return '<a href="' . check_plain(filter_xss_bad_protocol($value)) . '">' . check_plain($value) . '</a>';
       case 'date':
         $format = substr(variable_get('date_format_short', 'm/d/Y - H:i'), 0, 5);
         // Note: Avoid PHP's date() because it does not handle dates before
diff --git modules/search/search.pages.inc modules/search/search.pages.inc
index b7f6ba2..4361dd3 100644
--- modules/search/search.pages.inc
+++ modules/search/search.pages.inc
@@ -91,7 +91,7 @@ function template_preprocess_search_results(&$variables) {
  */
 function template_preprocess_search_result(&$variables) {
   $result = $variables['result'];
-  $variables['url'] = check_url($result['link']);
+  $variables['url'] = check_plain(filter_xss_bad_protocol($result['link']));
   $variables['title'] = check_plain($result['title']);
 
   $info = array();
diff --git modules/simpletest/tests/common.test modules/simpletest/tests/common.test
index 089d07c..8d89801 100644
--- modules/simpletest/tests/common.test
+++ modules/simpletest/tests/common.test
@@ -76,7 +76,7 @@ class CommonURLUnitTest extends DrupalWebTestCase {
     $text = $this->randomName();
     $path = "<SCRIPT>alert('XSS')</SCRIPT>";
     $link = l($text, $path);
-    $sanitized_path = check_url(url($path));
+    $sanitized_path = check_plain(filter_xss_bad_protocol(url($path)));
     $this->assertTrue(strpos($link, $sanitized_path) !== FALSE, t('XSS attack @path was filtered', array('@path' => $path)));
   }
 
diff --git update.php update.php
index e3d7f5c..38c1768 100644
--- update.php
+++ update.php
@@ -316,7 +316,7 @@ function update_check_requirements() {
     update_task_list('requirements');
     drupal_set_title('Requirements problem');
     $status_report = theme('status_report', array('requirements' => $requirements));
-    $status_report .= 'Check the error messages and <a href="' . check_url(request_uri()) . '">try again</a>.';
+    $status_report .= 'Check the error messages and <a href="' . check_plain(filter_xss_bad_protocol(request_uri())) . '">try again</a>.';
     print theme('update_page', array('content' => $status_report));
     exit();
   }