From 61118a18ea0ba9e58f8b133a16855fa67f5eff82 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Tue, 20 May 2014 09:48:50 +0200 Subject: [PATCH 01/20] Check that a plugin is loaded when accessing its pages A plugin can be registered yet fail to load for example due to unmet dependencies. Fixes #17359 --- core/constant_inc.php | 1 + lang/strings_english.txt | 1 + plugin.php | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/core/constant_inc.php b/core/constant_inc.php index 3608556e94..f11d28151f 100644 --- a/core/constant_inc.php +++ b/core/constant_inc.php @@ -383,6 +383,7 @@ define( 'ERROR_PLUGIN_UPGRADE_FAILED', 2503 ); define( 'ERROR_PLUGIN_INSTALL_FAILED', 2504 ); define( 'ERROR_PLUGIN_UPGRADE_NEEDED', 2505 ); +define( 'ERROR_PLUGIN_NOT_LOADED', 2506 ); define( 'ERROR_PLUGIN_GENERIC', 2599 ); # ERROR_COLUMNS_* diff --git a/lang/strings_english.txt b/lang/strings_english.txt index 0c601118ee..435e3a4e12 100644 --- a/lang/strings_english.txt +++ b/lang/strings_english.txt @@ -1668,6 +1668,7 @@ $MANTIS_ERROR[ERROR_TAG_ALREADY_ATTACHED] = 'That tag already attached to that i $MANTIS_ERROR[ERROR_TOKEN_NOT_FOUND] = 'Token could not be found.'; $MANTIS_ERROR[ERROR_EVENT_UNDECLARED] = 'Event "%1$s" has not yet been declared.'; $MANTIS_ERROR[ERROR_PLUGIN_NOT_REGISTERED] = 'Plugin is not registered with MantisBT.'; +$MANTIS_ERROR[ERROR_PLUGIN_NOT_LOADED] = 'Plugin is not loaded, make sure its dependencies are met.'; $MANTIS_ERROR[ERROR_PLUGIN_ALREADY_INSTALLED] = 'Plugin is already installed.'; $MANTIS_ERROR[ERROR_PLUGIN_PAGE_NOT_FOUND] = 'Plugin page not found.'; $MANTIS_ERROR[ERROR_PLUGIN_INSTALL_FAILED] = 'Plugin installation failed: %1$s.'; diff --git a/plugin.php b/plugin.php index 0bca801262..a7ea3f95d8 100644 --- a/plugin.php +++ b/plugin.php @@ -50,6 +50,11 @@ trigger_error( ERROR_PLUGIN_NOT_REGISTERED, ERROR ); } +# Plugin can be registered but fail to load e.g. due to unmet dependencies +if( !plugin_is_loaded( $t_basename ) ) { + trigger_error( ERROR_PLUGIN_NOT_LOADED, ERROR ); +} + $t_page = $t_plugin_path.$t_basename.DIRECTORY_SEPARATOR. 'pages'.DIRECTORY_SEPARATOR.$t_action.'.php'; From 5ffa67f7842ef808df9af1f573ef204bbe5ab9c2 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Tue, 20 May 2014 09:50:23 +0200 Subject: [PATCH 02/20] Whitespace --- plugin.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin.php b/plugin.php index a7ea3f95d8..965314f16f 100644 --- a/plugin.php +++ b/plugin.php @@ -59,7 +59,7 @@ 'pages'.DIRECTORY_SEPARATOR.$t_action.'.php'; if ( !is_file( $t_page ) ) { - trigger_error( ERROR_PLUGIN_PAGE_NOT_FOUND, ERROR ); + trigger_error( ERROR_PLUGIN_PAGE_NOT_FOUND, ERROR ); } if( plugin_needs_upgrade( $g_plugin_cache[$t_basename] ) ) { @@ -69,4 +69,3 @@ plugin_push_current( $t_basename ); include( $t_page ); - From 355612e4b3826160177ba19e551535a259d59831 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Tue, 20 May 2014 09:51:38 +0200 Subject: [PATCH 03/20] plugin.php: replace DIRECTORY_SEPARATOR by '/' --- plugin.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin.php b/plugin.php index 965314f16f..67551bf29c 100644 --- a/plugin.php +++ b/plugin.php @@ -55,8 +55,7 @@ trigger_error( ERROR_PLUGIN_NOT_LOADED, ERROR ); } -$t_page = $t_plugin_path.$t_basename.DIRECTORY_SEPARATOR. - 'pages'.DIRECTORY_SEPARATOR.$t_action.'.php'; +$t_page = $t_plugin_path . $t_basename . '/pages/' . $t_action . '.php'; if ( !is_file( $t_page ) ) { trigger_error( ERROR_PLUGIN_PAGE_NOT_FOUND, ERROR ); From 221442c6f259b982941da77aed29dd5af7d8241c Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Thu, 22 May 2014 10:53:20 +0200 Subject: [PATCH 04/20] Move plugin upgrade needed check before loaded check Ensures we capture display appropriate error message to the user. Before that, they would get the 'not loaded, dependencies not met' message instead of the 'upgrade needed' one. --- plugin.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugin.php b/plugin.php index 67551bf29c..c116df243b 100644 --- a/plugin.php +++ b/plugin.php @@ -50,6 +50,11 @@ trigger_error( ERROR_PLUGIN_NOT_REGISTERED, ERROR ); } +if( plugin_needs_upgrade( $g_plugin_cache[$t_basename] ) ) { + error_parameters( $t_basename ); + trigger_error( ERROR_PLUGIN_UPGRADE_NEEDED, ERROR ); +} + # Plugin can be registered but fail to load e.g. due to unmet dependencies if( !plugin_is_loaded( $t_basename ) ) { trigger_error( ERROR_PLUGIN_NOT_LOADED, ERROR ); @@ -61,10 +66,5 @@ trigger_error( ERROR_PLUGIN_PAGE_NOT_FOUND, ERROR ); } -if( plugin_needs_upgrade( $g_plugin_cache[$t_basename] ) ) { - error_parameters( $t_basename ); - trigger_error( ERROR_PLUGIN_UPGRADE_NEEDED, ERROR ); -} - plugin_push_current( $t_basename ); include( $t_page ); From 256c1077c4252a71ea0ca74f4ccf3d1c173f2549 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Thu, 22 May 2014 11:08:50 +0200 Subject: [PATCH 05/20] Add plugin basename to error messages Helps users identifying what plugin is having issues. Fixes #17368 --- core/plugin_api.php | 1 + lang/strings_english.txt | 6 +++--- plugin.php | 2 ++ plugin_file.php | 1 + 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/core/plugin_api.php b/core/plugin_api.php index bc235df430..afb463995a 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -610,6 +610,7 @@ function plugin_install( $p_plugin ) { access_ensure_global_level( config_get_global( 'manage_plugin_threshold' ) ); if( plugin_is_installed( $p_plugin->basename ) ) { + error_parameters( $p_plugin->basename ); trigger_error( ERROR_PLUGIN_ALREADY_INSTALLED, WARNING ); return null; } diff --git a/lang/strings_english.txt b/lang/strings_english.txt index 435e3a4e12..d7a89d2c69 100644 --- a/lang/strings_english.txt +++ b/lang/strings_english.txt @@ -1667,9 +1667,9 @@ $MANTIS_ERROR[ERROR_TAG_NOT_ATTACHED] = 'That tag is not attached to that issue. $MANTIS_ERROR[ERROR_TAG_ALREADY_ATTACHED] = 'That tag already attached to that issue.'; $MANTIS_ERROR[ERROR_TOKEN_NOT_FOUND] = 'Token could not be found.'; $MANTIS_ERROR[ERROR_EVENT_UNDECLARED] = 'Event "%1$s" has not yet been declared.'; -$MANTIS_ERROR[ERROR_PLUGIN_NOT_REGISTERED] = 'Plugin is not registered with MantisBT.'; -$MANTIS_ERROR[ERROR_PLUGIN_NOT_LOADED] = 'Plugin is not loaded, make sure its dependencies are met.'; -$MANTIS_ERROR[ERROR_PLUGIN_ALREADY_INSTALLED] = 'Plugin is already installed.'; +$MANTIS_ERROR[ERROR_PLUGIN_NOT_REGISTERED] = 'Plugin "%1$s" is not registered with MantisBT.'; +$MANTIS_ERROR[ERROR_PLUGIN_NOT_LOADED] = 'Plugin "%1$s" is not loaded, make sure its dependencies are met.'; +$MANTIS_ERROR[ERROR_PLUGIN_ALREADY_INSTALLED] = 'Plugin "%1$s" is already installed.'; $MANTIS_ERROR[ERROR_PLUGIN_PAGE_NOT_FOUND] = 'Plugin page not found.'; $MANTIS_ERROR[ERROR_PLUGIN_INSTALL_FAILED] = 'Plugin installation failed: %1$s.'; $MANTIS_ERROR[ERROR_PLUGIN_UPGRADE_FAILED] = 'Upgrading the plugin schema failed in block #%1$s.'; diff --git a/plugin.php b/plugin.php index c116df243b..4e38c367b1 100644 --- a/plugin.php +++ b/plugin.php @@ -47,6 +47,7 @@ global $g_plugin_cache; if ( !isset( $g_plugin_cache[$t_basename] ) ) { + error_parameters( $t_basename ); trigger_error( ERROR_PLUGIN_NOT_REGISTERED, ERROR ); } @@ -57,6 +58,7 @@ # Plugin can be registered but fail to load e.g. due to unmet dependencies if( !plugin_is_loaded( $t_basename ) ) { + error_parameters( $t_basename ); trigger_error( ERROR_PLUGIN_NOT_LOADED, ERROR ); } diff --git a/plugin_file.php b/plugin_file.php index a8d47d8e97..996fbf0051 100644 --- a/plugin_file.php +++ b/plugin_file.php @@ -50,6 +50,7 @@ global $g_plugin_cache; if ( !isset( $g_plugin_cache[$t_basename] ) ) { + error_parameters( $t_basename ); trigger_error( ERROR_PLUGIN_NOT_REGISTERED, ERROR ); } From d992cf21f8dc8bbc92a9acf861a78bebb187224b Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Thu, 22 May 2014 15:05:03 +0200 Subject: [PATCH 06/20] More plugin error messages improvements - ERROR_PLUGIN_PAGE_NOT_FOUND: Add plugin basename and page name - ERROR_PLUGIN_FILE_NOT_FOUND: new message based on above - New specific messages for invalid page and file specification (ERROR_PLUGIN_INVALID_PAGE and ERROR_PLUGIN_INVALID_FILE) instead of using ERROR_GENERIC Fixes #17368 --- core/constant_inc.php | 3 +++ core/plugin_api.php | 3 ++- lang/strings_english.txt | 5 ++++- plugin.php | 4 +++- plugin_file.php | 3 ++- 5 files changed, 14 insertions(+), 4 deletions(-) diff --git a/core/constant_inc.php b/core/constant_inc.php index f11d28151f..d8e4aa8a95 100644 --- a/core/constant_inc.php +++ b/core/constant_inc.php @@ -384,6 +384,9 @@ define( 'ERROR_PLUGIN_INSTALL_FAILED', 2504 ); define( 'ERROR_PLUGIN_UPGRADE_NEEDED', 2505 ); define( 'ERROR_PLUGIN_NOT_LOADED', 2506 ); +define( 'ERROR_PLUGIN_INVALID_PAGE', 2507 ); +define( 'ERROR_PLUGIN_INVALID_FILE', 2508 ); +define( 'ERROR_PLUGIN_FILE_NOT_FOUND', 2509 ); define( 'ERROR_PLUGIN_GENERIC', 2599 ); # ERROR_COLUMNS_* diff --git a/core/plugin_api.php b/core/plugin_api.php index afb463995a..0284c43ef5 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -153,7 +153,8 @@ function plugin_file_include( $p_filename, $p_basename = null ) { $t_file_path = plugin_file_path( $p_filename, $t_current ); if( false === $t_file_path ) { - trigger_error( ERROR_GENERIC, ERROR ); + error_parameters( $t_current, $p_filename ); + trigger_error( ERROR_PLUGIN_FILE_NOT_FOUND, ERROR ); } $t_content_type = ''; diff --git a/lang/strings_english.txt b/lang/strings_english.txt index d7a89d2c69..6063b16746 100644 --- a/lang/strings_english.txt +++ b/lang/strings_english.txt @@ -1670,10 +1670,13 @@ $MANTIS_ERROR[ERROR_EVENT_UNDECLARED] = 'Event "%1$s" has not yet been declared. $MANTIS_ERROR[ERROR_PLUGIN_NOT_REGISTERED] = 'Plugin "%1$s" is not registered with MantisBT.'; $MANTIS_ERROR[ERROR_PLUGIN_NOT_LOADED] = 'Plugin "%1$s" is not loaded, make sure its dependencies are met.'; $MANTIS_ERROR[ERROR_PLUGIN_ALREADY_INSTALLED] = 'Plugin "%1$s" is already installed.'; -$MANTIS_ERROR[ERROR_PLUGIN_PAGE_NOT_FOUND] = 'Plugin page not found.'; +$MANTIS_ERROR[ERROR_PLUGIN_PAGE_NOT_FOUND] = 'Page "%2$s" does not exist in Plugin "%1$s".'; +$MANTIS_ERROR[ERROR_PLUGIN_FILE_NOT_FOUND] = 'File "%2$s" does not exist in Plugin "%1$s".'; $MANTIS_ERROR[ERROR_PLUGIN_INSTALL_FAILED] = 'Plugin installation failed: %1$s.'; $MANTIS_ERROR[ERROR_PLUGIN_UPGRADE_FAILED] = 'Upgrading the plugin schema failed in block #%1$s.'; $MANTIS_ERROR[ERROR_PLUGIN_UPGRADE_NEEDED] = 'The "%1$s" plugin needs to be upgraded before you can access this page.'; +$MANTIS_ERROR[ERROR_PLUGIN_INVALID_PAGE] = 'The format of the specified plugin page "%1$s" is invalid. It must match "Plugin[/path/to]/page".'; +$MANTIS_ERROR[ERROR_PLUGIN_INVALID_FILE] = 'The format of the specified plugin file "%1$s" is invalid. It must match "Plugin[/path/to]/file[.ext]".'; $MANTIS_ERROR[ERROR_PLUGIN_GENERIC] = 'There was an unknown error "%1$s" during execution of the "%2$s" plugin.'; $MANTIS_ERROR[ERROR_COLUMNS_DUPLICATE] = 'Field "%1$s" contains duplicate column "%2$s".'; $MANTIS_ERROR[ERROR_COLUMNS_INVALID] = 'Field "%1$s" contains invalid field "%2$s".'; diff --git a/plugin.php b/plugin.php index 4e38c367b1..8be9db4a5c 100644 --- a/plugin.php +++ b/plugin.php @@ -39,7 +39,8 @@ $t_matches = array(); if ( !preg_match( '/^([a-zA-Z0-9_-]+)\/([a-zA-Z0-9_-]+[\/a-zA-Z0-9_-]*)/', $f_page, $t_matches ) ) { - trigger_error( ERROR_GENERIC, ERROR ); + error_parameters( $f_page ); + trigger_error( ERROR_PLUGIN_INVALID_PAGE, ERROR ); } $t_basename = $t_matches[1]; @@ -65,6 +66,7 @@ $t_page = $t_plugin_path . $t_basename . '/pages/' . $t_action . '.php'; if ( !is_file( $t_page ) ) { + error_parameters( $t_basename, $t_action ); trigger_error( ERROR_PLUGIN_PAGE_NOT_FOUND, ERROR ); } diff --git a/plugin_file.php b/plugin_file.php index 996fbf0051..cbdfe6ebe0 100644 --- a/plugin_file.php +++ b/plugin_file.php @@ -42,7 +42,8 @@ $t_matches = array(); if ( !preg_match( '/^([a-zA-Z0-9_-]+)\/([a-zA-Z0-9_-]+[\/a-zA-Z0-9_-]*\.?[a-zA-Z0-9_-]*)/', $f_file, $t_matches ) ) { - trigger_error( ERROR_GENERIC, ERROR ); + error_parameters( $f_file ); + trigger_error( ERROR_PLUGIN_INVALID_FILE, ERROR ); } $t_basename = $t_matches[1]; From da2cdabfd918aaf398c22c5dee2239bb65283641 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Thu, 22 May 2014 15:17:37 +0200 Subject: [PATCH 07/20] Plugin API: consistent handling of MantisCore pseudo-plugin - return true and not 1 if we declare return type to be boolean - plugin_is_installed() returns true for MantisCore since by definition it is always installed --- core/plugin_api.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/plugin_api.php b/core/plugin_api.php index 0284c43ef5..89555ed01a 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -558,9 +558,9 @@ function plugin_dependency( $p_base_name, $p_required, $p_initialized = false ) function plugin_protected( $p_base_name ) { global $g_plugin_cache_protected; - # For pseudo-plugin MantisCore, return protected as 1. + # Pseudo-plugin MantisCore is protected if( $p_base_name == 'MantisCore' ) { - return 1; + return true; } return $g_plugin_cache_protected[$p_base_name]; @@ -588,6 +588,11 @@ function plugin_priority( $p_base_name ) { * @return bool True if plugin is installed */ function plugin_is_installed( $p_basename ) { + # Pseudo-plugin MantisCore is always installed + if( $p_basename == 'MantisCore' ) { + return true; + } + $t_plugin_table = db_get_table( 'plugin' ); $t_forced_plugins = config_get_global( 'plugins_force_installed' ); From fb68df7f3576bb36d9df192b551b61db8b126409 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Thu, 22 May 2014 15:19:33 +0200 Subject: [PATCH 08/20] Remove direct access to $g_plugin_cache global variable Introduces 2 new API functions - plugin_is_registered() - plugin_get() Fixes #17366 --- core/plugin_api.php | 34 ++++++++++++++++++++++++++++++++++ manage_plugin_page.php | 4 +--- plugin.php | 8 ++------ plugin_file.php | 7 +------ 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/core/plugin_api.php b/core/plugin_api.php index 89555ed01a..9b3711f419 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -82,6 +82,29 @@ function plugin_pop_current() { return( isset( $g_plugin_current[0] ) ? array_shift( $g_plugin_current ) : null ); } +/** + * Returns an object representing the specified plugin + * Triggers an error if the plugin is not registered + * @param string|null $p_basename Plugin base name (defaults to current plugin) + * @return object Plugin Object + */ +function plugin_get( $p_basename = null ) { + global $g_plugin_cache; + + if( is_null( $p_basename ) ) { + $t_current = plugin_get_current(); + } else { + $t_current = $p_basename; + } + + if ( !plugin_is_registered( $t_current ) ) { + error_parameters( $t_current ); + trigger_error( ERROR_PLUGIN_NOT_REGISTERED, ERROR ); + } + + return $g_plugin_cache[$p_basename]; +} + /** * Get the URL to the plugin wrapper page. * @param string $p_page Page name @@ -822,6 +845,17 @@ function plugin_require_api( $p_file, $p_basename = null ) { require_once( $t_path . $p_file ); } +/** + * Determine if a given plugin is registered. + * @param string $p_basename Plugin basename + * @return boolean True if plugin is registered + */ +function plugin_is_registered( $p_basename ) { + global $g_plugin_cache; + + return isset( $g_plugin_cache[$p_basename] ); +} + /** * Register a plugin with MantisBT. * The plugin class must already be loaded before calling. diff --git a/manage_plugin_page.php b/manage_plugin_page.php index 2a2cc162a1..645ca96f00 100644 --- a/manage_plugin_page.php +++ b/manage_plugin_page.php @@ -68,14 +68,12 @@ function plugin_sort( $p_plugin1, $p_plugin2 ) { $t_plugins = plugin_find_all(); uasort( $t_plugins, 'plugin_sort' ); -global $g_plugin_cache; - $t_plugins_installed = array(); $t_plugins_available = array(); $t_forced_plugins = config_get_global( 'plugins_force_installed' ); foreach( $t_plugins as $t_basename => $t_plugin ) { - if ( isset( $g_plugin_cache[$t_basename] ) ) { + if( plugin_is_registered( $t_basename ) ) { $t_plugins_installed[$t_basename] = $t_plugin; } else { $t_plugins_available[$t_basename] = $t_plugin; diff --git a/plugin.php b/plugin.php index 8be9db4a5c..56faec4be9 100644 --- a/plugin.php +++ b/plugin.php @@ -46,13 +46,9 @@ $t_basename = $t_matches[1]; $t_action = $t_matches[2]; -global $g_plugin_cache; -if ( !isset( $g_plugin_cache[$t_basename] ) ) { - error_parameters( $t_basename ); - trigger_error( ERROR_PLUGIN_NOT_REGISTERED, ERROR ); -} +$t_plugin = plugin_get( $t_basename ); -if( plugin_needs_upgrade( $g_plugin_cache[$t_basename] ) ) { +if( plugin_needs_upgrade( $t_plugin ) ) { error_parameters( $t_basename ); trigger_error( ERROR_PLUGIN_UPGRADE_NEEDED, ERROR ); } diff --git a/plugin_file.php b/plugin_file.php index cbdfe6ebe0..87c852973a 100644 --- a/plugin_file.php +++ b/plugin_file.php @@ -49,11 +49,6 @@ $t_basename = $t_matches[1]; $t_file = $t_matches[2]; -global $g_plugin_cache; -if ( !isset( $g_plugin_cache[$t_basename] ) ) { - error_parameters( $t_basename ); - trigger_error( ERROR_PLUGIN_NOT_REGISTERED, ERROR ); -} +$t_plugin = plugin_get( $t_basename ); plugin_file_include( $t_file, $t_basename ); - From f7a5cd8a87242eb8686c729072902938d1138006 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Thu, 22 May 2014 15:28:33 +0200 Subject: [PATCH 09/20] Code cleanup and whitespace - Use Closure instead of defining a function to sort plugins - Remove unnecessary variable assignments --- core/plugin_api.php | 4 ++-- manage_plugin_page.php | 16 +++++----------- plugin.php | 5 ++--- plugin_file.php | 3 +-- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/core/plugin_api.php b/core/plugin_api.php index 9b3711f419..a77f169d24 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -166,7 +166,7 @@ function plugin_file( $p_file, $p_redirect = false, $p_base_name = null ) { */ function plugin_file_include( $p_filename, $p_basename = null ) { - global $g_plugin_mime_types; + global $g_plugin_mime_types; if( is_null( $p_basename ) ) { $t_current = plugin_get_current(); @@ -200,7 +200,7 @@ function plugin_file_include( $p_filename, $p_basename = null ) { } if ( $t_content_type ) - header('Content-Type: ' . $t_content_type ); + header('Content-Type: ' . $t_content_type ); readfile( $t_file_path ); } diff --git a/manage_plugin_page.php b/manage_plugin_page.php index 645ca96f00..7526598156 100644 --- a/manage_plugin_page.php +++ b/manage_plugin_page.php @@ -55,18 +55,12 @@ print_manage_menu( 'manage_plugin_page.php' ); -/** - * Sort Plugins by name - * @param MantisPlugin $p_plugin1 Plugin 1 - * @param MantisPlugin $p_plugin2 Plugin 2 - * @return string - */ -function plugin_sort( $p_plugin1, $p_plugin2 ) { - return strcasecmp( $p_plugin1->name, $p_plugin2->name ); -} - $t_plugins = plugin_find_all(); -uasort( $t_plugins, 'plugin_sort' ); +uasort( $t_plugins, + function ( $p1, $p2 ) { + return strcasecmp( $p1->name, $p2->name ); + } +); $t_plugins_installed = array(); $t_plugins_available = array(); diff --git a/plugin.php b/plugin.php index 56faec4be9..222ada93e3 100644 --- a/plugin.php +++ b/plugin.php @@ -35,10 +35,9 @@ $t_plugin_path = config_get( 'plugin_path' ); -$f_page= gpc_get_string( 'page' ); -$t_matches = array(); +$f_page = gpc_get_string( 'page' ); -if ( !preg_match( '/^([a-zA-Z0-9_-]+)\/([a-zA-Z0-9_-]+[\/a-zA-Z0-9_-]*)/', $f_page, $t_matches ) ) { +if( !preg_match( '/^([a-zA-Z0-9_-]+)\/([a-zA-Z0-9_-]+[\/a-zA-Z0-9_-]*)/', $f_page, $t_matches ) ) { error_parameters( $f_page ); trigger_error( ERROR_PLUGIN_INVALID_PAGE, ERROR ); } diff --git a/plugin_file.php b/plugin_file.php index 87c852973a..376a7491f0 100644 --- a/plugin_file.php +++ b/plugin_file.php @@ -39,9 +39,8 @@ $t_plugin_path = config_get( 'plugin_path' ); $f_file = gpc_get_string( 'file' ); -$t_matches = array(); -if ( !preg_match( '/^([a-zA-Z0-9_-]+)\/([a-zA-Z0-9_-]+[\/a-zA-Z0-9_-]*\.?[a-zA-Z0-9_-]*)/', $f_file, $t_matches ) ) { +if( !preg_match( '/^([a-zA-Z0-9_-]+)\/([a-zA-Z0-9_-]+[\/a-zA-Z0-9_-]*\.?[a-zA-Z0-9_-]*)/', $f_file, $t_matches ) ) { error_parameters( $f_file ); trigger_error( ERROR_PLUGIN_INVALID_FILE, ERROR ); } From 3262cca56b1bba6ba2b613671e99802ad8cc7f8a Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Thu, 22 May 2014 16:57:30 +0200 Subject: [PATCH 10/20] Revise plugin file regex to allow multiple '.' in filename Allows inclusion of plugin files named like file.name.ext --- plugin_file.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin_file.php b/plugin_file.php index 376a7491f0..e4dbd0b2df 100644 --- a/plugin_file.php +++ b/plugin_file.php @@ -40,7 +40,7 @@ $f_file = gpc_get_string( 'file' ); -if( !preg_match( '/^([a-zA-Z0-9_-]+)\/([a-zA-Z0-9_-]+[\/a-zA-Z0-9_-]*\.?[a-zA-Z0-9_-]*)/', $f_file, $t_matches ) ) { +if( !preg_match( '/^([a-zA-Z0-9_-]+)\/((?:(?:[a-zA-Z0-9_-]+\/)*)(?:[.a-zA-Z0-9_-]+))/', $f_file, $t_matches ) ) { error_parameters( $f_file ); trigger_error( ERROR_PLUGIN_INVALID_FILE, ERROR ); } From 41042ba04fa4fd66d5692b25972d68ce7691217d Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Tue, 27 May 2014 08:33:15 +0200 Subject: [PATCH 11/20] Remove "with MantisBT" from ERROR_PLUGIN_NOT_REGISTERED --- lang/strings_english.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lang/strings_english.txt b/lang/strings_english.txt index 6063b16746..dd0d68bc46 100644 --- a/lang/strings_english.txt +++ b/lang/strings_english.txt @@ -1667,7 +1667,7 @@ $MANTIS_ERROR[ERROR_TAG_NOT_ATTACHED] = 'That tag is not attached to that issue. $MANTIS_ERROR[ERROR_TAG_ALREADY_ATTACHED] = 'That tag already attached to that issue.'; $MANTIS_ERROR[ERROR_TOKEN_NOT_FOUND] = 'Token could not be found.'; $MANTIS_ERROR[ERROR_EVENT_UNDECLARED] = 'Event "%1$s" has not yet been declared.'; -$MANTIS_ERROR[ERROR_PLUGIN_NOT_REGISTERED] = 'Plugin "%1$s" is not registered with MantisBT.'; +$MANTIS_ERROR[ERROR_PLUGIN_NOT_REGISTERED] = 'Plugin "%1$s" is not registered.'; $MANTIS_ERROR[ERROR_PLUGIN_NOT_LOADED] = 'Plugin "%1$s" is not loaded, make sure its dependencies are met.'; $MANTIS_ERROR[ERROR_PLUGIN_ALREADY_INSTALLED] = 'Plugin "%1$s" is already installed.'; $MANTIS_ERROR[ERROR_PLUGIN_PAGE_NOT_FOUND] = 'Page "%2$s" does not exist in Plugin "%1$s".'; From b8a4d7c27810d28baa3442665315703e4e1bb37a Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Fri, 30 May 2014 23:36:31 +0200 Subject: [PATCH 12/20] Do not register previously registered plugins It may happen that a plugin listed as enabled in the plugin table is in fact already registered with MantisBT if that plugin is also listed as force_installed in the configuration. In that case, re-registering it basically 'unprotects' it, and manage_plugin_page.php displays an Uninstall action even though the plugin can't actually be uninstalled. --- core/plugin_api.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/plugin_api.php b/core/plugin_api.php index a77f169d24..8aebf5b62e 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -928,9 +928,11 @@ function plugin_register_installed() { while( $t_row = db_fetch_array( $t_result ) ) { $t_basename = $t_row['basename']; - plugin_register( $t_basename ); - $g_plugin_cache_priority[$t_basename] = $t_row['priority']; - $g_plugin_cache_protected[$t_basename] = $t_row['protected']; + if( !plugin_is_registered( $t_basename ) ) { + plugin_register( $t_basename ); + $g_plugin_cache_priority[$t_basename] = $t_row['priority']; + $g_plugin_cache_protected[$t_basename] = $t_row['protected']; + } } } From 08f126be887b51cffec140fff367c17b2852e36c Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Fri, 30 May 2014 23:42:58 +0200 Subject: [PATCH 13/20] Fix data type issues in plugin API - enabled and protected are boolean - priority is int The type cast before assignment ensures we don't get type mismatch errors in check_selected/check_checked functions, since DB api seems to return these fields as strings. --- core/plugin_api.php | 6 +++--- core/print_api.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/plugin_api.php b/core/plugin_api.php index 8aebf5b62e..80a631fd2c 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -924,14 +924,14 @@ function plugin_register_installed() { $t_plugin_table = db_get_table( 'plugin' ); $t_query = "SELECT basename, priority, protected FROM $t_plugin_table WHERE enabled=" . db_param() . ' ORDER BY priority DESC'; - $t_result = db_query_bound( $t_query, array( 1 ) ); + $t_result = db_query_bound( $t_query, array( true ) ); while( $t_row = db_fetch_array( $t_result ) ) { $t_basename = $t_row['basename']; if( !plugin_is_registered( $t_basename ) ) { plugin_register( $t_basename ); - $g_plugin_cache_priority[$t_basename] = $t_row['priority']; - $g_plugin_cache_protected[$t_basename] = $t_row['protected']; + $g_plugin_cache_priority[$t_basename] = (int)$t_row['priority']; + $g_plugin_cache_protected[$t_basename] = (bool)$t_row['protected']; } } } diff --git a/core/print_api.php b/core/print_api.php index 463631d56e..7724d2c621 100644 --- a/core/print_api.php +++ b/core/print_api.php @@ -1152,7 +1152,7 @@ function print_plugin_priority_list( $p_priority ) { } for( $i = 5;$i >= 1;$i-- ) { - echo ''; + echo ''; } } From 3da96c65830184f2c843c9b80a3357d3ecfde4d6 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Sat, 31 May 2014 00:12:40 +0200 Subject: [PATCH 14/20] New API to return list of force-installed plugins --- core/plugin_api.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/core/plugin_api.php b/core/plugin_api.php index 80a631fd2c..76238e760f 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -82,6 +82,20 @@ function plugin_pop_current() { return( isset( $g_plugin_current[0] ) ? array_shift( $g_plugin_current ) : null ); } +/** + * Returns the list of force-installed plugins + * @see $g_plugins_force_installed + * @return array List of plugins (basename => priority) + */ +function plugin_get_force_installed() { + $t_forced_plugins = config_get_global( 'plugins_force_installed' ); + + # MantisCore pseudo-plugin is force-installed by definition, with priority 3 + $t_forced_plugins['MantisCore'] = 3; + + return $t_forced_plugins; +} + /** * Returns an object representing the specified plugin * Triggers an error if the plugin is not registered From c892b7f4e21b6f9e7a433b7c89c4287ce3f23379 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Sat, 31 May 2014 00:14:22 +0200 Subject: [PATCH 15/20] Remove unnecessary MantisCore-specific code Using the new plugin_get_force_installed() function (which always returns 'MantisCore') allows simplification of several plugin API, as they no longer require specific code to handle the pseudo plugin. --- core/plugin_api.php | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/core/plugin_api.php b/core/plugin_api.php index 76238e760f..cd6c9999a9 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -595,11 +595,6 @@ function plugin_dependency( $p_base_name, $p_required, $p_initialized = false ) function plugin_protected( $p_base_name ) { global $g_plugin_cache_protected; - # Pseudo-plugin MantisCore is protected - if( $p_base_name == 'MantisCore' ) { - return true; - } - return $g_plugin_cache_protected[$p_base_name]; } @@ -611,11 +606,6 @@ function plugin_protected( $p_base_name ) { function plugin_priority( $p_base_name ) { global $g_plugin_cache_priority; - # For pseudo-plugin MantisCore, return priority as 3. - if( $p_base_name == 'MantisCore' ) { - return 3; - } - return $g_plugin_cache_priority[$p_base_name]; } @@ -625,20 +615,13 @@ function plugin_priority( $p_base_name ) { * @return bool True if plugin is installed */ function plugin_is_installed( $p_basename ) { - # Pseudo-plugin MantisCore is always installed - if( $p_basename == 'MantisCore' ) { - return true; - } - - $t_plugin_table = db_get_table( 'plugin' ); - - $t_forced_plugins = config_get_global( 'plugins_force_installed' ); - foreach( $t_forced_plugins as $t_basename => $t_priority ) { + foreach( plugin_get_force_installed() as $t_basename => $t_priority ) { if ( $t_basename == $p_basename ) { return true; } } + $t_plugin_table = db_get_table( 'plugin' ); $t_query = "SELECT COUNT(*) FROM $t_plugin_table WHERE basename=" . db_param(); $t_result = db_query_bound( $t_query, array( $p_basename ) ); return( 0 < db_result( $t_result ) ); @@ -927,8 +910,7 @@ function plugin_register_installed() { global $g_plugin_cache_priority, $g_plugin_cache_protected; # register plugins specified in the site configuration - $t_forced_plugins = config_get_global( 'plugins_force_installed' ); - foreach( $t_forced_plugins as $t_basename => $t_priority ) { + foreach( plugin_get_force_installed() as $t_basename => $t_priority ) { plugin_register( $t_basename ); $g_plugin_cache_priority[$t_basename] = $t_priority; $g_plugin_cache_protected[$t_basename] = true; @@ -966,7 +948,7 @@ function plugin_init_installed() { $g_plugin_cache_priority = array(); $g_plugin_cache_protected = array(); - plugin_register( 'MantisCore' ); +# plugin_register( 'MantisCore' ); plugin_register_installed(); $t_plugins = array_keys( $g_plugin_cache ); From 44f6f69a7eca169850dd99cb0f9bcb5a36d722a2 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Sat, 31 May 2014 00:19:43 +0200 Subject: [PATCH 16/20] Simplify code to list installed plugins --- manage_plugin_page.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/manage_plugin_page.php b/manage_plugin_page.php index 7526598156..7d9cc48256 100644 --- a/manage_plugin_page.php +++ b/manage_plugin_page.php @@ -64,7 +64,6 @@ function ( $p1, $p2 ) { $t_plugins_installed = array(); $t_plugins_available = array(); -$t_forced_plugins = config_get_global( 'plugins_force_installed' ); foreach( $t_plugins as $t_basename => $t_plugin ) { if( plugin_is_registered( $t_basename ) ) { @@ -113,7 +112,6 @@ function ( $p1, $p2 ) { $t_url = $t_plugin->url; $t_requires = $t_plugin->requires; $t_depends = array(); - $t_forced = isset( $t_forced_plugins[ $t_basename ] ); $t_priority = plugin_priority( $t_basename ); $t_protected = plugin_protected( $t_basename ); @@ -139,7 +137,6 @@ function ( $p1, $p2 ) { } $t_upgrade = plugin_needs_upgrade( $t_plugin ); - $t_uninstall = ( 'MantisCore' != $t_basename && !$t_protected ); if ( is_array( $t_requires ) ) { foreach( $t_requires as $t_plugin => $t_version ) { @@ -170,16 +167,20 @@ function ( $p1, $p2 ) { echo '',$t_depends,''; if ( 'MantisCore' == $t_basename ) { echo '  '; - } else if ( $t_forced ) { - echo '','',''; - echo '','',''; } else { - echo '','',''; - echo '','',''; + echo '', + '',''; + echo '', + '', + '',''; } echo ''; if ( $t_upgrade ) { print_bracket_link( 'manage_plugin_upgrade.php?name=' . $t_basename . form_security_param( 'manage_plugin_upgrade' ), lang_get( 'plugin_upgrade' ) ); } - if ( $t_uninstall ) { print_bracket_link( 'manage_plugin_uninstall.php?name=' . $t_basename . form_security_param( 'manage_plugin_uninstall' ), lang_get( 'plugin_uninstall' ) ); } + if ( !$t_protected ) { print_bracket_link( 'manage_plugin_uninstall.php?name=' . $t_basename . form_security_param( 'manage_plugin_uninstall' ), lang_get( 'plugin_uninstall' ) ); } echo ''; } ?> From ee3e36c80db3521c710dcaa1b4a61ebfad0bddc2 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Sat, 31 May 2014 00:22:58 +0200 Subject: [PATCH 17/20] Coding guidelines --- manage_plugin_page.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/manage_plugin_page.php b/manage_plugin_page.php index 7d9cc48256..544c807aaa 100644 --- a/manage_plugin_page.php +++ b/manage_plugin_page.php @@ -179,8 +179,18 @@ function ( $p1, $p2 ) { '',''; } echo ''; - if ( $t_upgrade ) { print_bracket_link( 'manage_plugin_upgrade.php?name=' . $t_basename . form_security_param( 'manage_plugin_upgrade' ), lang_get( 'plugin_upgrade' ) ); } - if ( !$t_protected ) { print_bracket_link( 'manage_plugin_uninstall.php?name=' . $t_basename . form_security_param( 'manage_plugin_uninstall' ), lang_get( 'plugin_uninstall' ) ); } + if( $t_upgrade ) { + print_bracket_link( + 'manage_plugin_upgrade.php?name=' . $t_basename . form_security_param( 'manage_plugin_upgrade' ), + lang_get( 'plugin_upgrade' ) + ); + } + if( !$t_protected ) { + print_bracket_link( + 'manage_plugin_uninstall.php?name=' . $t_basename . form_security_param( 'manage_plugin_uninstall' ), + lang_get( 'plugin_uninstall' ) + ); + } echo ''; } ?> @@ -279,7 +289,12 @@ function ( $p1, $p2 ) { echo '',$t_description,$t_author,$t_url,''; echo '',$t_depends,''; echo ''; - if ( $t_ready ) { print_bracket_link( 'manage_plugin_install.php?name=' . $t_basename . form_security_param( 'manage_plugin_install' ), lang_get( 'plugin_install' ) ); } + if( $t_ready ) { + print_bracket_link( + 'manage_plugin_install.php?name=' . $t_basename . form_security_param( 'manage_plugin_install' ), + lang_get( 'plugin_install' ) + ); + } echo ''; } ?> From 4f61786bd50044ddea6a33c52756020d8474f030 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Sat, 31 May 2014 01:04:26 +0200 Subject: [PATCH 18/20] Improve plugin path regex to avoid arbitrary includes The path and filename components are not allowed to start with a '.', which prevents uses like ../../../etc/password --- plugin_file.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/plugin_file.php b/plugin_file.php index e4dbd0b2df..60fd8edefb 100644 --- a/plugin_file.php +++ b/plugin_file.php @@ -40,7 +40,16 @@ $f_file = gpc_get_string( 'file' ); -if( !preg_match( '/^([a-zA-Z0-9_-]+)\/((?:(?:[a-zA-Z0-9_-]+\/)*)(?:[.a-zA-Z0-9_-]+))/', $f_file, $t_matches ) ) { +$t_regex = '/^' + # File must start with plugin name, ending with / + . '([a-zA-Z0-9_-]+)\/' + # Path must not start with a '.' to avoid arbitrary includes higher in the file system + . '('. '(?:(?:[a-zA-Z0-9_-][.a-zA-Z0-9_-]*\/)*)' + # Same goes for filename + . '(?:[a-zA-Z0-9_-][.a-zA-Z0-9_-]*)' + . ')$/'; + +if( !preg_match( $t_regex, $f_file, $t_matches ) ) { error_parameters( $f_file ); trigger_error( ERROR_PLUGIN_INVALID_FILE, ERROR ); } From 62811370fb44c2bb71f7d663499839655861c574 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Sat, 31 May 2014 01:08:58 +0200 Subject: [PATCH 19/20] Remove leftover commented out line --- core/plugin_api.php | 1 - 1 file changed, 1 deletion(-) diff --git a/core/plugin_api.php b/core/plugin_api.php index cd6c9999a9..be5fa6fb0c 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -948,7 +948,6 @@ function plugin_init_installed() { $g_plugin_cache_priority = array(); $g_plugin_cache_protected = array(); -# plugin_register( 'MantisCore' ); plugin_register_installed(); $t_plugins = array_keys( $g_plugin_cache ); From 14a095938f53f536538c04e1fcac903d52298208 Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Sat, 31 May 2014 01:11:23 +0200 Subject: [PATCH 20/20] Document MantisCore inclusion in plugin_register_installed() --- core/plugin_api.php | 1 + 1 file changed, 1 insertion(+) diff --git a/core/plugin_api.php b/core/plugin_api.php index be5fa6fb0c..76f8ffe320 100644 --- a/core/plugin_api.php +++ b/core/plugin_api.php @@ -905,6 +905,7 @@ function plugin_register( $p_basename, $p_return = false, $p_child = null ) { /** * Find and register all installed plugins. + * This includes the MantisCore pseudo-plugin. */ function plugin_register_installed() { global $g_plugin_cache_priority, $g_plugin_cache_protected;