From 0a88ed48ef7751829d74c8ab3c1f3bb85b01382d Mon Sep 17 00:00:00 2001 From: Carlos Proensa Date: Sun, 4 Dec 2016 02:07:26 +0100 Subject: [PATCH 1/7] Improve parameter substitution for database log For showing the database query log, only mysql style parameters "?" were supported. Other like postgres "$nnn" or oracle ":xxx" were not treated. Change the token substitution to allow other token types to be replaced with the parameter value for log output. Fixes: #22005 --- core/database_api.php | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/core/database_api.php b/core/database_api.php index 23118d8b3e..c1e7daf9d4 100644 --- a/core/database_api.php +++ b/core/database_api.php @@ -409,36 +409,46 @@ function db_query( $p_query, array $p_arr_parms = null, $p_limit = -1, $p_offset $t_lastoffset = 0; $i = 0; if( !empty( $p_arr_parms ) ) { - while( preg_match( '/\?/', $p_query, $t_matches, PREG_OFFSET_CAPTURE, $t_lastoffset ) ) { - $t_matches = $t_matches[0]; + # For mysql, tokens are '?', and parameters are binded sequentially + # For pgsql, tokens are '$number', and parameters are binded by the denoted index (1-based) in the parameter array + # For oracle, tokens are ':string', but ADOdb treat them as sequential in appearance order, so they behave like mysql + while( preg_match( '/(?\?|\$|:)(?[0-9]*)/', $p_query, $t_matches, PREG_OFFSET_CAPTURE, $t_lastoffset ) ) { + $t_match_param = $t_matches[0]; # Realign the offset returned by preg_match as it is byte-based, # which causes issues with UTF-8 characters in the query string # (e.g. from custom fields names) - $t_utf8_offset = utf8_strlen( substr( $p_query, 0, $t_matches[1] ), mb_internal_encoding() ); + $t_utf8_offset = utf8_strlen( substr( $p_query, 0, $t_match_param[1] ), mb_internal_encoding() ); if( $i <= count( $p_arr_parms ) ) { - if( is_null( $p_arr_parms[$i] ) ) { + if( 'pgsql' == $t_db_type ) { + # For pgsql, the binded value is indexed by the parameter name + $t_index = (int)$t_matches['index'][0]; + $t_value = $p_arr_parms[$t_index-1]; + } else { + $t_value = $p_arr_parms[$i]; + } + if( is_null( $t_value ) ) { $t_replace = 'NULL'; - } else if( is_string( $p_arr_parms[$i] ) ) { - $t_replace = "'" . $p_arr_parms[$i] . "'"; - } else if( is_integer( $p_arr_parms[$i] ) || is_float( $p_arr_parms[$i] ) ) { - $t_replace = (float)$p_arr_parms[$i]; - } else if( is_bool( $p_arr_parms[$i] ) ) { + } else if( is_string( $t_value ) ) { + $t_replace = "'" . $t_value . "'"; + } else if( is_integer( $t_value ) || is_float( $t_value ) ) { + $t_replace = (float)$t_value; + } else if( is_bool( $t_value ) ) { switch( $t_db_type ) { case 'pgsql': - $t_replace = '\'' . $p_arr_parms[$i] . '\''; + $t_replace = '\'' . $t_value . '\''; break; default: - $t_replace = $p_arr_parms[$i]; + $t_replace = $t_value; break; } } else { echo( 'Invalid argument type passed to query_bound(): ' . ( $i + 1 ) ); exit( 1 ); } - $p_query = utf8_substr( $p_query, 0, $t_utf8_offset ) . $t_replace . utf8_substr( $p_query, $t_utf8_offset + utf8_strlen( $t_matches[0] ) ); - $t_lastoffset = $t_matches[1] + strlen( $t_replace ) + 1; + $p_query = utf8_substr( $p_query, 0, $t_utf8_offset ) . $t_replace . utf8_substr( $p_query, $t_utf8_offset + utf8_strlen( $t_match_param[0] ) ); + $t_lastoffset = $t_match_param[1] + strlen( $t_replace ) + 1; } else { - $t_lastoffset = $t_matches[1] + 1; + $t_lastoffset = $t_match_param[1] + 1; } $i++; } From d697a845a69ca8134e011f526818fcf60c08befb Mon Sep 17 00:00:00 2001 From: Carlos Proensa Date: Mon, 5 Dec 2016 19:50:32 +0100 Subject: [PATCH 2/7] Move query log formatting to separate function Move into a separe function the code that prepares a query string that is going to be shown in database log. --- core/database_api.php | 115 +++++++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 51 deletions(-) diff --git a/core/database_api.php b/core/database_api.php index c1e7daf9d4..24c78d11ee 100644 --- a/core/database_api.php +++ b/core/database_api.php @@ -406,59 +406,13 @@ function db_query( $p_query, array $p_arr_parms = null, $p_limit = -1, $p_offset $t_elapsed = number_format( microtime( true ) - $t_start, 4 ); if( ON == $g_db_log_queries ) { - $t_lastoffset = 0; - $i = 0; - if( !empty( $p_arr_parms ) ) { - # For mysql, tokens are '?', and parameters are binded sequentially - # For pgsql, tokens are '$number', and parameters are binded by the denoted index (1-based) in the parameter array - # For oracle, tokens are ':string', but ADOdb treat them as sequential in appearance order, so they behave like mysql - while( preg_match( '/(?\?|\$|:)(?[0-9]*)/', $p_query, $t_matches, PREG_OFFSET_CAPTURE, $t_lastoffset ) ) { - $t_match_param = $t_matches[0]; - # Realign the offset returned by preg_match as it is byte-based, - # which causes issues with UTF-8 characters in the query string - # (e.g. from custom fields names) - $t_utf8_offset = utf8_strlen( substr( $p_query, 0, $t_match_param[1] ), mb_internal_encoding() ); - if( $i <= count( $p_arr_parms ) ) { - if( 'pgsql' == $t_db_type ) { - # For pgsql, the binded value is indexed by the parameter name - $t_index = (int)$t_matches['index'][0]; - $t_value = $p_arr_parms[$t_index-1]; - } else { - $t_value = $p_arr_parms[$i]; - } - if( is_null( $t_value ) ) { - $t_replace = 'NULL'; - } else if( is_string( $t_value ) ) { - $t_replace = "'" . $t_value . "'"; - } else if( is_integer( $t_value ) || is_float( $t_value ) ) { - $t_replace = (float)$t_value; - } else if( is_bool( $t_value ) ) { - switch( $t_db_type ) { - case 'pgsql': - $t_replace = '\'' . $t_value . '\''; - break; - default: - $t_replace = $t_value; - break; - } - } else { - echo( 'Invalid argument type passed to query_bound(): ' . ( $i + 1 ) ); - exit( 1 ); - } - $p_query = utf8_substr( $p_query, 0, $t_utf8_offset ) . $t_replace . utf8_substr( $p_query, $t_utf8_offset + utf8_strlen( $t_match_param[0] ) ); - $t_lastoffset = $t_match_param[1] + strlen( $t_replace ) + 1; - } else { - $t_lastoffset = $t_match_param[1] + 1; - } - $i++; - } - } - $t_log_msg = array( $p_query, $t_elapsed ); - log_event( LOG_DATABASE, $t_log_msg ); - array_push( $g_queries_array, $t_log_msg ); + $t_query_text = db_format_query_log_msg( $p_query, $p_arr_parms ); + log_event( LOG_DATABASE, array( $t_query_text, $t_elapsed ) ); } else { - array_push( $g_queries_array, array( '', $t_elapsed ) ); + # If not logging the queries the actual text is not needed + $t_query_text = ''; } + array_push( $g_queries_array, array( $t_query_text, $t_elapsed ) ); # Restore param stack: only pop if asked to AND the query has params if( $p_pop_param && !empty( $p_arr_parms ) ) { @@ -1333,3 +1287,62 @@ function db_mysql_fix_utf8( $p_string ) { function db_empty_result() { return new ADORecordSet_empty(); } + +/** + * Process a query string by replacing token parameters by their bound values + * @param string $p_query Query string + * @param array $p_arr_parms Parameter array + * @return string Processed query string + */ +function db_format_query_log_msg( $p_query, array $p_arr_parms ) { + $t_lastoffset = 0; + $i = 0; + if( !empty( $p_arr_parms ) ) { + # For mysql, tokens are '?', and parameters are bound sequentially + # For pgsql, tokens are '$number', and parameters are bound by the denoted + # index (1-based) in the parameter array + # For oracle, tokens are ':string', but mantis rewrites them as sequentially + # ordered, so they behave like mysql. See db_oracle_order_binds_sequentially() + $t_regex = '/(?\?|\$|:)(?[0-9]*)/'; + while( preg_match( $t_regex , $p_query, $t_matches, PREG_OFFSET_CAPTURE, $t_lastoffset ) ) { + $t_match_param = $t_matches[0]; + # Realign the offset returned by preg_match as it is byte-based, + # which causes issues with UTF-8 characters in the query string + # (e.g. from custom fields names) + $t_utf8_offset = utf8_strlen( substr( $p_query, 0, $t_match_param[1] ), mb_internal_encoding() ); + if( $i <= count( $p_arr_parms ) ) { + if( db_is_pgsql() ) { + # For pgsql, the bound value is indexed by the parameter name + $t_index = (int)$t_matches['index'][0]; + $t_value = $p_arr_parms[$t_index-1]; + } else { + $t_value = $p_arr_parms[$i]; + } + if( is_null( $t_value ) ) { + $t_replace = 'NULL'; + } else if( is_string( $t_value ) ) { + $t_replace = "'" . $t_value . "'"; + } else if( is_integer( $t_value ) || is_float( $t_value ) ) { + $t_replace = (float)$t_value; + } else if( is_bool( $t_value ) ) { + if( db_is_pgsql() ) { + $t_replace = '\'' . $t_value . '\''; + } else { + $t_replace = $t_value; + } + } else { + echo( 'Invalid argument type passed to query_bound(): ' . ( $i + 1 ) ); + exit( 1 ); + } + $p_query = utf8_substr( $p_query, 0, $t_utf8_offset ) + . $t_replace + . utf8_substr( $p_query, $t_utf8_offset + utf8_strlen( $t_match_param[0] ) ); + $t_lastoffset = $t_match_param[1] + strlen( $t_replace ) + 1; + } else { + $t_lastoffset = $t_match_param[1] + 1; + } + $i++; + } + } + return $p_query; +} \ No newline at end of file From 9bdc53946cc47a55405fc078efd2a0f98fa9e0c5 Mon Sep 17 00:00:00 2001 From: Carlos Proensa Date: Tue, 6 Dec 2016 13:20:43 +0100 Subject: [PATCH 3/7] Remove exit for unsupported parameter type Skip subtitution of unsupported type parameter, instead of directly stop execution. If unsupported types are present in the parameter array, the query execution may fail, but at this point is not the responsability of this code to stop the script. --- core/database_api.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/database_api.php b/core/database_api.php index 24c78d11ee..7e8e3e635b 100644 --- a/core/database_api.php +++ b/core/database_api.php @@ -1331,8 +1331,9 @@ function db_format_query_log_msg( $p_query, array $p_arr_parms ) { $t_replace = $t_value; } } else { - echo( 'Invalid argument type passed to query_bound(): ' . ( $i + 1 ) ); - exit( 1 ); + # Could not find a supported type for this parameter value. + # Skip this token, so replacing it with itself. + $t_replace = $t_match_param[0]; } $p_query = utf8_substr( $p_query, 0, $t_utf8_offset ) . $t_replace From b75b58efefc8c316cabe84718ba1efa7d75ae30a Mon Sep 17 00:00:00 2001 From: Carlos Proensa Date: Wed, 7 Dec 2016 00:58:09 +0100 Subject: [PATCH 4/7] Fix subtitution of boolean parameters Use the correct literal used by the database driver. Fixes: #22018 --- core/database_api.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/core/database_api.php b/core/database_api.php index 7e8e3e635b..920850eb10 100644 --- a/core/database_api.php +++ b/core/database_api.php @@ -1295,6 +1295,8 @@ function db_empty_result() { * @return string Processed query string */ function db_format_query_log_msg( $p_query, array $p_arr_parms ) { + global $g_db; + $t_lastoffset = 0; $i = 0; if( !empty( $p_arr_parms ) ) { @@ -1325,11 +1327,8 @@ function db_format_query_log_msg( $p_query, array $p_arr_parms ) { } else if( is_integer( $t_value ) || is_float( $t_value ) ) { $t_replace = (float)$t_value; } else if( is_bool( $t_value ) ) { - if( db_is_pgsql() ) { - $t_replace = '\'' . $t_value . '\''; - } else { - $t_replace = $t_value; - } + # use the actual literal from db driver + $t_replace = $t_value ? $g_db->true : $g_db->false; } else { # Could not find a supported type for this parameter value. # Skip this token, so replacing it with itself. From bbf4cac702cbf050d5aa6cae40e4d997195a129f Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Wed, 4 Jan 2017 14:43:37 +0100 Subject: [PATCH 5/7] Allow helper_mantis_url() to be called twice Starting with Mantis 2.0, the Layout API's layout_sidebar_menu() function takes care of calling helper_mantis_url() for relative URLs in menu items. Since that function blindly prepends $g_short_path to the given URL, this causes issues for plugins relying on a plugin_page() call with default parameters, e.g. to display menu items via EVENT_MENU_MAIN event, because the short path is prefixed twice resulting in generation of an invalid URL. As pointed out by user @TiKu in ~54922 using plugin_page()'s $p_redirect parameter = true as it was done for the Source Integration plugin [1] is really a workaround. This commit fixes the issue by only prepending the short path if it is not yet part of the given URL string. Fixes #22107 [1] https://github.com/mantisbt-plugins/source-integration/commit/1491ba13b288d1b8ac18d3025e3db29e62265cb5 --- core/helper_api.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/core/helper_api.php b/core/helper_api.php index 47d6b0e813..d63d1e27b5 100644 --- a/core/helper_api.php +++ b/core/helper_api.php @@ -605,7 +605,14 @@ function helper_mantis_url( $p_url ) { if( is_blank( $p_url ) ) { return $p_url; } - return config_get_global( 'short_path' ) . $p_url; + + # Return URL as-is if it already starts with short path + $t_short_path = config_get_global( 'short_path' ); + if( strpos( $p_url, $t_short_path ) === 0 ) { + return $p_url; + } + + return $t_short_path . $p_url; } /** From 5dc6026de468e2946936d0b51286f32eee753e3e Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Wed, 11 Jan 2017 13:07:34 +0100 Subject: [PATCH 6/7] gitignore exlude parsedown library --- .gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 7643e2afb1..f974a25559 100644 --- a/.gitignore +++ b/.gitignore @@ -46,6 +46,8 @@ nbproject/ web.config !library/securimage/web.config -#libraries +# Libraries library/FirePHPCore library/ezc +# Parsedown submodule added in 2.1 +library/parsedown From 0f9b7eb4f204ff9980f6c887282f87103260fdb5 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Wed, 11 Jan 2017 13:07:34 +0100 Subject: [PATCH 7/7] gitignore exlude parsedown library --- .gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index d88c288a46..1ba12ffc16 100755 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,8 @@ nbproject/ web.config !library/securimage/web.config -#libraries +# Libraries library/jpgraph library/FirePHPCore +# Parsedown submodule added in 2.1 +library/parsedown