From c96ade84631a2ae24d7335b50a2c8c5b1f48b242 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Sat, 9 Nov 2013 23:08:56 -0800 Subject: [PATCH 01/16] [ticket/11508] Split parts of build_url() into reusable functions. PHPBB3-11508 --- phpBB/includes/functions.php | 106 +++++++++++++++++++++++++---------- 1 file changed, 75 insertions(+), 31 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 0cb88cd8ee..49b7293ed6 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2368,26 +2368,45 @@ function build_url($strip_vars = false) // Append SID $redirect = append_sid($page, false, false); - - // Add delimiter if not there... - if (strpos($redirect, '?') === false) - { - $redirect .= '?'; - } + $basic_parts = phpbb_get_url_parts($redirect, '&'); + $params = $basic_parts['params']; // Strip vars... - if ($strip_vars !== false && strpos($redirect, '?') !== false) + if ($strip_vars !== false) { if (!is_array($strip_vars)) { $strip_vars = array($strip_vars); } - $query = $_query = array(); + // Strip the vars off + foreach ($strip_vars as $strip) + { + if (isset($params[$strip])) + { + unset($params[$strip]); + } + } + } - $args = substr($redirect, strpos($redirect, '?') + 1); - $args = ($args) ? explode('&', $args) : array(); - $redirect = substr($redirect, 0, strpos($redirect, '?')); + return $basic_parts['base'] . (($params) ? '?' . phpbb_glue_url_params($params) : ''); +} + +/** +* Get the base and parameters of a URL +* +* @param string $url URL to break apart +* @param string $separator Parameter separator. Defaults to & +* @return array Returns the base and parameters in the form of array('base' => string, 'params' => array(name => value)) +*/ +function phpbb_get_url_parts($url, $separator = '&') { + $params = array(); + + if (strpos($url, '?') !== false) + { + $base = substr($url, 0, strpos($url, '?')); + $args = substr($url, strlen($base) + 1); + $args = ($args) ? explode($separator, $args) : array(); foreach ($args as $argument) { @@ -2400,29 +2419,54 @@ function build_url($strip_vars = false) continue; } - $query[$key] = implode('=', $arguments); + $params[$key] = implode('=', $arguments); } - - // Strip the vars off - foreach ($strip_vars as $strip) - { - if (isset($query[$strip])) - { - unset($query[$strip]); - } - } - - // Glue the remaining parts together... already urlencoded - foreach ($query as $key => $value) - { - $_query[] = $key . '=' . $value; - } - $query = implode('&', $_query); - - $redirect .= ($query) ? '?' . $query : ''; + } + else + { + $base = $url; } - return str_replace('&', '&', $redirect); + return array( + 'base' => $base, + 'params' => $params, + ); +} + +/** +* Glue URL parameters together +* +* @param array $params URL parameters in the form of array(name => value) +* @return array Returns the glued string. +*/ +function phpbb_glue_url_params($params) { + $_params = array(); + + foreach ($params as $key => $value) + { + $_params[] = $key . '=' . $value; + } + return implode('&', $_params); +} + +/** +* Append parameters to an already built URL. +* +* @param array $new_params Parameters to add in the form of array(name => value) +* @return string Returns the new URL. +*/ +function phpbb_append_url_params($url, $new_params) { + $url_parts = phpbb_get_url_parts($url); + $params = array_merge($url_parts['params'], $new_params); + + // Move the sid to the end if it's set + if (isset($params['sid'])) { + $sid = $params['sid']; + unset($params['sid']); + $params['sid'] = $sid; + } + + return $url_parts['base'] . '?' . phpbb_glue_url_params($params); } /** From ef4202d8590cdf3d88049329fe02864881369906 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Sat, 9 Nov 2013 23:18:10 -0800 Subject: [PATCH 02/16] [ticket/11508] Build the jumpbox hidden fields using the _form action_ PHPBB3-11508 --- phpBB/includes/functions.php | 5 ++--- phpBB/includes/functions_content.php | 8 +++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 49b7293ed6..aa09b90b35 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2437,7 +2437,7 @@ function phpbb_get_url_parts($url, $separator = '&') { * Glue URL parameters together * * @param array $params URL parameters in the form of array(name => value) -* @return array Returns the glued string. +* @return string Returns the glued string. */ function phpbb_glue_url_params($params) { $_params = array(); @@ -2452,6 +2452,7 @@ function phpbb_glue_url_params($params) { /** * Append parameters to an already built URL. * +* @param string $url URL to append parameters to * @param array $new_params Parameters to add in the form of array(name => value) * @return string Returns the new URL. */ @@ -4965,7 +4966,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id = } } - $hidden_fields_for_jumpbox = phpbb_build_hidden_fields_for_query_params($request, array('f')); $notification_mark_hash = generate_link_hash('mark_all_notifications_read'); // The following assigns all _common_ variables that may be used at any point in a template. @@ -4982,7 +4982,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id = 'LOGGED_IN_USER_LIST' => $online_userlist, 'RECORD_USERS' => $l_online_record, 'PRIVATE_MESSAGE_COUNT' => (!empty($user->data['user_unread_privmsg'])) ? $user->data['user_unread_privmsg'] : 0, - 'HIDDEN_FIELDS_FOR_JUMPBOX' => $hidden_fields_for_jumpbox, 'UNREAD_NOTIFICATIONS_COUNT' => ($notifications !== false) ? $notifications['unread_count'] : '', 'NOTIFICATIONS_COUNT' => ($notifications !== false) ? $notifications['unread_count'] : '', diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index b1f69c5756..387695a9e8 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -195,11 +195,13 @@ function make_jumpbox($action, $forum_id = false, $select_all = false, $acl_list } $db->sql_freeresult($result); unset($padding_store); + $url_parts = phpbb_get_url_parts($action); $template->assign_vars(array( - 'S_DISPLAY_JUMPBOX' => $display_jumpbox, - 'S_JUMPBOX_ACTION' => $action) - ); + 'S_DISPLAY_JUMPBOX' => $display_jumpbox, + 'S_JUMPBOX_ACTION' => $action, + 'HIDDEN_FIELDS_FOR_JUMPBOX' => build_hidden_fields($url_parts['params']), + )); return; } From e1c6b40ee82a79a58276d8e85de9c3593a6f3703 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Mon, 11 Nov 2013 15:45:02 -0800 Subject: [PATCH 03/16] [ticket/11508] Move the stripping param code to separate function as well. PHPBB3-11508 --- phpBB/includes/functions.php | 56 ++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index aa09b90b35..71716ae6b3 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2368,28 +2368,13 @@ function build_url($strip_vars = false) // Append SID $redirect = append_sid($page, false, false); - $basic_parts = phpbb_get_url_parts($redirect, '&'); - $params = $basic_parts['params']; - // Strip vars... if ($strip_vars !== false) { - if (!is_array($strip_vars)) - { - $strip_vars = array($strip_vars); - } - - // Strip the vars off - foreach ($strip_vars as $strip) - { - if (isset($params[$strip])) - { - unset($params[$strip]); - } - } + $redirect = phpbb_strip_url_params($redirect, $strip_vars, '&'); } - return $basic_parts['base'] . (($params) ? '?' . phpbb_glue_url_params($params) : ''); + return $redirect; } /** @@ -2449,15 +2434,48 @@ function phpbb_glue_url_params($params) { return implode('&', $_params); } +/** +* Strip parameters from an already built URL. +* +* @param string $url URL to strip parameters from +* @param array|string $strip Parameters to strip. +* @param string $separator Parameter separator. Defaults to & +* @return string Returns the new URL. +*/ +function phpbb_strip_url_params($url, $strip, $separator = '&') { + $url_parts = phpbb_get_url_parts($url, $separator); + $params = $url_parts['params']; + + if (!is_array($strip)) + { + $strip = array($strip); + } + + if (!empty($params)) + { + // Strip the parameters off + foreach ($strip as $param) + { + if (isset($params[$param])) + { + unset($params[$param]); + } + } + } + + return $url_parts['base'] . (($params) ? '?' . phpbb_glue_url_params($params) : ''); +} + /** * Append parameters to an already built URL. * * @param string $url URL to append parameters to * @param array $new_params Parameters to add in the form of array(name => value) +* @param string $separator Parameter separator. Defaults to & * @return string Returns the new URL. */ -function phpbb_append_url_params($url, $new_params) { - $url_parts = phpbb_get_url_parts($url); +function phpbb_append_url_params($url, $new_params, $separator = '&') { + $url_parts = phpbb_get_url_parts($url, $separator); $params = array_merge($url_parts['params'], $new_params); // Move the sid to the end if it's set From 3163388f63a6d0bac77ada86e2f1561f1d7f9714 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Fri, 6 Dec 2013 13:38:54 -0800 Subject: [PATCH 04/16] [ticket/11508] Move helper functions to path_helper class. PHPBB3-11508 --- phpBB/includes/functions.php | 123 ++------------------------- phpBB/includes/functions_content.php | 6 +- phpBB/phpbb/path_helper.php | 115 +++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 118 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 71716ae6b3..9c77aa935c 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2344,8 +2344,10 @@ function reapply_sid($url) */ function build_url($strip_vars = false) { - global $config, $user, $phpEx, $phpbb_root_path; + global $config, $user, $phpbb_container; + $path_helper = $phpbb_container->get('path_helper'); + $php_ext = $path_helper->get_php_ext(); $page = $user->page['page']; // We need to be cautious here. @@ -2358,12 +2360,12 @@ function build_url($strip_vars = false) if ($url_parts === false || empty($url_parts['scheme']) || empty($url_parts['host'])) { // Remove 'app.php/' from the page, when rewrite is enabled - if ($config['enable_mod_rewrite'] && strpos($page, 'app.' . $phpEx . '/') === 0) + if ($config['enable_mod_rewrite'] && strpos($page, 'app.' . $php_ext . '/') === 0) { - $page = substr($page, strlen('app.' . $phpEx . '/')); + $page = substr($page, strlen('app.' . $php_ext . '/')); } - $page = $phpbb_root_path . $page; + $page = $path_helper->get_phpbb_root_path() . $page; } // Append SID @@ -2371,123 +2373,12 @@ function build_url($strip_vars = false) if ($strip_vars !== false) { - $redirect = phpbb_strip_url_params($redirect, $strip_vars, '&'); + $redirect = $path_helper->strip_url_params($redirect, $strip_vars, '&'); } return $redirect; } -/** -* Get the base and parameters of a URL -* -* @param string $url URL to break apart -* @param string $separator Parameter separator. Defaults to & -* @return array Returns the base and parameters in the form of array('base' => string, 'params' => array(name => value)) -*/ -function phpbb_get_url_parts($url, $separator = '&') { - $params = array(); - - if (strpos($url, '?') !== false) - { - $base = substr($url, 0, strpos($url, '?')); - $args = substr($url, strlen($base) + 1); - $args = ($args) ? explode($separator, $args) : array(); - - foreach ($args as $argument) - { - $arguments = explode('=', $argument); - $key = $arguments[0]; - unset($arguments[0]); - - if ($key === '') - { - continue; - } - - $params[$key] = implode('=', $arguments); - } - } - else - { - $base = $url; - } - - return array( - 'base' => $base, - 'params' => $params, - ); -} - -/** -* Glue URL parameters together -* -* @param array $params URL parameters in the form of array(name => value) -* @return string Returns the glued string. -*/ -function phpbb_glue_url_params($params) { - $_params = array(); - - foreach ($params as $key => $value) - { - $_params[] = $key . '=' . $value; - } - return implode('&', $_params); -} - -/** -* Strip parameters from an already built URL. -* -* @param string $url URL to strip parameters from -* @param array|string $strip Parameters to strip. -* @param string $separator Parameter separator. Defaults to & -* @return string Returns the new URL. -*/ -function phpbb_strip_url_params($url, $strip, $separator = '&') { - $url_parts = phpbb_get_url_parts($url, $separator); - $params = $url_parts['params']; - - if (!is_array($strip)) - { - $strip = array($strip); - } - - if (!empty($params)) - { - // Strip the parameters off - foreach ($strip as $param) - { - if (isset($params[$param])) - { - unset($params[$param]); - } - } - } - - return $url_parts['base'] . (($params) ? '?' . phpbb_glue_url_params($params) : ''); -} - -/** -* Append parameters to an already built URL. -* -* @param string $url URL to append parameters to -* @param array $new_params Parameters to add in the form of array(name => value) -* @param string $separator Parameter separator. Defaults to & -* @return string Returns the new URL. -*/ -function phpbb_append_url_params($url, $new_params, $separator = '&') { - $url_parts = phpbb_get_url_parts($url, $separator); - $params = array_merge($url_parts['params'], $new_params); - - // Move the sid to the end if it's set - if (isset($params['sid'])) { - $sid = $params['sid']; - unset($params['sid']); - $params['sid'] = $sid; - } - - return $url_parts['base'] . '?' . phpbb_glue_url_params($params); -} - /** * Meta refresh assignment * Adds META template variable with meta http tag. diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index 387695a9e8..ff75a5b05d 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -110,7 +110,7 @@ function gen_sort_selects(&$limit_days, &$sort_by_text, &$sort_days, &$sort_key, */ function make_jumpbox($action, $forum_id = false, $select_all = false, $acl_list = false, $force_display = false) { - global $config, $auth, $template, $user, $db; + global $config, $auth, $template, $user, $db, $phpbb_container; // We only return if the jumpbox is not forced to be displayed (in case it is needed for functionality) if (!$config['load_jumpbox'] && $force_display === false) @@ -195,7 +195,9 @@ function make_jumpbox($action, $forum_id = false, $select_all = false, $acl_list } $db->sql_freeresult($result); unset($padding_store); - $url_parts = phpbb_get_url_parts($action); + + $path_helper = $phpbb_container->get('path_helper'); + $url_parts = $path_helper->get_url_parts($action); $template->assign_vars(array( 'S_DISPLAY_JUMPBOX' => $display_jumpbox, diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index fefef39c51..ff36ce5fc5 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -216,4 +216,119 @@ class path_helper return $scheme . $this->filesystem->clean_path($path); } + + /** + * Glue URL parameters together + * + * @param array $params URL parameters in the form of array(name => value) + * @return string Returns the glued string, e.g. name1=value1&name2=value2 + */ + public function glue_url_params($params) + { + $_params = array(); + + foreach ($params as $key => $value) + { + $_params[] = $key . '=' . $value; + } + return implode('&', $_params); + } + + /** + * Get the base and parameters of a URL + * + * @param string $url URL to break apart + * @param string $separator Parameter separator. Defaults to & + * @return array Returns the base and parameters in the form of array('base' => string, 'params' => array(name => value)) + */ + public function get_url_parts($url, $separator = '&') + { + $params = array(); + + if (strpos($url, '?') !== false) + { + $base = substr($url, 0, strpos($url, '?')); + $args = substr($url, strlen($base) + 1); + $args = ($args) ? explode($separator, $args) : array(); + + foreach ($args as $argument) + { + $arguments = explode('=', $argument); + $key = $arguments[0]; + unset($arguments[0]); + + if ($key === '') + { + continue; + } + + $params[$key] = implode('=', $arguments); + } + } + else + { + $base = $url; + } + + return array( + 'base' => $base, + 'params' => $params, + ); + } + + /** + * Strip parameters from an already built URL. + * + * @param string $url URL to strip parameters from + * @param array|string $strip Parameters to strip. + * @param string $separator Parameter separator. Defaults to & + * @return string Returns the new URL. + */ + public function strip_url_params($url, $strip, $separator = '&') + { + $url_parts = $this->get_url_parts($url, $separator); + $params = $url_parts['params']; + + if (!is_array($strip)) + { + $strip = array($strip); + } + + if (!empty($params)) + { + // Strip the parameters off + foreach ($strip as $param) + { + if (isset($params[$param])) + { + unset($params[$param]); + } + } + } + + return $url_parts['base'] . (($params) ? '?' . $this->glue_url_params($params) : ''); + } + + /** + * Append parameters to an already built URL. + * + * @param string $url URL to append parameters to + * @param array $new_params Parameters to add in the form of array(name => value) + * @param string $separator Parameter separator. Defaults to & + * @return string Returns the new URL. + */ + public function append_url_params($url, $new_params, $separator = '&') + { + $url_parts = $this->get_url_parts($url, $separator); + $params = array_merge($url_parts['params'], $new_params); + + // Move the sid to the end if it's set + if (isset($params['sid'])) { + $sid = $params['sid']; + unset($params['sid']); + $params['sid'] = $sid; + } + + return $url_parts['base'] . '?' . $this->glue_url_params($params); + } } From 8987fc95f97e2e64915fcbe809a7e14051b8a328 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Fri, 7 Feb 2014 09:20:49 -0800 Subject: [PATCH 05/16] [ticket/11508] Change separator parameter to a simple true|false $is_amp. PHPBB3-11508 --- phpBB/includes/functions.php | 2 +- phpBB/phpbb/path_helper.php | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 9c77aa935c..2062cd062c 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2373,7 +2373,7 @@ function build_url($strip_vars = false) if ($strip_vars !== false) { - $redirect = $path_helper->strip_url_params($redirect, $strip_vars, '&'); + $redirect = $path_helper->strip_url_params($redirect, $strip_vars, false); } return $redirect; diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index ff36ce5fc5..b4e7836640 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -238,11 +238,12 @@ class path_helper * Get the base and parameters of a URL * * @param string $url URL to break apart - * @param string $separator Parameter separator. Defaults to & + * @param bool $is_amp Is the parameter separator &. Defaults to true. * @return array Returns the base and parameters in the form of array('base' => string, 'params' => array(name => value)) */ - public function get_url_parts($url, $separator = '&') + public function get_url_parts($url, $is_amp = true) { + $separator = ($is_amp) ? '&' : '&'; $params = array(); if (strpos($url, '?') !== false) @@ -281,12 +282,12 @@ class path_helper * * @param string $url URL to strip parameters from * @param array|string $strip Parameters to strip. - * @param string $separator Parameter separator. Defaults to & + * @param bool $is_amp Is the parameter separator &. Defaults to true. * @return string Returns the new URL. */ - public function strip_url_params($url, $strip, $separator = '&') + public function strip_url_params($url, $strip, $is_amp = true) { - $url_parts = $this->get_url_parts($url, $separator); + $url_parts = $this->get_url_parts($url, $is_amp); $params = $url_parts['params']; if (!is_array($strip)) @@ -314,12 +315,12 @@ class path_helper * * @param string $url URL to append parameters to * @param array $new_params Parameters to add in the form of array(name => value) - * @param string $separator Parameter separator. Defaults to & + * @param string $is_amp Is the parameter separator &. Defaults to true. * @return string Returns the new URL. */ - public function append_url_params($url, $new_params, $separator = '&') + public function append_url_params($url, $new_params, $is_amp = true) { - $url_parts = $this->get_url_parts($url, $separator); + $url_parts = $this->get_url_parts($url, $is_amp); $params = array_merge($url_parts['params'], $new_params); // Move the sid to the end if it's set From f05e0ec9eb30bdfdc4cf4b0355c8da7c40d6a6f1 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Fri, 7 Feb 2014 09:56:17 -0800 Subject: [PATCH 06/16] [ticket/11508] Do not add the '?' unless there are parameters. PHPBB3-11508 --- phpBB/phpbb/path_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index b4e7836640..ac671fc400 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -330,6 +330,6 @@ class path_helper $params['sid'] = $sid; } - return $url_parts['base'] . '?' . $this->glue_url_params($params); + return $url_parts['base'] . (($params) ? '?' . $this->glue_url_params($params) : ''); } } From 47f8a6375f12d9ca18b88432a8d2e51d168909fe Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 27 Feb 2014 23:33:39 -0800 Subject: [PATCH 07/16] [ticket/11508] Add tests. PHPBB3-11508 --- tests/functions/build_url_test.php | 77 ++++++++++ ...oot_path_test.php => path_helper_test.php} | 138 +++++++++++++++++- 2 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 tests/functions/build_url_test.php rename tests/path_helper/{web_root_path_test.php => path_helper_test.php} (59%) diff --git a/tests/functions/build_url_test.php b/tests/functions/build_url_test.php new file mode 100644 index 0000000000..8d0aa76317 --- /dev/null +++ b/tests/functions/build_url_test.php @@ -0,0 +1,77 @@ +path_helper = new \phpbb\path_helper( + new \phpbb\symfony_request( + new phpbb_mock_request() + ), + new \phpbb\filesystem(), + $phpbb_root_path, + 'php' + ); + $phpbb_container->set('path_helper', $path_helper); + } + public function build_url_test_data() + { + return array( + array( + 'index.php', + false, + 'phpBB/index.php?', + ), + array( + 'index.php', + 't', + 'phpBB/index.php?', + ), + array( + 'viewtopic.php?f=2&style=1&t=6', + 'f', + 'phpBB/viewtopic.php?style=1&t=6', + ), + array( + 'viewtopic.php?f=2&style=1&t=6', + array('f', 'style', 't'), + 'phpBB/viewtopic.php', + ), + array( + 'http://test.phpbb.com/viewtopic.php?f=2&style=1&t=6', + array('f', 'style', 't'), + 'http://test.phpbb.com/viewtopic.php', + ), + ); + } + + /** + * @dataProvider build_url_test_data + */ + public function test_build_url($page, $strip_vars, $expected) + { + global $user, $phpbb_root_path; + + $user->page['page'] = $page; + $output = build_url($strip_vars); + + $this->assertEquals($expected, $output); + } +} diff --git a/tests/path_helper/web_root_path_test.php b/tests/path_helper/path_helper_test.php similarity index 59% rename from tests/path_helper/web_root_path_test.php rename to tests/path_helper/path_helper_test.php index ec04135997..e77ad6d343 100644 --- a/tests/path_helper/web_root_path_test.php +++ b/tests/path_helper/path_helper_test.php @@ -7,7 +7,7 @@ * */ -class phpbb_path_helper_web_root_path_test extends phpbb_test_case +class phpbb_path_helper_test extends phpbb_test_case { protected $path_helper; protected $phpbb_root_path = ''; @@ -176,4 +176,140 @@ class phpbb_path_helper_web_root_path_test extends phpbb_test_case { $this->assertEquals($expected, $this->path_helper->clean_url($input)); } + + public function glue_url_params_data() + { + return array( + array( + array(), + '', + ), + array( + array('test' => 'xyz'), + 'test=xyz', + ), + array( + array('test' => 'xyz', 'var' => 'value'), + 'test=xyz&var=value', + ), + ); + } + + /** + * @dataProvider glue_url_params_data + */ + public function test_glue_url_params($params, $expected) + { + $this->assertEquals($expected, $this->path_helper->glue_url_params($params)); + } + + public function get_url_parts_data() + { + return array( + array( + 'viewtopic.php', + true, + array('base' => 'viewtopic.php', 'params' => array()), + ), + array( + './viewtopic.php?t=5&f=6', + true, + array('base' => './viewtopic.php', 'params' => array('t' => '5', 'f' => '6')), + ), + array( + 'viewtopic.php?t=5&f=6', + false, + array('base' => 'viewtopic.php', 'params' => array('t' => '5', 'f' => '6')), + ), + array( + 'https://phpbb.com/community/viewtopic.php?t=5&f=6', + true, + array('base' => 'https://phpbb.com/community/viewtopic.php', 'params' => array('t' => '5', 'f' => '6')), + ), + ); + } + + /** + * @dataProvider get_url_parts_data + */ + public function test_get_url_parts($url, $is_amp, $expected) + { + $this->assertEquals($expected, $this->path_helper->get_url_parts($url, $is_amp)); + } + + public function strip_url_params_data() + { + return array( + array( + 'viewtopic.php', + 'sid', + false, + 'viewtopic.php', + ), + array( + './viewtopic.php?t=5&f=6', + 'f', + true, + './viewtopic.php?t=5', + ), + array( + 'viewtopic.php?t=5&f=6&sid=19adc288814103cbb4625e74e77455aa', + array('t'), + false, + 'viewtopic.php?f=6&sid=19adc288814103cbb4625e74e77455aa', + ), + array( + 'https://phpbb.com/community/viewtopic.php?t=5&f=6', + array('t', 'f'), + true, + 'https://phpbb.com/community/viewtopic.php', + ), + ); + } + + /** + * @dataProvider strip_url_params_data + */ + public function test_strip_url_params($url, $strip, $is_amp, $expected) + { + $this->assertEquals($expected, $this->path_helper->strip_url_params($url, $strip, $is_amp)); + } + + public function append_url_params_data() + { + return array( + array( + 'viewtopic.php', + array(), + false, + 'viewtopic.php', + ), + array( + './viewtopic.php?t=5&f=6', + array('t' => '7'), + true, + './viewtopic.php?t=7&f=6', + ), + array( + 'viewtopic.php?t=5&f=6&sid=19adc288814103cbb4625e74e77455aa', + array('p' => '5'), + false, + 'viewtopic.php?t=5&f=6&p=5&sid=19adc288814103cbb4625e74e77455aa', + ), + array( + 'https://phpbb.com/community/viewtopic.php', + array('t' => '7', 'f' => '8'), + true, + 'https://phpbb.com/community/viewtopic.php?t=7&f=8', + ), + ); + } + + /** + * @dataProvider append_url_params_data + */ + public function test_append_url_params($url, $params, $is_amp, $expected) + { + $this->assertEquals($expected, $this->path_helper->append_url_params($url, $params, $is_amp)); + } } From 5e0ab146d9f592c62661ccd495ce096701b8208d Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 27 Feb 2014 23:34:01 -0800 Subject: [PATCH 08/16] [ticket/11508] The question mark is expected even if there are no parameters. PHPBB3-11508 --- phpBB/includes/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 2062cd062c..17cc109432 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2376,7 +2376,7 @@ function build_url($strip_vars = false) $redirect = $path_helper->strip_url_params($redirect, $strip_vars, false); } - return $redirect; + return $redirect . ((strpos($redirect, '?') === false) ? '?' : ''); } /** From 5e7bcd2fa6ef4e24dc98d15c1eddd76593f56c1a Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 27 Feb 2014 23:42:43 -0800 Subject: [PATCH 09/16] [ticket/11508] Curly brace should be on its own line... PHPBB3-11508 --- phpBB/phpbb/path_helper.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index ac671fc400..cbb9c72f44 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -324,7 +324,8 @@ class path_helper $params = array_merge($url_parts['params'], $new_params); // Move the sid to the end if it's set - if (isset($params['sid'])) { + if (isset($params['sid'])) + { $sid = $params['sid']; unset($params['sid']); $params['sid'] = $sid; From 141d386025b8a47a787b95f82058cc3c766a4007 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Fri, 28 Feb 2014 00:02:40 -0800 Subject: [PATCH 10/16] [ticket/11508] Fix build_url test. PHPBB3-11508 --- tests/functions/build_url_test.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functions/build_url_test.php b/tests/functions/build_url_test.php index 8d0aa76317..7e1c620664 100644 --- a/tests/functions/build_url_test.php +++ b/tests/functions/build_url_test.php @@ -21,7 +21,7 @@ class phpbb_build_url_test extends phpbb_test_case $user = new phpbb_mock_user(); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $this->path_helper = new \phpbb\path_helper( + $path_helper = new \phpbb\path_helper( new \phpbb\symfony_request( new phpbb_mock_request() ), @@ -52,12 +52,12 @@ class phpbb_build_url_test extends phpbb_test_case array( 'viewtopic.php?f=2&style=1&t=6', array('f', 'style', 't'), - 'phpBB/viewtopic.php', + 'phpBB/viewtopic.php?', ), array( 'http://test.phpbb.com/viewtopic.php?f=2&style=1&t=6', array('f', 'style', 't'), - 'http://test.phpbb.com/viewtopic.php', + 'http://test.phpbb.com/viewtopic.php?', ), ); } From b0dd532c14830b6c92dee8f10cec14c3057369a9 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Wed, 19 Mar 2014 21:55:10 -0700 Subject: [PATCH 11/16] [ticket/11508] Remove unnecessary isset check. PHPBB3-11508 --- phpBB/phpbb/path_helper.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index cbb9c72f44..f770cb560d 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -300,10 +300,7 @@ class path_helper // Strip the parameters off foreach ($strip as $param) { - if (isset($params[$param])) - { - unset($params[$param]); - } + unset($params[$param]); } } From 9e605b1338821b5866102c527264304e91d8240f Mon Sep 17 00:00:00 2001 From: Cesar G Date: Wed, 19 Mar 2014 22:22:48 -0700 Subject: [PATCH 12/16] [ticket/11508] Remove unnecessary implode. PHPBB3-11508 --- phpBB/phpbb/path_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index f770cb560d..a9b520be15 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -263,7 +263,7 @@ class path_helper continue; } - $params[$key] = implode('=', $arguments); + $params[$key] = $arguments[1]; } } else From e287eea2c6c1530c843ca57d75c35f0e5062507d Mon Sep 17 00:00:00 2001 From: Cesar G Date: Tue, 15 Apr 2014 13:13:37 -0700 Subject: [PATCH 13/16] [ticket/11508] Allow equal sign in parameter value. PHPBB3-11508 --- phpBB/phpbb/path_helper.php | 10 ++++++---- tests/path_helper/path_helper_test.php | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index a9b520be15..f92c2b72b2 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -254,16 +254,18 @@ class path_helper foreach ($args as $argument) { - $arguments = explode('=', $argument); - $key = $arguments[0]; - unset($arguments[0]); + if (empty($argument)) + { + continue; + } + list($key, $value) = explode('=', $argument, 2); if ($key === '') { continue; } - $params[$key] = $arguments[1]; + $params[$key] = $value; } } else diff --git a/tests/path_helper/path_helper_test.php b/tests/path_helper/path_helper_test.php index e77ad6d343..724f26956c 100644 --- a/tests/path_helper/path_helper_test.php +++ b/tests/path_helper/path_helper_test.php @@ -226,6 +226,21 @@ class phpbb_path_helper_test extends phpbb_test_case true, array('base' => 'https://phpbb.com/community/viewtopic.php', 'params' => array('t' => '5', 'f' => '6')), ), + array( + 'test.php?topic=post=5&f=3', + true, + array('base' => 'test.php', 'params' => array('topic' => 'post=5', 'f' => '3')), + ), + array( + 'mcp.php?&t=4&f=3', + true, + array('base' => 'mcp.php', 'params' => array('t' => '4', 'f' => '3')), + ), + array( + 'mcp.php?=4&f=3', + true, + array('base' => 'mcp.php', 'params' => array('f' => '3')), + ), ); } From c2ac76e8b11c0cfb5917febe90f56216d74bc10a Mon Sep 17 00:00:00 2001 From: Cesar G Date: Tue, 22 Apr 2014 12:40:28 -0700 Subject: [PATCH 14/16] [ticket/11508] Use $phpbb_path_helper. PHPBB3-11508 --- phpBB/includes/functions.php | 9 ++++----- phpBB/includes/functions_content.php | 5 ++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 17cc109432..6e5f6896a6 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2344,10 +2344,9 @@ function reapply_sid($url) */ function build_url($strip_vars = false) { - global $config, $user, $phpbb_container; + global $config, $user, $phpbb_path_helper; - $path_helper = $phpbb_container->get('path_helper'); - $php_ext = $path_helper->get_php_ext(); + $php_ext = $phpbb_path_helper->get_php_ext(); $page = $user->page['page']; // We need to be cautious here. @@ -2365,7 +2364,7 @@ function build_url($strip_vars = false) $page = substr($page, strlen('app.' . $php_ext . '/')); } - $page = $path_helper->get_phpbb_root_path() . $page; + $page = $phpbb_path_helper->get_phpbb_root_path() . $page; } // Append SID @@ -2373,7 +2372,7 @@ function build_url($strip_vars = false) if ($strip_vars !== false) { - $redirect = $path_helper->strip_url_params($redirect, $strip_vars, false); + $redirect = $phpbb_path_helper->strip_url_params($redirect, $strip_vars, false); } return $redirect . ((strpos($redirect, '?') === false) ? '?' : ''); diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index ff75a5b05d..b8314c076c 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -110,7 +110,7 @@ function gen_sort_selects(&$limit_days, &$sort_by_text, &$sort_days, &$sort_key, */ function make_jumpbox($action, $forum_id = false, $select_all = false, $acl_list = false, $force_display = false) { - global $config, $auth, $template, $user, $db, $phpbb_container; + global $config, $auth, $template, $user, $db, $phpbb_path_helper; // We only return if the jumpbox is not forced to be displayed (in case it is needed for functionality) if (!$config['load_jumpbox'] && $force_display === false) @@ -196,8 +196,7 @@ function make_jumpbox($action, $forum_id = false, $select_all = false, $acl_list $db->sql_freeresult($result); unset($padding_store); - $path_helper = $phpbb_container->get('path_helper'); - $url_parts = $path_helper->get_url_parts($action); + $url_parts = $phpbb_path_helper->get_url_parts($action); $template->assign_vars(array( 'S_DISPLAY_JUMPBOX' => $display_jumpbox, From e104c364f494b44a05e1fae4e0caf1f67de7ea63 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Tue, 22 Apr 2014 15:20:35 -0700 Subject: [PATCH 15/16] [ticket/11508] Add functional test for jumpbox. PHPBB3-11508 --- tests/functional/jumpbox_test.php | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/functional/jumpbox_test.php diff --git a/tests/functional/jumpbox_test.php b/tests/functional/jumpbox_test.php new file mode 100644 index 0000000000..b987d2b99b --- /dev/null +++ b/tests/functional/jumpbox_test.php @@ -0,0 +1,33 @@ +login(); + + $crawler = self::request('GET', "viewtopic.php?t=1&sid={$this->sid}"); + $form = $crawler->filter('#quickmodform')->selectButton($this->lang('GO'))->form(array( + 'action' => 'merge_topic', + )); + + $crawler = self::submit($form); + $this->assertContains($this->lang('FORUM') . ': Your first forum', $crawler->filter('#cp-main h2')->text()); + $form = $crawler->filter('#jumpbox')->selectButton($this->lang('GO'))->form(array( + 'f' => 1, + )); + + $crawler = self::submit($form); + $this->assertContains($this->lang('FORUM') . ': Your first category', $crawler->filter('#cp-main h2')->text()); + } +} From e9b00071dd1e7ae40a04e26d3f337d70bafbcf93 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Tue, 22 Apr 2014 16:03:13 -0700 Subject: [PATCH 16/16] [ticket/11508] Fix build_url test. PHPBB3-11508 --- tests/functions/build_url_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functions/build_url_test.php b/tests/functions/build_url_test.php index 7e1c620664..795427ffe8 100644 --- a/tests/functions/build_url_test.php +++ b/tests/functions/build_url_test.php @@ -13,7 +13,7 @@ class phpbb_build_url_test extends phpbb_test_case { protected function setUp() { - global $user, $phpbb_dispatcher, $phpbb_container, $phpbb_root_path; + global $user, $phpbb_dispatcher, $phpbb_container, $phpbb_root_path, $phpbb_path_helper; parent::setUp(); @@ -21,7 +21,7 @@ class phpbb_build_url_test extends phpbb_test_case $user = new phpbb_mock_user(); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $path_helper = new \phpbb\path_helper( + $phpbb_path_helper = new \phpbb\path_helper( new \phpbb\symfony_request( new phpbb_mock_request() ),