mirror/php-phpbb
1
0
Fork 0
mirror of https://github.com/phpbb/phpbb.git synced 2025-01-18 22:58:10 +01:00
Code Issues Releases Wiki Activity

Merge remote-tracking branch 'github-prototech/ticket/11508' into develop-ascraeus

Browse Source 
* github-prototech/ticket/11508:
  [ticket/11508] Fix build_url test.
  [ticket/11508] Add functional test for jumpbox.
  [ticket/11508] Use $phpbb_path_helper.
  [ticket/11508] Allow equal sign in parameter value.
  [ticket/11508] Remove unnecessary implode.
  [ticket/11508] Remove unnecessary isset check.
  [ticket/11508] Fix build_url test.
  [ticket/11508] Curly brace should be on its own line...
  [ticket/11508] The question mark is expected even if there are no parameters.
  [ticket/11508] Add tests.
  [ticket/11508] Do not add the '?' unless there are parameters.
  [ticket/11508] Change separator parameter to a simple true|false $is_amp.
  [ticket/11508] Move helper functions to path_helper class.
  [ticket/11508] Move the stripping param code to separate function as well.
  [ticket/11508] Build the jumpbox hidden fields using the _form action_
  [ticket/11508] Split parts of build_url() into reusable functions.
This commit is contained in:
Nils Adermann 2014-05-02 16:40:01 +02:00
commit e65c51e3cd
6 changed files with 393 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
phpBB/includes/functions.php
View File

@ -2326,8 +2326,9 @@ function reapply_sid($url)
*/
function build_url($strip_vars = false)
{
global $config, $user, $phpEx, $phpbb_root_path;
global $config, $user, $phpbb_path_helper;
$php_ext = $phpbb_path_helper->get_php_ext();
$page = $user->page['page'];
// We need to be cautious here.
@ -2340,71 +2341,23 @@ 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 = $phpbb_path_helper->get_phpbb_root_path() . $page;
}
// Append SID
$redirect = append_sid($page, false, false);
// Add delimiter if not there...
if (strpos($redirect, '?') === false)
if ($strip_vars !== false)
{
$redirect .= '?';
$redirect = $phpbb_path_helper->strip_url_params($redirect, $strip_vars, false);
}
// Strip vars...
if ($strip_vars !== false && strpos($redirect, '?') !== false)
{
if (!is_array($strip_vars))
{
$strip_vars = array($strip_vars);
}
$query = $_query = array();
$args = substr($redirect, strpos($redirect, '?') + 1);
$args = ($args) ? explode('&', $args) : array();
$redirect = substr($redirect, 0, strpos($redirect, '?'));
foreach ($args as $argument)
{
$arguments = explode('=', $argument);
$key = $arguments[0];
unset($arguments[0]);
if ($key === '')
{
continue;
}
$query[$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 : '';
}
return str_replace('&', '&', $redirect);
return $redirect . ((strpos($redirect, '?') === false) ? '?' : '');
}
/**
@ -4902,7 +4855,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.
@ -4919,7 +4871,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'] : '',

11
phpBB/includes/functions_content.php
View File

@ -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_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,10 +196,13 @@ function make_jumpbox($action, $forum_id = false, $select_all = false, $acl_list
$db->sql_freeresult($result);
unset($padding_store);
$url_parts = $phpbb_path_helper->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;
}

116
phpBB/phpbb/path_helper.php
View File

@ -216,4 +216,120 @@ 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 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, $is_amp = true)
{
$separator = ($is_amp) ? '&' : '&';
$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)
{
if (empty($argument))
{
continue;
}
list($key, $value) = explode('=', $argument, 2);
if ($key === '')
{
continue;
}
$params[$key] = $value;
}
}
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 bool $is_amp Is the parameter separator &. Defaults to true.
* @return string Returns the new URL.
*/
public function strip_url_params($url, $strip, $is_amp = true)
{
$url_parts = $this->get_url_parts($url, $is_amp);
$params = $url_parts['params'];
if (!is_array($strip))
{
$strip = array($strip);
}
if (!empty($params))
{
// Strip the parameters off
foreach ($strip as $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 $is_amp Is the parameter separator &. Defaults to true.
* @return string Returns the new URL.
*/
public function append_url_params($url, $new_params, $is_amp = true)
{
$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
if (isset($params['sid']))
{
$sid = $params['sid'];
unset($params['sid']);
$params['sid'] = $sid;
}
return $url_parts['base'] . (($params) ? '?' . $this->glue_url_params($params) : '');
}
}

33
tests/functional/jumpbox_test.php Normal file
View File

@ -0,0 +1,33 @@
<?php
/**
*
* @package testing
* @copyright (c) 2014 phpBB Group
* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
*
*/
/**
* @group functional
*/
class phpbb_functional_jumpbox_test extends phpbb_functional_test_case
{
public function test_jumpbox()
{
$this->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());
}
}

77
tests/functions/build_url_test.php Normal file
View File

@ -0,0 +1,77 @@
<?php
/**
*
* @package testing
* @copyright (c) 2014 phpBB Group
* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
*
*/
require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
class phpbb_build_url_test extends phpbb_test_case
{
protected function setUp()
{
global $user, $phpbb_dispatcher, $phpbb_container, $phpbb_root_path, $phpbb_path_helper;
parent::setUp();
$phpbb_container = new phpbb_mock_container_builder();
$user = new phpbb_mock_user();
$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
$phpbb_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&amp;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);
}
}

153
tests/path_helper/web_root_path_test.php → tests/path_helper/path_helper_test.php
View File

@ -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,155 @@ 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&amp;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&amp;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&amp;f=6',
true,
array('base' => 'https://phpbb.com/community/viewtopic.php', 'params' => array('t' => '5', 'f' => '6')),
),
array(
'test.php?topic=post=5&amp;f=3',
true,
array('base' => 'test.php', 'params' => array('topic' => 'post=5', 'f' => '3')),
),
array(
'mcp.php?&amp;t=4&amp;f=3',
true,
array('base' => 'mcp.php', 'params' => array('t' => '4', 'f' => '3')),
),
array(
'mcp.php?=4&amp;f=3',
true,
array('base' => 'mcp.php', 'params' => array('f' => '3')),
),
);
}
/**
* @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&amp;f=6',
'f',
true,
'./viewtopic.php?t=5',
),
array(
'viewtopic.php?t=5&f=6&sid=19adc288814103cbb4625e74e77455aa',
array('t'),
false,
'viewtopic.php?f=6&amp;sid=19adc288814103cbb4625e74e77455aa',
),
array(
'https://phpbb.com/community/viewtopic.php?t=5&amp;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&amp;f=6',
array('t' => '7'),
true,
'./viewtopic.php?t=7&amp;f=6',
),
array(
'viewtopic.php?t=5&f=6&sid=19adc288814103cbb4625e74e77455aa',
array('p' => '5'),
false,
'viewtopic.php?t=5&amp;f=6&amp;p=5&amp;sid=19adc288814103cbb4625e74e77455aa',
),
array(
'https://phpbb.com/community/viewtopic.php',
array('t' => '7', 'f' => '8'),
true,
'https://phpbb.com/community/viewtopic.php?t=7&amp;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));
}
}