Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ticket/15351] Makes confirm_works in a router context (app.php) #4932

Merged
merged 1 commit into from Sep 9, 2017
Merged

[ticket/15351] Makes confirm_works in a router context (app.php) #4932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions phpBB/includes/functions.php
View file
Open in desktop
Expand Up @@ -1839,7 +1839,7 @@ function redirect($url, $return = false, $disable_cd_check = false)
/**
* Re-Apply session id after page reloads
*/
function reapply_sid($url)
function reapply_sid($url, $is_route = false)
{
global $phpEx, $phpbb_root_path;

Expand All @@ -1861,7 +1861,7 @@ function reapply_sid($url)
$url = preg_replace("/$phpEx(&|&)+?/", "$phpEx?", $url);
}

return append_sid($url);
return append_sid($url, false, true, false, $is_route);
}

/**
Expand Down Expand Up @@ -2184,7 +2184,7 @@ function confirm_box($check, $title = '', $hidden = '', $html_body = 'confirm_bo

// re-add sid / transform & to & for user->page (user->page is always using &)
$use_page = ($u_action) ? $u_action : str_replace('&', '&', $user->page['page']);
$u_action = reapply_sid($phpbb_path_helper->get_valid_page($use_page, $config['enable_mod_rewrite']));
$u_action = reapply_sid($phpbb_path_helper->get_valid_page($use_page, $config['enable_mod_rewrite']), $phpbb_path_helper->is_router_used());
$u_action .= ((strpos($u_action, '?') === false) ? '?' : '&') . 'confirm_key=' . $confirm_key;

$template->assign_vars(array(
Expand Down
13 changes: 13 additions & 0 deletions phpBB/phpbb/path_helper.php
View file
Open in desktop
Expand Up @@ -496,4 +496,17 @@ public function get_valid_page($page, $mod_rewrite = false)

return $page;
}

/**
* Tells if the router is currently in use (if the current page is a route or not)
*
* @return bool
*/
public function is_router_used()
{
// Script name URI (e.g. phpBB/app.php)
$script_name = $this->symfony_request->getScriptName();

return basename($script_name) === 'app.' . $this->php_ext;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also work when the .htaccess redirection is in place?
Bugs that are connected to this are nasty.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it works. The script name should always be defined

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely it's always defined but does it also always point to the app.php when routing through it but is redirected by .htaccess?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep

}
}
15 changes: 12 additions & 3 deletions phpBB/phpbb/session.php
View file
Open in desktop
Expand Up @@ -91,9 +91,18 @@ static function extract_current_page($root_path)
$page_name .= str_replace('%2F', '/', urlencode($symfony_request_path));
}

// current directory within the phpBB root (for example: adm)
$root_dirs = explode('/', str_replace('\\', '/', $phpbb_filesystem->realpath($root_path)));
$page_dirs = explode('/', str_replace('\\', '/', $phpbb_filesystem->realpath('./')));
if (substr($root_path, 0, 2) === './' && strpos($root_path, '..') === false)
{
$root_dirs = explode('/', str_replace('\\', '/', rtrim($root_path, '/')));
$page_dirs = explode('/', str_replace('\\', '/', '.'));
}
else
{
// current directory within the phpBB root (for example: adm)
$root_dirs = explode('/', str_replace('\\', '/', $phpbb_filesystem->realpath($root_path)));
$page_dirs = explode('/', str_replace('\\', '/', $phpbb_filesystem->realpath('./')));
}

$intersection = array_intersect_assoc($root_dirs, $page_dirs);

$root_dirs = array_diff_assoc($root_dirs, $intersection);
Expand Down
16 changes: 16 additions & 0 deletions tests/session/extract_page_test.php
View file
Open in desktop
Expand Up @@ -136,6 +136,22 @@ static public function extract_current_page_data()
'forum' => 0,
),
),
array(
'./community',
'/app.php',
'',
'/',
'/kb',
array(
'page_name' => 'app.php/kb',
'page_dir' => '..',
'query_string' => '',
'script_path' => '/',
'root_script_path' => '/community/',
'page' => '../app.php/kb',
'forum' => 0,
),
),
);
}

Expand Down