Skip to content
Browse files

Fix performance issue on adm_config_report.php

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...
1 parent 57f5740 commit 5c3ac412d0429d824563a639f5718de3425927e7 @dregad dregad committed Jan 2, 2013
Showing with 44 additions and 15 deletions.
  1. +14 −4 adm_config_report.php
  2. +11 −5 core/form_api.php
  3. +19 −6 core/print_api.php
View
18 adm_config_report.php
@@ -339,9 +339,17 @@ function print_option_list_from_array( $p_array, $p_filter_value ) {
<?php if ( $t_read_write_access ): ?>
<th><?php echo lang_get( 'actions' ) ?></th>
<?php endif; ?>
- </tr><?php
+ </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' ); ?>
+ extract( $row, EXTR_PREFIX_ALL, 'v' );
+
+?>
+<!-- Repeated Info Rows -->
<tr <?php echo helper_alternate_class() ?> width="100%">
<td >
<?php echo ($v_user_id == 0) ? lang_get( 'all_users' ) : string_display_line( user_get_name( $v_user_id ) ) ?>
@@ -371,7 +379,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
@@ -382,7 +391,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;';
View
16 core/form_api.php
@@ -81,20 +81,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;
}
View
25 core/print_api.php
@@ -1161,18 +1161,31 @@ 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 ), '" class="action-button">';
echo '<fieldset>';
- 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 ) {

0 comments on commit 5c3ac41

Please sign in to comment.
Something went wrong with that request. Please try again.