diff --git a/includes/common.inc b/includes/common.inc index 0d95442a8000e2938edaefb7466f2f9a71440d4c..ea57e6593d3013a44f4181d50947639aea54f49c 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -233,7 +233,7 @@ function drupal_query_string_encode($query, $exclude = array(), $parent = '') { $params = array(); foreach ($query as $key => $value) { - $key = drupal_urlencode($key); + $key = rawurlencode($key); if ($parent) { $key = $parent . '[' . $key . ']'; } @@ -246,7 +246,7 @@ function drupal_query_string_encode($query, $exclude = array(), $parent = '') { $params[] = drupal_query_string_encode($value, $exclude, $key); } else { - $params[] = $key . '=' . drupal_urlencode($value); + $params[] = $key . '=' . rawurlencode($value); } } @@ -1958,8 +1958,8 @@ function format_date($timestamp, $type = 'medium', $format = '', $timezone = NUL * @param $options * An associative array of additional options, with the following keys: * - 'query' - * A query string to append to the link, or an array of query key/value - * properties. + * A URL-encoded query string to append to the link, or an array of query + * key/value-pairs without any URL-encoding. * - 'fragment' * A fragment identifier (or named anchor) to append to the link. * Do not include the '#' character. @@ -2063,7 +2063,7 @@ function url($path = NULL, array $options = array()) { $base = $options['absolute'] ? $options['base_url'] . '/' : base_path(); $prefix = empty($path) ? rtrim($options['prefix'], '/') : $options['prefix']; - $path = drupal_urlencode($prefix . $path); + $path = drupal_encode_path($prefix . $path); if (variable_get('clean_url', '0')) { // With Clean URLs. @@ -3168,11 +3168,13 @@ function drupal_json($var = NULL) { * characters are double escaped so PHP will still see the encoded version. * - With clean URLs, Apache changes '//' to '/', so every second slash is * double escaped. + * - This function should only be used on paths, not on query string arguments, + * otherwise unwanted double encoding will occur. * * @param $text * String to encode */ -function drupal_urlencode($text) { +function drupal_encode_path($text) { if (variable_get('clean_url', '0')) { return str_replace(array('%2F', '%26', '%23', '//'), array('/', '%2526', '%2523', '/%252F'), diff --git a/misc/autocomplete.js b/misc/autocomplete.js index 706a0cf2fed4d86b5b98324be45d381b10af6fa5..bed9b65ae5ae13c6cbd85dc78e3ef24620c25422 100644 --- a/misc/autocomplete.js +++ b/misc/autocomplete.js @@ -276,7 +276,7 @@ Drupal.ACDB.prototype.search = function (searchString) { // Ajax GET request for autocompletion. $.ajax({ type: 'GET', - url: db.uri + '/' + Drupal.encodeURIComponent(searchString), + url: db.uri + '/' + Drupal.encodePath(searchString), dataType: 'json', success: function (matches) { if (typeof matches.status == 'undefined' || matches.status != 0) { diff --git a/misc/drupal.js b/misc/drupal.js index e71adda757599381829e0af47e48274b69297a27..40a21b0f117abe36f944b819a6e1bf757d4ae57d 100644 --- a/misc/drupal.js +++ b/misc/drupal.js @@ -263,10 +263,11 @@ Drupal.unfreezeHeight = function () { }; /** - * Wrapper to address the mod_rewrite url encoding bug - * (equivalent of drupal_urlencode() in PHP). + * Wrapper around encodeURIComponent() which avoids Apache quirks (equivalent of + * drupal_encode_path() in PHP). This function should only be used on paths, not + * on query string arguments. */ -Drupal.encodeURIComponent = function (item, uri) { +Drupal.encodePath = function (item, uri) { uri = uri || location.href; item = encodeURIComponent(item).replace(/%2F/g, '/'); return (uri.indexOf('?q=') != -1) ? item : item.replace(/%26/g, '%2526').replace(/%23/g, '%2523').replace(/\/\//g, '/%252F'); diff --git a/modules/comment/comment.module b/modules/comment/comment.module index fc16604674dab39cc0f97515512ccded3c9daccf..775a2db254bfe245ef6ccf0b38d22a105ec85168 100644 --- a/modules/comment/comment.module +++ b/modules/comment/comment.module @@ -2030,10 +2030,10 @@ function theme_comment_post_forbidden($node) { // We cannot use drupal_get_destination() because these links // sometimes appear on /node and taxonomy listing pages. if (variable_get('comment_form_location_' . $node->type, COMMENT_FORM_BELOW) == COMMENT_FORM_SEPARATE_PAGE) { - $destination = 'destination=' . drupal_urlencode("comment/reply/$node->nid#comment-form"); + $destination = 'destination=' . rawurlencode("comment/reply/$node->nid#comment-form"); } else { - $destination = 'destination=' . drupal_urlencode("node/$node->nid#comment-form"); + $destination = 'destination=' . rawurlencode("node/$node->nid#comment-form"); } if (variable_get('user_register', 1)) { diff --git a/modules/search/search.test b/modules/search/search.test index 6788744c1636078803c6a9901536465f6e7c7e74..f39261d3bbfd5e588761275d067f4fda99021249 100644 --- a/modules/search/search.test +++ b/modules/search/search.test @@ -266,11 +266,11 @@ class SearchAdvancedSearchForm extends DrupalWebTestCase { $this->assertNotEqual($dummy_title, $this->node->title, t("Dummy title doens't equal node title")); // Search for the dummy title with a GET query. - $this->drupalGet('search/node/' . drupal_urlencode($dummy_title)); + $this->drupalGet('search/node/' . $dummy_title); $this->assertNoText($this->node->title, t('Page node is not found with dummy title.')); // Search for the title of the node with a GET query. - $this->drupalGet('search/node/' . drupal_urlencode($this->node->title)); + $this->drupalGet('search/node/' . $this->node->title); $this->assertText($this->node->title, t('Page node is found with GET query.')); // Search for the title of the node with a POST query. diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test index 7c0455952b1d5e31909629007140ed569c17c933..d4a07be73ee406abb963500ba11c4f44f9672b03 100644 --- a/modules/simpletest/tests/common.test +++ b/modules/simpletest/tests/common.test @@ -8,8 +8,8 @@ class CommonLUnitTest extends DrupalUnitTestCase { public static function getInfo() { return array( - 'name' => t('Tests for the l() function'), - 'description' => t('Confirm that url() works correctly with various input.'), + 'name' => t('URL generation tests'), + 'description' => t('Confirm that url(), drupal_query_string_encode(), and l() work correctly with various input.'), 'group' => t('System'), ); } @@ -22,8 +22,18 @@ class CommonLUnitTest extends DrupalUnitTestCase { $path = "<SCRIPT>alert('XSS')</SCRIPT>"; $link = l($text, $path); $sanitized_path = check_url(url($path)); - $this->assertTrue(strpos($link, $sanitized_path) != FALSE, t('XSS attack @path was filtered', array('@path' => $path))); + $this->assertTrue(strpos($link, $sanitized_path) !== FALSE, t('XSS attack @path was filtered', array('@path' => $path))); } + + /** + * Test drupal_query_string_encode(). + */ + function testDrupalQueryStringEncode() { + $this->assertEqual(drupal_query_string_encode(array('a' => ' &#//+%20@Ûž')), 'a=%20%26%23%2F%2F%2B%2520%40%DB%9E', t('Value was properly encoded.')); + $this->assertEqual(drupal_query_string_encode(array(' &#//+%20@Ûž' => 'a')), '%20%26%23%2F%2F%2B%2520%40%DB%9E=a', t('Key was properly encoded.')); + $this->assertEqual(drupal_query_string_encode(array('a' => '1', 'b' => '2', 'c' => '3'), array('b')), 'a=1&c=3', t('Value was properly excluded.')); + $this->assertEqual(drupal_query_string_encode(array('a' => array('b' => '2', 'c' => '3')), array('b', 'a[c]')), 'a[b]=2', t('Nested array was properly encoded.')); + } } class CommonSizeTestCase extends DrupalUnitTestCase { diff --git a/modules/system/system.js b/modules/system/system.js index 92925c3a7951059bbea17290d0a0a96328af9bf1..7bea58deeb592cfb0bebebf58e2783f03cde1999 100644 --- a/modules/system/system.js +++ b/modules/system/system.js @@ -92,7 +92,7 @@ Drupal.behaviors.dateTime = { // Attach keyup handler to custom format inputs. $('input.custom-format:not(.date-time-processed)', context).addClass('date-time-processed').keyup(function () { var input = $(this); - var url = settings.dateTime.lookup +(settings.dateTime.lookup.match(/\?q=/) ? '&format=' : '?format=') + Drupal.encodeURIComponent(input.val()); + var url = settings.dateTime.lookup + (settings.dateTime.lookup.match(/\?q=/) ? '&format=' : '?format=') + encodeURIComponent(input.val()); $.getJSON(url, function (data) { $('div.description span', input.parent()).html(data); }); diff --git a/modules/update/update.fetch.inc b/modules/update/update.fetch.inc index 67a4add81588a153b39b3775dacecff3ab24d1a3..82a0784440093ada4930219018cda381dece9629 100644 --- a/modules/update/update.fetch.inc +++ b/modules/update/update.fetch.inc @@ -114,10 +114,10 @@ function _update_build_fetch_url($project, $site_key = '') { if (!empty($site_key) && (strpos($project['project_type'], 'disabled') === FALSE)) { $url .= (strpos($url, '?') === TRUE) ? '&' : '?'; $url .= 'site_key='; - $url .= drupal_urlencode($site_key); + $url .= rawurlencode($site_key); if (!empty($project['info']['version'])) { $url .= '&version='; - $url .= drupal_urlencode($project['info']['version']); + $url .= rawurlencode($project['info']['version']); } } return $url;