Commit 233208 by dries
- Patch #310139 by pwolanin, c960657: drupal_query_string_encode() should not call drupal_urlencode().
--- includes/common.inc 2009/07/03 18:26:34 1.927
+++ includes/common.inc 2009/07/03 19:21:54 1.928
@@ -233,7 +233,7 @@
$params = array();
foreach ($query as $key => $value) {
- $key = drupal_urlencode($key);
+ $key = rawurlencode($key);
if ($parent) {
$key = $parent . '[' . $key . ']';
}
@@ -246,7 +246,7 @@
$params[] = drupal_query_string_encode($value, $exclude, $key);
}
else {
- $params[] = $key . '=' . drupal_urlencode($value);
+ $params[] = $key . '=' . rawurlencode($value);
}
}
@@ -1958,8 +1958,8 @@
* @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 @@
$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 @@
* 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'),
--- misc/autocomplete.js 2009/06/28 13:37:29 1.30
+++ misc/autocomplete.js 2009/07/03 19:21:54 1.31
@@ -276,7 +276,7 @@
// 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) {
--- misc/drupal.js 2009/05/03 07:35:37 1.55
+++ misc/drupal.js 2009/07/03 19:21:54 1.56
@@ -263,10 +263,11 @@
};
/**
- * 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');
--- modules/comment/comment.module 2009/07/03 04:35:30 1.736
+++ modules/comment/comment.module 2009/07/03 19:21:54 1.737
@@ -2030,10 +2030,10 @@
// 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)) {
--- modules/search/search.test 2009/06/12 08:39:38 1.23
+++ modules/search/search.test 2009/07/03 19:21:54 1.24
@@ -266,11 +266,11 @@
$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.
--- modules/simpletest/tests/common.test 2009/07/03 05:15:41 1.49
+++ modules/simpletest/tests/common.test 2009/07/03 19:21:54 1.50
@@ -8,8 +8,8 @@
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 @@
$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 {
--- modules/system/system.js 2009/06/20 08:02:29 1.27
+++ modules/system/system.js 2009/07/03 19:21:55 1.28
@@ -92,7 +92,7 @@
// 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);
});
--- modules/update/update.fetch.inc 2009/06/06 06:26:13 1.21
+++ modules/update/update.fetch.inc 2009/07/03 19:21:55 1.22
@@ -114,10 +114,10 @@
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;
+++ includes/common.inc 2009/07/03 19:21:54 1.928
@@ -233,7 +233,7 @@
$params = array();
foreach ($query as $key => $value) {
- $key = drupal_urlencode($key);
+ $key = rawurlencode($key);
if ($parent) {
$key = $parent . '[' . $key . ']';
}
@@ -246,7 +246,7 @@
$params[] = drupal_query_string_encode($value, $exclude, $key);
}
else {
- $params[] = $key . '=' . drupal_urlencode($value);
+ $params[] = $key . '=' . rawurlencode($value);
}
}
@@ -1958,8 +1958,8 @@
* @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 @@
$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 @@
* 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'),
--- misc/autocomplete.js 2009/06/28 13:37:29 1.30
+++ misc/autocomplete.js 2009/07/03 19:21:54 1.31
@@ -276,7 +276,7 @@
// 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) {
--- misc/drupal.js 2009/05/03 07:35:37 1.55
+++ misc/drupal.js 2009/07/03 19:21:54 1.56
@@ -263,10 +263,11 @@
};
/**
- * 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');
--- modules/comment/comment.module 2009/07/03 04:35:30 1.736
+++ modules/comment/comment.module 2009/07/03 19:21:54 1.737
@@ -2030,10 +2030,10 @@
// 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)) {
--- modules/search/search.test 2009/06/12 08:39:38 1.23
+++ modules/search/search.test 2009/07/03 19:21:54 1.24
@@ -266,11 +266,11 @@
$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.
--- modules/simpletest/tests/common.test 2009/07/03 05:15:41 1.49
+++ modules/simpletest/tests/common.test 2009/07/03 19:21:54 1.50
@@ -8,8 +8,8 @@
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 @@
$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 {
--- modules/system/system.js 2009/06/20 08:02:29 1.27
+++ modules/system/system.js 2009/07/03 19:21:55 1.28
@@ -92,7 +92,7 @@
// 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);
});
--- modules/update/update.fetch.inc 2009/06/06 06:26:13 1.21
+++ modules/update/update.fetch.inc 2009/07/03 19:21:55 1.22
@@ -114,10 +114,10 @@
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;