Index: install.php =================================================================== RCS file: /cvs/drupal/drupal/install.php,v retrieving revision 1.210 diff -u -p -r1.210 install.php --- install.php 18 Sep 2009 00:12:45 -0000 1.210 +++ install.php 22 Sep 2009 20:32:09 -0000 @@ -639,7 +639,7 @@ function install_tasks_to_display($insta * @see install_full_redirect_url() */ function install_redirect_url($install_state) { - return 'install.php?' . drupal_query_string_encode($install_state['parameters']); + return 'install.php?' . drupal_http_build_query($install_state['parameters']); } /** Index: includes/common.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/common.inc,v retrieving revision 1.994 diff -u -p -r1.994 common.inc --- includes/common.inc 21 Sep 2009 07:56:07 -0000 1.994 +++ includes/common.inc 22 Sep 2009 22:32:38 -0000 @@ -317,36 +317,81 @@ function drupal_get_feeds($delimiter = " */ /** - * Parse an array into a valid urlencoded query string. + * Process a URL query parameter array to remove unwanted elements. * * @param $query - * The array to be processed e.g. $_GET. + * (optional) An array to be processed. Defaults to $_GET. * @param $exclude - * The array filled with keys to be excluded. Use parent[child] to exclude - * nested items. + * (optional) A list of $query array keys to remove. Use "parent[child]" to + * exclude nested items. Defaults to array('q'). * @param $parent - * Should not be passed, only used in recursive calls. + * Internal use only. Used to build the $query array key for nested items. + * * @return - * An urlencoded string which can be appended to/as the URL query string. + * An array containing query parameters, which can used for url(). */ -function drupal_query_string_encode($query, $exclude = array(), $parent = '') { - $params = array(); +function drupal_get_query_parameters(array $query = NULL, array $exclude = array('q'), $parent = '') { + // Set defaults, if none given. + if (!isset($query)) { + $query = $_GET; + } + // If $exclude is empty, there is nothing to filter. + if (empty($exclude)) { + return $query; + } + $params = array(); foreach ($query as $key => $value) { - $key = rawurlencode($key); - if ($parent) { - $key = $parent . '[' . $key . ']'; + $string_key = ($parent ? $parent . '[' . $key . ']' : $key); + if (in_array($string_key, $exclude)) { + continue; } - if (in_array($key, $exclude)) { - continue; + if (is_array($value)) { + $params[$key] = drupal_get_query_parameters($value, $exclude, $string_key); + } + else { + $params[$key] = $value; } + } + + return $params; +} +/** + * Parse an array into a valid, rawurlencoded query string. + * + * This differs from http_build_query() as we need to rawurlencode() (instead of + * urlencode()) all query parameters. + * + * @param $query + * The array to be processed e.g. $_GET. + * @param $parent + * Internal use only. Used to build the $query array key for nested items. + * + * @return + * A rawurlencoded string which can be as or appended to the URL query string. + * + * @see drupal_get_query_parameters() + */ +function drupal_http_build_query(array $query, $parent = '') { + $params = array(); + + foreach ($query as $key => $value) { + $key = ($parent ? $parent . '[' . rawurlencode($key) . ']' : rawurlencode($key)); + + // Recurse into children. if (is_array($value)) { - $params[] = drupal_query_string_encode($value, $exclude, $key); + $params[] = drupal_http_build_query($value, $key); + } + // If a query parameter value is NULL, only append its key. + elseif (is_null($value)) { + $params[] = $key; } else { - $params[] = $key . '=' . rawurlencode($value); + // For better readability of paths in query strings, we decode slashes. + // @see drupal_encode_path() + $params[] = $key . '=' . str_replace('%2F', '/', rawurlencode($value)); } } @@ -354,7 +399,7 @@ function drupal_query_string_encode($que } /** - * Prepare a destination query string for use in combination with drupal_goto(). + * Prepare a 'destination' URL query parameter for use in combination with drupal_goto(). * * Used to direct the user back to the referring page after completing a form. * By default the current URL is returned. If a destination exists in the @@ -365,16 +410,119 @@ function drupal_query_string_encode($que */ function drupal_get_destination() { if (isset($_GET['destination'])) { - return 'destination=' . urlencode($_GET['destination']); + return array('destination' => $_GET['destination']); } else { // Use $_GET here to retrieve the original path in source form. $path = isset($_GET['q']) ? $_GET['q'] : ''; - $query = drupal_query_string_encode($_GET, array('q')); + $query = drupal_http_build_query(drupal_get_query_parameters()); if ($query != '') { $path .= '?' . $query; } - return 'destination=' . urlencode($path); + return array('destination' => $path); + } +} + +/** + * Wrapper around parse_url() to parse a given URL into an associative array, suitable for url(). + * + * The returned array contains a 'path' that may be passed separately to URL. + * For example: + * @code + * $options = drupal_parse_url($_GET['destination']); + * $my_url = url($options['path'], $options); + * $my_link = l('Example link', $options['path'], $options); + * @endcode + * + * This is required, because url() does not support relative URLs containing a + * query string or fragment in its $path argument. Instead, any query string + * needs to be parsed into an associative query parameter array in + * $options['query'] and the fragment into $options['fragment']. + * + * @param $url + * The URL string to parse, f.e. $_GET['destination']. + * + * @return + * An associative array containing the keys: + * - 'path': The path of the URL. If the given $url is absolute/external, this + * includes the scheme and host. + * - 'query': An array of query parameters of $url, if existent. + * - 'fragment': The fragment of $url, if existent. + * - 'absolute': Boolean whether the given $url is absolute. + * + * @see url() + * @see drupal_goto() + */ +function drupal_parse_url($url) { + $options = array( + 'path' => NULL, + 'query' => array(), + 'fragment' => '', + 'absolute' => FALSE, + ); + + // Absolute/external URLs. + if (strpos($url, '://') !== FALSE) { + $options['absolute'] = TRUE; + // Split off everything before the query string into 'path'. + // @todo Alternatively, we could skip this parsing completely for absolute + // URLs. + $parts = explode('?', $url); + $options['path'] = $parts[0]; + if (isset($parts[1])) { + $query_parts = explode('#', $parts); + parse_str($query_parts[0], $options['query']); + if (isset($query_parts[1])) { + $options['fragment'] = $query_parts[1]; + } + } + } + // Relative URLs. + else { + $parts = parse_url($url); + $options['path'] = $parts['path']; + if (isset($parts['query'])) { + parse_str($parts['query'], $options['query']); + } + if (isset($parts['fragment'])) { + $options['fragment'] = $parts['fragment']; + } + } + + return $options; +} + +/** + * Wrapper around rawurlencode() which avoids Apache quirks. + * + * Should be used when placing arbitrary data into the path component of an URL. + * + * Do not use this function to pass a path to url(). url() properly handles + * paths internally. + * This function should only be used on paths, not on query string arguments. + * Otherwise, unwanted double encoding will occur. + * + * Notes: + * - For esthetic reasons, we do not escape slashes. This also avoids a 'feature' + * in Apache where it 404s on any path containing '%2F'. + * - mod_rewrite unescapes %-encoded ampersands, hashes, and slashes when clean + * URLs are used, which are interpreted as delimiters by PHP. These + * 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. + * + * @param $path + * The URL path component to encode. + */ +function drupal_encode_path($path) { + if (!empty($GLOBALS['conf']['clean_url'])) { + return str_replace(array('%2F', '%26', '%23', '//'), + array('/', '%2526', '%2523', '/%252F'), + rawurlencode($path) + ); + } + else { + return str_replace('%2F', '/', rawurlencode($path)); } } @@ -417,9 +565,9 @@ function drupal_get_destination() { * supported. * @see drupal_get_destination() */ -function drupal_goto($path = '', $query = NULL, $fragment = NULL, $http_response_code = 302) { +function drupal_goto($path = '', array $query = array(), $fragment = NULL, $http_response_code = 302) { if (isset($_GET['destination'])) { - extract(parse_url(urldecode($_GET['destination']))); + extract(drupal_parse_url(urldecode($_GET['destination']))); } $args = array( @@ -2132,42 +2280,35 @@ function _format_date_callback(array $ma */ /** - * Generate a URL from a Drupal menu path. Will also pass-through existing URLs. + * Generate a URL. * * @param $path - * The Drupal path being linked to, such as "admin/content", or an - * existing URL like "http://drupal.org/". The special path - * '' may also be given and will generate the site's base URL. + * The Drupal path being linked to, such as "admin/content", or an existing + * URL like "http://drupal.org/". The special path '' may also be given + * and will generate the site's base URL. * @param $options * An associative array of additional options, with the following keys: - * - 'query' - * 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. - * - 'absolute' (default FALSE) - * Whether to force the output to be an absolute link (beginning with - * http:). Useful for links that will be displayed outside the site, such - * as in an RSS feed. - * - 'alias' (default FALSE) - * Whether the given path is an alias already. - * - 'external' - * Whether the given path is an external URL. - * - 'language' - * An optional language object. Used to build the URL to link to and - * look up the proper alias for the link. - * - 'https' - * Whether this URL should point to a secure location. If not specified, - * the current scheme is used, so the user stays on http or https - * respectively. TRUE enforces HTTPS and FALSE enforces HTTP, but HTTPS - * can only be enforced when the variable 'https' is set to TRUE. - * - 'base_url' - * Only used internally, to modify the base URL when a language dependent - * URL requires so. - * - 'prefix' - * Only used internally, to modify the path when a language dependent URL - * requires so. + * - 'query': An array of query key/value-pairs (without any URL-encoding) to + * append to the link. + * - 'fragment': A fragment identifier (or named anchor) to append to the + * link. Do not include the leading '#' character. + * - 'absolute': Defaults to FALSE. Whether to force the output to be an + * absolute link (beginning with http:). Useful for links that will be + * displayed outside the site, such as in a RSS feed. + * - 'alias': Defaults to FALSE. Whether the given path is a URL alias + * already. + * - 'external': Whether the given path is an external URL. + * - 'language': An optional language object. Used to build the URL to link to + * and look up the proper alias for the link. + * - 'https': Whether this URL should point to a secure location. If not + * specified, the current scheme is used, so the user stays on http or https + * respectively. TRUE enforces HTTPS and FALSE enforces HTTP, but HTTPS can + * only be enforced when the variable 'https' is set to TRUE. + * - 'base_url': Only used internally, to modify the base URL when a language + * dependent URL requires so. + * - 'prefix': Only used internally, to modify the path when a language + * dependent URL requires so. + * * @return * A string containing a URL to the given path. * @@ -2178,7 +2319,7 @@ function url($path = NULL, array $option // Merge in defaults. $options += array( 'fragment' => '', - 'query' => '', + 'query' => array(), 'absolute' => FALSE, 'alias' => FALSE, 'https' => FALSE, @@ -2199,21 +2340,19 @@ function url($path = NULL, array $option if ($options['fragment']) { $options['fragment'] = '#' . $options['fragment']; } - if (is_array($options['query'])) { - $options['query'] = drupal_query_string_encode($options['query']); - } if ($options['external']) { // Split off the fragment. if (strpos($path, '#') !== FALSE) { list($path, $old_fragment) = explode('#', $path, 2); + // If $options contains no fragment, take it over from the path. if (isset($old_fragment) && !$options['fragment']) { $options['fragment'] = '#' . $old_fragment; } } // Append the query. if ($options['query']) { - $path .= (strpos($path, '?') !== FALSE ? '&' : '?') . $options['query']; + $path .= (strpos($path, '?') !== FALSE ? '&' : '?') . drupal_http_build_query($options['query']); } // Reassemble. return $path . $options['fragment']; @@ -2264,28 +2403,31 @@ function url($path = NULL, array $option $base = $options['absolute'] ? $options['base_url'] . '/' : base_path(); $prefix = empty($path) ? rtrim($options['prefix'], '/') : $options['prefix']; - $path = drupal_encode_path($prefix . $path); - if (variable_get('clean_url', '0')) { - // With Clean URLs. + // With Clean URLs. + if (!empty($GLOBALS['conf']['clean_url'])) { + $path = drupal_encode_path($prefix . $path); if ($options['query']) { - return $base . $path . '?' . $options['query'] . $options['fragment']; + return $base . $path . '?' . drupal_http_build_query($options['query']) . $options['fragment']; } else { return $base . $path . $options['fragment']; } } + // Without Clean URLs. else { - // Without Clean URLs. - $variables = array(); + $path = $prefix . $path; + $query = array(); if (!empty($path)) { - $variables[] = 'q=' . $path; + $query['q'] = $path; } - if (!empty($options['query'])) { - $variables[] = $options['query']; + if ($options['query']) { + // We do not use array_merge() here to prevent overriding $path via query + // parameters. + $query += $options['query']; } - if ($query = join('&', $variables)) { - return $base . $script . '?' . $query . $options['fragment']; + if ($query) { + return $base . $script . '?' . drupal_http_build_query($query) . $options['fragment']; } else { return $base . $options['fragment']; @@ -3609,38 +3751,6 @@ function drupal_json_output($var = NULL) } /** - * Wrapper around urlencode() which avoids Apache quirks. - * - * Should be used when placing arbitrary data in an URL. Note that Drupal paths - * are urlencoded() when passed through url() and do not require urlencoding() - * of individual components. - * - * Notes: - * - For esthetic reasons, we do not escape slashes. This also avoids a 'feature' - * in Apache where it 404s on any path containing '%2F'. - * - mod_rewrite unescapes %-encoded ampersands, hashes, and slashes when clean - * URLs are used, which are interpreted as delimiters by PHP. These - * 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_encode_path($text) { - if (variable_get('clean_url', '0')) { - return str_replace(array('%2F', '%26', '%23', '//'), - array('/', '%2526', '%2523', '/%252F'), - rawurlencode($text)); - } - else { - return str_replace('%2F', '/', rawurlencode($text)); - } -} - -/** * Returns a string of highly randomized bytes (over the full 8-bit range). * * This function is better than simply calling mt_rand() or any other built-in Index: includes/form.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/form.inc,v retrieving revision 1.375 diff -u -p -r1.375 form.inc --- includes/form.inc 21 Sep 2009 06:44:13 -0000 1.375 +++ includes/form.inc 22 Sep 2009 18:10:42 -0000 @@ -2984,7 +2984,7 @@ function batch_process($redirect = NULL, // Set the batch number in the session to guarantee that it will stay alive. $_SESSION['batches'][$batch['id']] = TRUE; - drupal_goto($batch['url'], 'op=start&id=' . $batch['id']); + drupal_goto($batch['url'], array('op' => 'start', 'id' => $batch['id'])); } else { // Non-progressive execution: bypass the whole progressbar workflow Index: includes/pager.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/pager.inc,v retrieving revision 1.72 diff -u -p -r1.72 pager.inc --- includes/pager.inc 18 Sep 2009 00:04:21 -0000 1.72 +++ includes/pager.inc 22 Sep 2009 20:48:46 -0000 @@ -171,18 +171,18 @@ class PagerDefault extends SelectQueryEx } /** - * Compose a query string to append to pager requests. + * Compose a URL query parameter array for pager links. * * @return - * A query string that consists of all components of the current page request - * except for those pertaining to paging. + * A URL query parameter array that consists of all components of the current + * page request except for those pertaining to paging. */ -function pager_get_querystring() { - static $string = NULL; - if (!isset($string)) { - $string = drupal_query_string_encode($_REQUEST, array_merge(array('q', 'page'), array_keys($_COOKIE))); +function pager_get_query_parameters() { + $query = &drupal_static(__FUNCTION__); + if (!isset($query)) { + $query = drupal_get_query_parameters($_REQUEST, array_merge(array('q', 'page'), array_keys($_COOKIE))); } - return $string; + return $query; } /** @@ -465,11 +465,10 @@ function theme_pager_link($text, $page_n $query = array(); if (count($parameters)) { - $query[] = drupal_query_string_encode($parameters, array()); + $query = drupal_get_query_parameters($parameters, array()); } - $querystring = pager_get_querystring(); - if ($querystring != '') { - $query[] = $querystring; + if ($query_pager = pager_get_query_parameters()) { + $query = array_merge($query, $query_pager); } // Set each pager link title @@ -491,7 +490,7 @@ function theme_pager_link($text, $page_n } } - return l($text, $_GET['q'], array('attributes' => $attributes, 'query' => count($query) ? implode('&', $query) : NULL)); + return l($text, $_GET['q'], array('attributes' => $attributes, 'query' => $query)); } /** Index: includes/tablesort.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/tablesort.inc,v retrieving revision 1.54 diff -u -p -r1.54 tablesort.inc --- includes/tablesort.inc 18 Sep 2009 00:04:21 -0000 1.54 +++ includes/tablesort.inc 22 Sep 2009 20:53:40 -0000 @@ -59,7 +59,7 @@ class TableSort extends SelectQueryExten protected function init() { $ts = $this->order(); $ts['sort'] = $this->getSort(); - $ts['query_string'] = $this->getQueryString(); + $ts['query'] = $this->getQueryParameters(); return $ts; } @@ -87,14 +87,16 @@ class TableSort extends SelectQueryExten } /** - * Compose a query string to append to table sorting requests. + * Compose a URL query parameter array to append to table sorting requests. * * @return - * A query string that consists of all components of the current page request - * except for those pertaining to table sorting. + * A URL query parameter array that consists of all components of the current + * page request except for those pertaining to table sorting. + * + * @see tablesort_get_query_parameters() */ - protected function getQueryString() { - return drupal_query_string_encode($_REQUEST, array_merge(array('q', 'sort', 'order'), array_keys($_COOKIE))); + protected function getQueryParameters() { + return tablesort_get_query_parameters(); } /** @@ -141,7 +143,7 @@ class TableSort extends SelectQueryExten function tablesort_init($header) { $ts = tablesort_get_order($header); $ts['sort'] = tablesort_get_sort($header); - $ts['query_string'] = tablesort_get_querystring(); + $ts['query'] = tablesort_get_query_parameters(); return $ts; } @@ -174,11 +176,7 @@ function tablesort_header($cell, $header $ts['sort'] = 'asc'; $image = ''; } - - if (!empty($ts['query_string'])) { - $ts['query_string'] = '&' . $ts['query_string']; - } - $cell['data'] = l($cell['data'] . $image, $_GET['q'], array('attributes' => array('title' => $title), 'query' => 'sort=' . $ts['sort'] . '&order=' . urlencode($cell['data']) . $ts['query_string'], 'html' => TRUE)); + $cell['data'] = l($cell['data'] . $image, $_GET['q'], array('attributes' => array('title' => $title), 'query' => array_merge($ts['query'], array('sort' => $ts['sort'], 'order' => $cell['data'])), 'html' => TRUE)); unset($cell['field'], $cell['sort']); } @@ -214,14 +212,14 @@ function tablesort_cell($cell, $header, } /** - * Compose a query string to append to table sorting requests. + * Compose a URL query parameter array for table sorting links. * * @return - * A query string that consists of all components of the current page request - * except for those pertaining to table sorting. + * A URL query parameter array that consists of all components of the current + * page request except for those pertaining to table sorting. */ -function tablesort_get_querystring() { - return drupal_query_string_encode($_REQUEST, array_merge(array('q', 'sort', 'order'), array_keys($_COOKIE))); +function tablesort_get_query_parameters() { + return drupal_get_query_parameters($_REQUEST, array_merge(array('q', 'sort', 'order'), array_keys($_COOKIE))); } /** Index: modules/aggregator/aggregator.test =================================================================== RCS file: /cvs/drupal/drupal/modules/aggregator/aggregator.test,v retrieving revision 1.32 diff -u -p -r1.32 aggregator.test --- modules/aggregator/aggregator.test 18 Sep 2009 00:12:45 -0000 1.32 +++ modules/aggregator/aggregator.test 22 Sep 2009 18:10:42 -0000 @@ -56,7 +56,7 @@ class AggregatorTestCase extends DrupalW $feed_name = $this->randomName(10); if (!$feed_url) { $feed_url = url('rss.xml', array( - 'query' => 'feed=' . $feed_name, + 'query' => array('feed' => $feed_name), 'absolute' => TRUE, )); } Index: modules/book/book.module =================================================================== RCS file: /cvs/drupal/drupal/modules/book/book.module,v retrieving revision 1.515 diff -u -p -r1.515 book.module --- modules/book/book.module 22 Sep 2009 15:26:45 -0000 1.515 +++ modules/book/book.module 22 Sep 2009 18:10:42 -0000 @@ -73,7 +73,7 @@ function book_node_view_link($node, $bui $links['book_add_child'] = array( 'title' => t('Add child page'), 'href' => 'node/add/' . str_replace('_', '-', $child_type), - 'query' => 'parent=' . $node->book['mlid'], + 'query' => array('parent' => $node->book['mlid']), ); } Index: modules/comment/comment.module =================================================================== RCS file: /cvs/drupal/drupal/modules/comment/comment.module,v retrieving revision 1.774 diff -u -p -r1.774 comment.module --- modules/comment/comment.module 22 Sep 2009 07:36:57 -0000 1.774 +++ modules/comment/comment.module 22 Sep 2009 18:10:42 -0000 @@ -445,7 +445,7 @@ function comment_new_page_count($num_com } if ($pageno >= 1) { - $pagenum = "page=" . intval($pageno); + $pagenum = array('page' => intval($pageno)); } return $pagenum; @@ -2153,10 +2153,10 @@ function theme_comment_post_forbidden($n // 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=' . rawurlencode("comment/reply/$node->nid#comment-form"); + $destination = array('destination', "comment/reply/$node->nid#comment-form"); } else { - $destination = 'destination=' . rawurlencode("node/$node->nid#comment-form"); + $destination = array('destination', "node/$node->nid#comment-form"); } if (variable_get('user_register', 1)) { Index: modules/comment/comment.test =================================================================== RCS file: /cvs/drupal/drupal/modules/comment/comment.test,v retrieving revision 1.45 diff -u -p -r1.45 comment.test --- modules/comment/comment.test 22 Sep 2009 07:36:57 -0000 1.45 +++ modules/comment/comment.test 22 Sep 2009 18:10:42 -0000 @@ -308,7 +308,7 @@ class CommentInterfaceTest extends Comme $this->setCommentsPerPage(2); $comment_new_page = $this->postComment($this->node, $this->randomName(), $this->randomName(), TRUE); $this->assertTrue($this->commentExists($comment_new_page), t('Page one exists. %s')); - $this->drupalGet('node/' . $this->node->nid, array('query' => 'page=1')); + $this->drupalGet('node/' . $this->node->nid, array('query' => array('page' => 1))); $this->assertTrue($this->commentExists($reply, TRUE), t('Page two exists. %s')); $this->setCommentsPerPage(50); @@ -588,13 +588,13 @@ class CommentPagerTest extends CommentHe $this->assertFalse($this->commentExists($comments[2]), t('Comment 3 does not appear on page 1.')); // Check the second page. - $this->drupalGet('node/' . $node->nid, array('query' => 'page=1')); + $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 1))); $this->assertTrue($this->commentExists($comments[1]), t('Comment 2 appears on page 2.')); $this->assertFalse($this->commentExists($comments[0]), t('Comment 1 does not appear on page 2.')); $this->assertFalse($this->commentExists($comments[2]), t('Comment 3 does not appear on page 2.')); // Check the third page. - $this->drupalGet('node/' . $node->nid, array('query' => 'page=2')); + $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 2))); $this->assertTrue($this->commentExists($comments[2]), t('Comment 3 appears on page 3.')); $this->assertFalse($this->commentExists($comments[0]), t('Comment 1 does not appear on page 3.')); $this->assertFalse($this->commentExists($comments[1]), t('Comment 2 does not appear on page 3.')); @@ -608,21 +608,21 @@ class CommentPagerTest extends CommentHe $this->setCommentsPerPage(2); // We are still in flat view - the replies should not be on the first page, // even though they are replies to the oldest comment. - $this->drupalGet('node/' . $node->nid, array('query' => 'page=0')); + $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 0))); $this->assertFalse($this->commentExists($reply, TRUE), t('In flat mode, reply does not appear on page 1.')); // If we switch to threaded mode, the replies on the oldest comment // should be bumped to the first page and comment 6 should be bumped // to the second page. $this->setCommentSettings('comment_default_mode', COMMENT_MODE_THREADED, t('Switched to threaded mode.')); - $this->drupalGet('node/' . $node->nid, array('query' => 'page=0')); + $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 0))); $this->assertTrue($this->commentExists($reply, TRUE), t('In threaded mode, reply appears on page 1.')); $this->assertFalse($this->commentExists($comments[1]), t('In threaded mode, comment 2 has been bumped off of page 1.')); // If (# replies > # comments per page) in threaded expanded view, // the overage should be bumped. $reply2 = $this->postComment(NULL, $this->randomName(), $this->randomName(), FALSE, TRUE); - $this->drupalGet('node/' . $node->nid, array('query' => 'page=0')); + $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 0))); $this->assertFalse($this->commentExists($reply2, TRUE), t('In threaded mode where # replies > # comments per page, the newest reply does not appear on page 1.')); $this->drupalLogout(); Index: modules/contact/contact.pages.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/contact/contact.pages.inc,v retrieving revision 1.24 diff -u -p -r1.24 contact.pages.inc --- modules/contact/contact.pages.inc 18 Sep 2009 00:12:46 -0000 1.24 +++ modules/contact/contact.pages.inc 22 Sep 2009 18:10:42 -0000 @@ -155,7 +155,7 @@ function contact_personal_page($account) global $user; if (!valid_email_address($user->mail)) { - $output = t('You need to provide a valid e-mail address to contact other users. Please update your user information and try again.', array('@url' => url("user/$user->uid/edit", array('query' => 'destination=' . drupal_get_destination())))); + $output = t('You need to provide a valid e-mail address to contact other users. Please update your user information and try again.', array('@url' => url("user/$user->uid/edit", array('query' => drupal_get_destination())))); } elseif (!flood_is_allowed('contact', variable_get('contact_hourly_threshold', 3)) && !user_access('administer site-wide contact form')) { $output = t("You cannot send more than %number messages per hour. Please try again later.", array('%number' => variable_get('contact_hourly_threshold', 3))); Index: modules/field_ui/field_ui.admin.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/field_ui/field_ui.admin.inc,v retrieving revision 1.16 diff -u -p -r1.16 field_ui.admin.inc --- modules/field_ui/field_ui.admin.inc 21 Sep 2009 06:44:13 -0000 1.16 +++ modules/field_ui/field_ui.admin.inc 22 Sep 2009 18:10:42 -0000 @@ -539,7 +539,8 @@ function field_ui_field_overview_form_su } if ($destinations) { - $destinations[] = urldecode(substr(drupal_get_destination(), 12)); + $destination = drupal_get_destination(); + $destinations[] = $destination['destination']; unset($_GET['destination']); $form_state['redirect'] = field_ui_get_destinations($destinations); } Index: modules/node/node.pages.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/node/node.pages.inc,v retrieving revision 1.81 diff -u -p -r1.81 node.pages.inc --- modules/node/node.pages.inc 21 Sep 2009 06:44:13 -0000 1.81 +++ modules/node/node.pages.inc 22 Sep 2009 18:10:42 -0000 @@ -300,7 +300,7 @@ function node_form($form, &$form_state, * Button submit function: handle the 'Delete' button on the node form. */ function node_form_delete_submit($form, &$form_state) { - $destination = ''; + $destination = array(); if (isset($_GET['destination'])) { $destination = drupal_get_destination(); unset($_GET['destination']); Index: modules/openid/tests/openid_test.module =================================================================== RCS file: /cvs/drupal/drupal/modules/openid/tests/openid_test.module,v retrieving revision 1.3 diff -u -p -r1.3 openid_test.module --- modules/openid/tests/openid_test.module 10 Jun 2009 20:13:20 -0000 1.3 +++ modules/openid/tests/openid_test.module 22 Sep 2009 18:10:42 -0000 @@ -229,5 +229,5 @@ function _openid_test_endpoint_authentic // Put the signed message into the query string of a URL supplied by the // Relying Party, and redirect the user. drupal_set_header('Content-Type', 'text/plain'); - header('Location: ' . url($_REQUEST['openid_return_to'], array('query' => http_build_query($response, '', '&'), 'external' => TRUE))); + header('Location: ' . url($_REQUEST['openid_return_to'], array('query' => $response, 'external' => TRUE))); } Index: modules/search/search.test =================================================================== RCS file: /cvs/drupal/drupal/modules/search/search.test,v retrieving revision 1.36 diff -u -p -r1.36 search.test --- modules/search/search.test 21 Sep 2009 06:36:54 -0000 1.36 +++ modules/search/search.test 22 Sep 2009 18:10:42 -0000 @@ -499,7 +499,7 @@ class SearchCommentTestCase extends Drup // Invoke search index update. $this->drupalLogout(); - $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . variable_get('cron_key', 'drupal'))); + $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => variable_get('cron_key', 'drupal')))); // Search for $title. $edit = array( @@ -521,7 +521,7 @@ class SearchCommentTestCase extends Drup // Invoke search index update. $this->drupalLogout(); - $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . variable_get('cron_key', 'drupal'))); + $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => variable_get('cron_key', 'drupal')))); // Search for $title. $this->drupalPost('', $edit, t('Search')); Index: modules/simpletest/drupal_web_test_case.php =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/drupal_web_test_case.php,v retrieving revision 1.152 diff -u -p -r1.152 drupal_web_test_case.php --- modules/simpletest/drupal_web_test_case.php 20 Sep 2009 07:32:18 -0000 1.152 +++ modules/simpletest/drupal_web_test_case.php 22 Sep 2009 18:10:42 -0000 @@ -1003,7 +1003,7 @@ class DrupalWebTestCase extends DrupalTe // Make a request to the logout page, and redirect to the user page, the // idea being if you were properly logged out you should be seeing a login // screen. - $this->drupalGet('user/logout', array('query' => 'destination=user')); + $this->drupalGet('user/logout', array('query' => array('destination' => 'user'))); $pass = $this->assertField('name', t('Username field found.'), t('Logout')); $pass = $pass && $this->assertField('pass', t('Password field found.'), t('Logout')); @@ -1777,7 +1777,7 @@ class DrupalWebTestCase extends DrupalTe $this->assertTrue(isset($urls[$index]), t('Clicked link %label (@url_target) from @url_before', array('%label' => $label, '@url_target' => $url_target, '@url_before' => $url_before)), t('Browser')); - if (isset($urls[$index])) { + if (isset($url_target)) { return $this->drupalGet($url_target); } return FALSE; @@ -1791,12 +1791,16 @@ class DrupalWebTestCase extends DrupalTe * query, too. Can even be an absolute path which is just passed through. * @return * An absolute path. + * + * @todo What is the intention of this function? It is only invoked from + * locations, where URLs from the *output* are turned into absolute URLs, + * so why do we pass that through url() again? */ protected function getAbsoluteUrl($path) { - $options = array('absolute' => TRUE); $parts = parse_url($path); - // This is more crude than the menu_is_external but enough here. + // This is more crude than menu_path_is_external() but enough here. if (empty($parts['host'])) { + $options = array('absolute' => TRUE); $path = $parts['path']; $base_path = base_path(); $n = strlen($base_path); @@ -1804,7 +1808,19 @@ class DrupalWebTestCase extends DrupalTe $path = substr($path, $n); } if (isset($parts['query'])) { - $options['query'] = $parts['query']; + parse_str($parts['query'], $options['query']); + // Let's make it a bit more crude. It's not clear why we invoke url() on + // a URL contained in the returned page output again, but since $path is + // FALSE (see $path = $parts['path'] above) with Clean URLs disabled, + // and url() now encodes the passed in query parameter array, we get a + // double-encoded 'q' query string in the resulting absolute URL + // generated by url(). This cannot be avoided in url(). But it could be + // completely avoided if this function wouldn't be calling url(). + // @see SimpleTestURLTestCase->testGetAbsoluteUrl() + if (isset($options['query']['q'])) { + $path = $options['query']['q']; + unset($options['query']['q']); + } } $path = url($path, $options); } @@ -2497,7 +2513,8 @@ class DrupalWebTestCase extends DrupalTe */ protected function verbose($message) { if ($id = simpletest_verbose($message)) { - $this->pass(l(t('Verbose message'), $this->originalFileDirectory . '/simpletest/verbose/' . get_class($this) . '-' . $id . '.html', array('attributes' => array('target' => '_blank'))), 'Debug'); + $url = file_create_url($this->originalFileDirectory . '/simpletest/verbose/' . get_class($this) . '-' . $id . '.html'); + $this->pass(l(t('Verbose message'), $url, array('attributes' => array('target' => '_blank'))), 'Debug'); } } Index: modules/simpletest/simpletest.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/simpletest.test,v retrieving revision 1.31 diff -u -p -r1.31 simpletest.test --- modules/simpletest/simpletest.test 1 Sep 2009 17:40:27 -0000 1.31 +++ modules/simpletest/simpletest.test 22 Sep 2009 18:10:42 -0000 @@ -270,6 +270,43 @@ class SimpleTestFunctionalTest extends D } } +/** + * Test internal testing framework URL handling. + */ +class SimpleTestURLTestCase extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => 'SimpleTest URL handling', + 'description' => 'Test the URL handling in the testing framework.', + 'group' => 'SimpleTest', + ); + } + + /** + * Test DrupalWebTestCase::getAbsoluteUrl(). + */ + function testGetAbsoluteUrl() { + // Testbed runs with Clean URLs disabled, so disable it here. + $GLOBALS['conf']['clean_url'] = 0; + $url = 'user/login'; + + $this->drupalGet($url); + $absolute = url($url, array('absolute' => TRUE)); + $this->assertEqual($absolute, $this->url, t('Passed and requested URL are equal.')); + $this->assertEqual($this->url, $this->getAbsoluteUrl($url), t('Requested and returned absolute URL are equal.')); + + $this->drupalPost(NULL, array(), t('Log in')); + $this->assertEqual($absolute, $this->url, t('Passed and requested URL are equal.')); + $this->assertEqual($this->url, $this->getAbsoluteUrl($url), t('Requested and returned absolute URL are equal.')); + + $this->clickLink('Create new account'); + $url = 'user/register'; + $absolute = url($url, array('absolute' => TRUE)); + $this->assertEqual($absolute, $this->url, t('Passed and requested URL are equal.')); + $this->assertEqual($this->url, $this->getAbsoluteUrl($url), t('Requested and returned absolute URL are equal.')); + } +} + class SimpleTestMailCaptureTestCase extends DrupalWebTestCase { /** * Implement getInfo(). Index: modules/simpletest/tests/browser_test.module =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/browser_test.module,v retrieving revision 1.2 diff -u -p -r1.2 browser_test.module --- modules/simpletest/tests/browser_test.module 18 Sep 2009 00:12:47 -0000 1.2 +++ modules/simpletest/tests/browser_test.module 22 Sep 2009 18:10:42 -0000 @@ -58,7 +58,7 @@ function browser_test_print_post_form_su function browser_test_refresh_meta() { if (!isset($_GET['refresh'])) { - $url = url('browser_test/refresh/meta', array('absolute' => TRUE, 'query' => 'refresh=true')); + $url = url('browser_test/refresh/meta', array('absolute' => TRUE, 'query' => array('refresh' => 'true'))); drupal_add_html_head(''); return ''; } @@ -68,7 +68,7 @@ function browser_test_refresh_meta() { function browser_test_refresh_header() { if (!isset($_GET['refresh'])) { - $url = url('browser_test/refresh/header', array('absolute' => TRUE, 'query' => 'refresh=true')); + $url = url('browser_test/refresh/header', array('absolute' => TRUE, 'query' => array('refresh' => 'true'))); drupal_set_header('Location', $url); return ''; } Index: modules/simpletest/tests/common.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/common.test,v retrieving revision 1.74 diff -u -p -r1.74 common.test --- modules/simpletest/tests/common.test 20 Sep 2009 17:40:41 -0000 1.74 +++ modules/simpletest/tests/common.test 22 Sep 2009 22:31:25 -0000 @@ -2,14 +2,13 @@ // $Id: common.test,v 1.74 2009/09/20 17:40:41 dries Exp $ /** - * Tests for the l() function. + * Tests for URL generation functions. */ -class CommonLUnitTest extends DrupalUnitTestCase { - +class CommonURLUnitTest extends DrupalUnitTestCase { public static function getInfo() { return array( 'name' => 'URL generation tests', - 'description' => 'Confirm that url(), drupal_query_string_encode(), and l() work correctly with various input.', + 'description' => 'Confirm that url(), drupal_get_query_parameters(), drupal_http_build_query(), and l() work correctly with various input.', 'group' => 'System', ); } @@ -26,48 +25,164 @@ class CommonLUnitTest extends DrupalUnit } /** - * 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.')); + * Test drupal_get_query_parameters(). + */ + function testDrupalGetQueryParameters() { + $original = array( + 'a' => 1, + 'b' => array( + 'd' => 4, + 'e' => array( + 'f' => 5, + ), + ), + 'c' => 3, + 'q' => 'foo/bar', + ); + + // Default arguments. + $result = $_GET; + unset($result['q']); + $this->assertEqual(drupal_get_query_parameters(), $result, t("\$_GET['q'] was removed.")); + + // Default exclusion. + $result = $original; + unset($result['q']); + $this->assertEqual(drupal_get_query_parameters($original), $result, t("'q' was removed.")); + + // First-level exclusion. + $result = $original; + unset($result['b']); + $this->assertEqual(drupal_get_query_parameters($original, array('b')), $result, t("'b' was removed.")); + + // Second-level exclusion. + $result = $original; + unset($result['b']['d']); + $this->assertEqual(drupal_get_query_parameters($original, array('b[d]')), $result, t("'b[d]' was removed.")); + + // Third-level exclusion. + $result = $original; + unset($result['b']['e']['f']); + $this->assertEqual(drupal_get_query_parameters($original, array('b[e][f]')), $result, t("'b[e][f]' was removed.")); + + // Multiple exclusions. + $result = $original; + unset($result['a'], $result['b']['e'], $result['c']); + $this->assertEqual(drupal_get_query_parameters($original, array('a', 'b[e]', 'c')), $result, t("'a', 'b[e]', 'c' were removed.")); + } + + /** + * Test drupal_http_build_query(). + */ + function testDrupalHttpBuildQuery() { + $this->assertEqual(drupal_http_build_query(array('a' => ' &#//+%20@۞')), 'a=%20%26%23//%2B%2520%40%DB%9E', t('Value was properly encoded.')); + $this->assertEqual(drupal_http_build_query(array(' &#//+%20@۞' => 'a')), '%20%26%23%2F%2F%2B%2520%40%DB%9E=a', t('Key was properly encoded.')); + $this->assertEqual(drupal_http_build_query(array('a' => '1', 'b' => '2', 'c' => '3')), 'a=1&b=2&c=3', t('Multiple values were properly concatenated.')); + $this->assertEqual(drupal_http_build_query(array('a' => array('b' => '2', 'c' => '3'), 'd' => 'foo')), 'a[b]=2&a[c]=3&d=foo', t('Nested array was properly encoded.')); } - + /** * Test url() with/without query, with/without fragment, absolute on/off and * assert all that works when clean URLs are on and off. */ function testUrl() { global $base_url; - + foreach (array(FALSE, TRUE) as $absolute) { // Get the expected start of the path string. $base = $absolute ? $base_url . '/' : base_path(); $absolute_string = $absolute ? 'absolute' : NULL; - - // Run tests with clean urls disabled. + + // Disable Clean URLs. $GLOBALS['conf']['clean_url'] = 0; - $clean_urls = 'disabled'; - - $this->assertTrue(url('node', array('absolute' => $absolute)) == $base . '?q=node', t('Creation of @absolute internal url with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - $this->assertTrue(url('node', array('fragment' => 'foo', 'absolute' => $absolute)) == $base . '?q=node#foo', t('Creation of @absolute internal url with fragment option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - $this->assertTrue(url('node', array('query' => 'foo', 'absolute' => $absolute)) == $base . '?q=node&foo', t('Creation of @absolute internal url with query option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - $this->assertTrue(url('node', array('query' => 'foo', 'fragment' => 'bar', 'absolute' => $absolute)) == $base . '?q=node&foo#bar', t('Creation of @absolute internal url with query and fragment option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - $this->assertTrue(url('', array('absolute' => $absolute)) == $base, t('Creation of @absolute internal url using front with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - - // Run tests again with clean urls enabled. + + $url = $base . '?q=node/123'; + $result = url('node/123', array('absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base . '?q=node/123#foo'; + $result = url('node/123', array('fragment' => 'foo', 'absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base . '?q=node/123&foo'; + $result = url('node/123', array('query' => array('foo' => NULL), 'absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base . '?q=node/123&foo=bar&bar=baz'; + $result = url('node/123', array('query' => array('foo' => 'bar', 'bar' => 'baz'), 'absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base . '?q=node/123&foo#bar'; + $result = url('node/123', array('query' => array('foo' => NULL), 'fragment' => 'bar', 'absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base; + $result = url('', array('absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + // Enable Clean URLs. $GLOBALS['conf']['clean_url'] = 1; - $clean_urls = 'enabled'; - - $this->assertTrue(url('node', array('absolute' => $absolute)) == $base . 'node', t('Creation of @absolute internal url with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - $this->assertTrue(url('node', array('fragment' => 'foo', 'absolute' => $absolute)) == $base . 'node#foo', t('Creation of @absolute internal url with fragment option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - $this->assertTrue(url('node', array('query' => 'foo', 'absolute' => $absolute)) == $base . 'node?foo', t('Creation of @absolute internal url with query option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - $this->assertTrue(url('node', array('query' => 'foo', 'fragment' => 'bar', 'absolute' => $absolute)) == $base . 'node?foo#bar', t('Creation of @absolute internal url with query and fragment option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); - $this->assertTrue(url('', array('absolute' => $absolute)) == $base, t('Creation of @absolute internal url using front with clean urls @clean_urls', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls))); + + $url = $base . 'node/123'; + $result = url('node/123', array('absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base . 'node/123#foo'; + $result = url('node/123', array('fragment' => 'foo', 'absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base . 'node/123?foo'; + $result = url('node/123', array('query' => array('foo' => NULL), 'absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base . 'node/123?foo=bar&bar=baz'; + $result = url('node/123', array('query' => array('foo' => 'bar', 'bar' => 'baz'), 'absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base . 'node/123?foo#bar'; + $result = url('node/123', array('query' => array('foo' => NULL), 'fragment' => 'bar', 'absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); + + $url = $base; + $result = url('', array('absolute' => $absolute)); + $this->assertEqual($url, $result, "$url == $result"); } } + + /** + * Test external URL handling. + */ + function testExternalUrls() { + $test_url = 'http://drupal.org/'; + + // Verify external URL can contain a fragment. + $url = $test_url . '#drupal'; + $result = url($url); + $this->assertEqual($url, $result, t('External URL with fragment works without a fragment in $options.')); + + // Verify fragment can be overidden in an external URL. + $url = $test_url . '#drupal'; + $fragment = $this->randomName(10); + $result = url($url, array('fragment' => $fragment)); + $this->assertEqual($test_url . '#' . $fragment, $result, t('External URL fragment is overidden with a custom fragment in $options.')); + + // Verify external URL can contain a query string. + $url = $test_url . '?drupal=awesome'; + $result = url($url); + $this->assertEqual($url, $result, t('External URL with query string works without a query string in $options.')); + + // Verify external URL can be extended with a query string. + $url = $test_url; + $query = array($this->randomName(5) => $this->randomName(5)); + $result = url($url, array('query' => $query)); + $this->assertEqual($url . '?' . http_build_query($query), $result, t('External URL can be extended with a query string in $options.')); + + // Verify query string can be extended in an external URL. + $url = $test_url . '?drupal=awesome'; + $query = array($this->randomName(5) => $this->randomName(5)); + $result = url($url, array('query' => $query)); + $this->assertEqual($url . '&' . http_build_query($query), $result, t('External URL query string can be extended with a custom query string in $options.')); + } } class CommonSizeTestCase extends DrupalUnitTestCase { @@ -225,54 +340,6 @@ class DrupalTagsHandlingTestCase extends } /** - * Tests url(). - */ -class UrlTestCase extends DrupalWebtestCase { - - public static function getInfo() { - return array( - 'name' => 'Tests for the url() function', - 'description' => 'Performs tests on the url() function.', - 'group' => 'System', - ); - } - - /** - * Test the url() function's $options array. - * - * Assert that calling url() with an external URL - * 1. containing a fragment works with and without a fragment in $options. - * 2. containing or not containing a query works with a query in $options. - */ - function testUrlOptions() { - // Testing the fragment handling. - $fragment = $this->randomName(10); - $test_url = 'http://www.drupal.org/#' . $fragment; - - $result_url = url($test_url); - $this->assertEqual($test_url, $result_url, t("External URL containing a fragment works without a fragment in options. url('http://drupal.org/#frag1');")); - - $result_url = url($test_url, array('fragment' => $fragment)); - $this->assertEqual($test_url, $result_url, t("External URL containing a fragment works with a fragment in options. url('http://drupal.org/#frag1', array('fragment' => 'frag1'));")); - - // Testing the query handling. - $query = $this->randomName(10); - $query2 = $this->randomName(10); - $test_url = 'http://www.drupal.org/?' . $query; - - // The external URL contains a query. - $result_url = url($test_url, array('query' => $query2)); - $this->assertEqual($test_url . '&' . $query2, $result_url, t("External URL with a query passed in the path paramater. url('http://drupal.org/?param1', array('query' => 'param2'));")); - - // The external URL does not contain a query. - $test_url = 'http://www.drupal.org'; - $result_url = url($test_url, array('query' => $query2)); - $this->assertEqual($test_url . '?' . $query2, $result_url, t("External URL without a query passed in the path paramater. url('http://drupal.org', array('query' => 'param2'));")); - } - -} - -/** * Test the Drupal CSS system. */ class CascadingStylesheetsTestCase extends DrupalWebTestCase { @@ -563,9 +630,15 @@ class DrupalHTTPRequestTestCase extends function testDrupalGetDestination() { $query = $this->randomName(10); - $url = url('system-test/destination', array('absolute' => TRUE, 'query' => $query)); - $this->drupalGet($url); - $this->assertText($query, t('The query passed to the page is correctly represented by drupal_get_detination().')); + + // Verify that a 'destination' query string is used as destination. + $this->drupalGet('system-test/destination', array('query' => array('destination' => $query))); + $this->assertText('The destination: ' . $query, t('The given query string destination is determined as destination.')); + + // Verify that the current path is used as destination. + $this->drupalGet('system-test/destination', array('query' => array($query => NULL))); + $url = 'system-test/destination?' . $query; + $this->assertText('The destination: ' . $url, t('The current path is determined as destination.')); } } Index: modules/simpletest/tests/form.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/form.test,v retrieving revision 1.15 diff -u -p -r1.15 form.test --- modules/simpletest/tests/form.test 18 Sep 2009 00:12:48 -0000 1.15 +++ modules/simpletest/tests/form.test 22 Sep 2009 18:10:43 -0000 @@ -430,10 +430,10 @@ class FormsFormStorageTestCase extends D $user = $this->drupalCreateUser(array('access content')); $this->drupalLogin($user); - $this->drupalPost('form_test/form-storage', array('title' => 'new', 'value' => 'value_is_set'), 'Continue', array('query' => 'cache=1')); + $this->drupalPost('form_test/form-storage', array('title' => 'new', 'value' => 'value_is_set'), 'Continue', array('query' => array('cache' => 1))); $this->assertText('Form constructions: 1', t('The form has been constructed one time till now.')); - $this->drupalPost(NULL, array(), 'Save', array('query' => 'cache=1')); + $this->drupalPost(NULL, array(), 'Save', array('query' => array('cache' => 1))); $this->assertText('Form constructions: 2', t('The form has been constructed two times till now.')); $this->assertText('Title: new', t('The form storage has stored the values.')); } Index: modules/simpletest/tests/system_test.module =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/system_test.module,v retrieving revision 1.15 diff -u -p -r1.15 system_test.module --- modules/simpletest/tests/system_test.module 17 Aug 2009 20:32:30 -0000 1.15 +++ modules/simpletest/tests/system_test.module 22 Sep 2009 18:10:43 -0000 @@ -116,7 +116,8 @@ function system_test_redirect_invalid_sc } function system_test_destination() { - return 'The destination: ' . drupal_get_destination(); + $destination = drupal_get_destination(); + return 'The destination: ' . $destination['destination']; } /** Index: modules/statistics/statistics.admin.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/statistics/statistics.admin.inc,v retrieving revision 1.32 diff -u -p -r1.32 statistics.admin.inc --- modules/statistics/statistics.admin.inc 29 Aug 2009 03:32:46 -0000 1.32 +++ modules/statistics/statistics.admin.inc 22 Sep 2009 18:10:43 -0000 @@ -130,9 +130,9 @@ function statistics_top_visitors() { $result = $query->execute(); $rows = array(); + $destination = drupal_get_destination(); foreach ($result as $account) { - $qs = drupal_get_destination(); - $ban_link = $account->iid ? l(t('unblock IP address'), "admin/config/people/ip-blocking/delete/$account->iid", array('query' => $qs)) : l(t('block IP address'), "admin/config/people/ip-blocking/$account->hostname", array('query' => $qs)); + $ban_link = $account->iid ? l(t('unblock IP address'), "admin/config/people/ip-blocking/delete/$account->iid", array('query' => $destination)) : l(t('block IP address'), "admin/config/people/ip-blocking/$account->hostname", array('query' => $destination)); $rows[] = array($account->hits, ($account->uid ? theme('username', $account) : $account->hostname), format_interval(round($account->total / 1000)), (user_access('block IP addresses') && !$account->uid) ? $ban_link : ''); } Index: modules/system/system.install =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.install,v retrieving revision 1.389 diff -u -p -r1.389 system.install --- modules/system/system.install 20 Sep 2009 17:56:24 -0000 1.389 +++ modules/system/system.install 22 Sep 2009 18:10:43 -0000 @@ -172,7 +172,7 @@ function system_requirements($phase) { } $description .= ' ' . $t('You can run cron manually.', array('@cron' => url('admin/reports/status/run-cron'))); - $description .= '
' . $t('To run cron from outside the site, go to !cron', array('!cron' => url($base_url . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . variable_get('cron_key', 'drupal'))))); + $description .= '
' . $t('To run cron from outside the site, go to !cron', array('!cron' => url($base_url . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => variable_get('cron_key', 'drupal')))))); $requirements['cron'] = array( 'title' => $t('Cron maintenance tasks'), Index: modules/system/system.module =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.module,v retrieving revision 1.795 diff -u -p -r1.795 system.module --- modules/system/system.module 22 Sep 2009 15:26:46 -0000 1.795 +++ modules/system/system.module 22 Sep 2009 18:10:43 -0000 @@ -2334,7 +2334,7 @@ function system_admin_compact_mode() { function system_admin_compact_page($mode = 'off') { global $user; user_save($user, array('admin_compact_mode' => ($mode == 'on'))); - drupal_goto(drupal_get_destination()); + drupal_goto(); } /** Index: modules/system/system.test =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.test,v retrieving revision 1.77 diff -u -p -r1.77 system.test --- modules/system/system.test 21 Sep 2009 06:44:14 -0000 1.77 +++ modules/system/system.test 22 Sep 2009 18:10:43 -0000 @@ -379,12 +379,12 @@ class CronRunTestCase extends DrupalWebT // Run cron anonymously with a random cron key. $key = $this->randomName(16); - $this->drupalGet($base_url . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . $key)); + $this->drupalGet($base_url . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => $key))); $this->assertResponse(403); // Run cron anonymously with the valid cron key. $key = variable_get('cron_key', 'drupal'); - $this->drupalGet($base_url . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . $key)); + $this->drupalGet($base_url . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => $key))); $this->assertResponse(200); // Execute cron directly. Index: modules/translation/translation.pages.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/translation/translation.pages.inc,v retrieving revision 1.8 diff -u -p -r1.8 translation.pages.inc --- modules/translation/translation.pages.inc 29 Jul 2009 06:39:35 -0000 1.8 +++ modules/translation/translation.pages.inc 22 Sep 2009 18:10:43 -0000 @@ -47,7 +47,7 @@ function translation_node_overview($node // No such translation in the set yet: help user to create it. $title = t('n/a'); if (node_access('create', $node)) { - $options[] = l(t('add translation'), 'node/add/' . str_replace('_', '-', $node->type), array('query' => "translation=$node->nid&language=$language->language")); + $options[] = l(t('add translation'), 'node/add/' . str_replace('_', '-', $node->type), array('query' => array('translation' => $node->nid, 'language' => $language->language))); } $status = t('Not translated'); } Index: modules/user/user.module =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.module,v retrieving revision 1.1052 diff -u -p -r1.1052 user.module --- modules/user/user.module 22 Sep 2009 15:26:46 -0000 1.1052 +++ modules/user/user.module 22 Sep 2009 18:10:43 -0000 @@ -3135,9 +3135,16 @@ function user_modules_uninstalled($modul } /** - * Rewrite the destination to prevent redirecting to login page after login. + * Helper function to rewrite the destination to avoid redirecting to login page after login. + * + * Third-party authentication modules may use this function to determine the + * proper destination after a user has been properly logged in. */ function user_login_destination() { $destination = drupal_get_destination(); - return $destination == 'destination=user%2Flogin' ? 'destination=user' : $destination; + if ($destination['destination'] == 'user/login') { + $destination['destination'] = 'user'; + } + return $destination; } + Index: modules/user/user.pages.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.pages.inc,v retrieving revision 1.56 diff -u -p -r1.56 user.pages.inc --- modules/user/user.pages.inc 22 Sep 2009 07:50:16 -0000 1.56 +++ modules/user/user.pages.inc 22 Sep 2009 18:10:43 -0000 @@ -292,7 +292,7 @@ function user_profile_form_submit($form, * Submit function for the 'Cancel account' button on the user edit form. */ function user_edit_cancel_submit($form, &$form_state) { - $destination = ''; + $destination = array(); if (isset($_GET['destination'])) { $destination = drupal_get_destination(); unset($_GET['destination']);