Skip to content

Commit

Permalink
Fix performance issue on adm_config_report.php
Browse files Browse the repository at this point in the history
In systems with large numbers of config items in mantis_config_table, the
Configuration Report page can take a very long time to load.

This behavior is due to each of the 'Delete' buttons being printed with
its own form, each one having a security token. The performance
bottleneck is actually the serialize/unserialize calls executed while
storing/retrieving the token from the PHP session.

To avoid this problem, the print_button() and form_security_field()
functions have been modified to accept a security token as an optional
parameter. This allows the calling page to generate a single token,
which is shared by all buttons.

Furthermore, print_button() also allows the security token parameter to
be 'OFF', which prevents the function from displaying a security field.
This is useful for buttons not resulting in modifications (i.e. not
requiring CSRF protection).

Fixes #13680
  • Loading branch information
dregad committed Jan 2, 2013
1 parent d76a210 commit db0e3d3
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 13 deletions.
10 changes: 8 additions & 2 deletions adm_config_report.php
Expand Up @@ -329,6 +329,10 @@ function print_option_list_from_array( $p_array, $p_filter_value ) {
</td>
</tr>
<?php
# Pre-generate a form security token to avoid performance issues when the
# db contains a large number of configurations
$t_form_security_token = form_security_token( 'adm_config_delete' );

while ( $row = db_fetch_array( $result ) ) {
extract( $row, EXTR_PREFIX_ALL, 'v' );

Expand Down Expand Up @@ -369,7 +373,8 @@ function print_option_list_from_array( $p_array, $p_filter_value ) {
'config_option' => $v_config_id,
'type' => $v_type,
'value' => $v_value,
)
),
OFF
);

# Delete button
Expand All @@ -380,7 +385,8 @@ function print_option_list_from_array( $p_array, $p_filter_value ) {
'user_id' => $v_user_id,
'project_id' => $v_project_id,
'config_option' => $v_config_id,
)
),
$t_form_security_token
);
} else {
echo '&#160;';
Expand Down
16 changes: 11 additions & 5 deletions core/form_api.php
Expand Up @@ -77,20 +77,26 @@ function form_security_token( $p_form_name ) {

/**
* Get a hidden form element containing a generated form security token.
* @param string Form name
* @param string $p_form_name Form name
* @param string $p_security_token Optional security token, previously generated for the same form
* @return string Hidden form element to output
*/
function form_security_field( $p_form_name ) {
function form_security_field( $p_form_name, $p_security_token = null ) {
if ( PHP_CLI == php_mode() || OFF == config_get_global( 'form_security_validation' ) ) {
return '';
}

$t_string = form_security_token( $p_form_name );
if( is_null( $p_security_token ) ) {
$p_security_token = form_security_token( $p_form_name );
}

# Create the form element HTML string for the security token
$t_form_token = $p_form_name . '_token';
$t_element = '<input type="hidden" name="%s" value="%s"/>';
$t_element = sprintf( $t_element, $t_form_token, $t_string );
$t_element = sprintf(
'<input type="hidden" name="%s" value="%s"/>',
$t_form_token,
$p_security_token
);

return $t_element;
}
Expand Down
25 changes: 19 additions & 6 deletions core/print_api.php
Expand Up @@ -1296,17 +1296,30 @@ function print_manage_project_sort_link( $p_page, $p_string, $p_field, $p_dir, $
print_link( "$p_page?sort=$t_field&dir=$t_dir", $p_string );
}

# print a button which presents a standalone form.
# $p_action_page - The action page
# $p_label - The button label
# $p_args_to_post - An associative array with key => value to be posted, can be null.
function print_button( $p_action_page, $p_label, $p_args_to_post = null ) {
/**
* Print a button which presents a standalone form.
* If $p_security_token is OFF, the button's form will not contain a security
* field; this is useful when form does not result in modifications (CSRF is not
* needed). If otherwise specified (i.e. not null), the parameter must contain
* a valid security token, previously generated by form_security_token().
* Use this to avoid performance issues when loading pages having many calls to
* this function, such as adm_config_report.php.
* @param string $p_action_page The action page
* @param string $p_label The button label
* @param array $p_args_to_post Associative array of arguments to be posted, with
* arg name => value, defaults to null (no args)
* @param mixed $p_security_token Optional; null (default), OFF or security token string
* @see form_security_token()
*/
function print_button( $p_action_page, $p_label, $p_args_to_post = null, $p_security_token = null ) {
$t_form_name = explode( '.php', $p_action_page, 2 );
# TODO: ensure all uses of print_button supply arguments via $p_args_to_post (POST)
# instead of via $p_action_page (GET). Then only add the CSRF form token if
# arguments are being sent via the POST method.
echo '<form method="post" action="', htmlspecialchars( $p_action_page ), '">';
echo form_security_field( $t_form_name[0] );
if( $p_security_token !== OFF ) {
echo form_security_field( $t_form_name[0], $p_security_token );
}
echo '<input type="submit" class="button-small" value="', $p_label, '" />';

if( $p_args_to_post !== null ) {
Expand Down

0 comments on commit db0e3d3

Please sign in to comment.