diff options
22 files changed, 245 insertions, 36 deletions
diff --git a/phpBB/common.php b/phpBB/common.php index c33e2cbb1f..6dd65739fc 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -44,8 +44,11 @@ if (!defined('PHPBB_INSTALLED')) // Replace any number of consecutive backslashes and/or slashes with a single slash // (could happen on some proxy setups and/or Windows servers) $script_path = preg_replace('#[\\\\/]{2,}#', '/', $script_path); + // Eliminate . and .. from the path - $script_path = phpbb_clean_path($script_path); + require($phpbb_root_path . 'includes/filesystem.' . $phpEx); + $phpbb_filesystem = new phpbb_filesystem(); + $script_path = $phpbb_filesystem->clean_path($script_path); $url = (($secure) ? 'https://' : 'http://') . $server_name; diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index b9c71844dc..3b60f0e83e 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -131,6 +131,7 @@ services: - @dbal.conn - @config - @migrator + - @filesystem - %tables.ext% - %core.root_path% - .%core.php_ext% @@ -140,11 +141,15 @@ services: class: phpbb_extension_finder arguments: - @ext.manager + - @filesystem - %core.root_path% - @cache.driver - .%core.php_ext% - _ext_finder + filesystem: + class: phpbb_filesystem + groupposition.legend: class: phpbb_groupposition_legend arguments: @@ -242,6 +247,7 @@ services: arguments: - @ext.manager - @style.path_provider + - %core.root_path% style.path_provider: class: phpbb_style_path_provider diff --git a/phpBB/includes/bbcode.php b/phpBB/includes/bbcode.php index e8681420d4..c198abeb54 100644 --- a/phpBB/includes/bbcode.php +++ b/phpBB/includes/bbcode.php @@ -133,7 +133,7 @@ class bbcode $this->template_bitfield = new bitfield($user->style['bbcode_bitfield']); $style_resource_locator = new phpbb_style_resource_locator(); - $style_path_provider = new phpbb_style_extension_path_provider($phpbb_extension_manager, new phpbb_style_path_provider()); + $style_path_provider = new phpbb_style_extension_path_provider($phpbb_extension_manager, new phpbb_style_path_provider(), $phpbb_root_path); $template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator, new phpbb_template_context(), $phpbb_extension_manager); $style = new phpbb_style($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator, $style_path_provider, $template); $style->set_style(); diff --git a/phpBB/includes/db/migration/data/310/style_update_p1.php b/phpBB/includes/db/migration/data/310/style_update_p1.php index e324ce7f24..d43537559d 100644 --- a/phpBB/includes/db/migration/data/310/style_update_p1.php +++ b/phpBB/includes/db/migration/data/310/style_update_p1.php @@ -19,6 +19,34 @@ class phpbb_db_migration_data_310_style_update_p1 extends phpbb_db_migration return array('phpbb_db_migration_data_30x_3_0_11'); } + public function update_schema() + { + return array( + 'add_columns' => array( + $this->table_prefix . 'styles' => array( + 'style_path' => array('VCHAR:100', ''), + 'bbcode_bitfield' => array('VCHAR:255', 'kNg='), + 'style_parent_id' => array('UINT', 0), + 'style_parent_tree' => array('TEXT', ''), + ), + ), + ); + } + + public function revert_schema() + { + return array( + 'drop_columns' => array( + $this->table_prefix . 'styles' => array( + 'style_path', + 'bbcode_bitfield', + 'style_parent_id', + 'style_parent_tree', + ), + ), + ); + } + public function update_data() { return array( diff --git a/phpBB/includes/extension/finder.php b/phpBB/includes/extension/finder.php index f71e32bc8d..02a9ebb8c3 100644 --- a/phpBB/includes/extension/finder.php +++ b/phpBB/includes/extension/finder.php @@ -23,6 +23,7 @@ if (!defined('IN_PHPBB')) class phpbb_extension_finder { protected $extension_manager; + protected $filesystem; protected $phpbb_root_path; protected $cache; protected $php_ext; @@ -54,15 +55,17 @@ class phpbb_extension_finder * @param phpbb_extension_manager $extension_manager An extension manager * instance that provides the finder with a list of active * extensions and their locations + * @param phpbb_filesystem $filesystem Filesystem instance * @param string $phpbb_root_path Path to the phpbb root directory * @param phpbb_cache_driver_interface $cache A cache instance or null * @param string $php_ext php file extension * @param string $cache_name The name of the cache variable, defaults to * _ext_finder */ - public function __construct(phpbb_extension_manager $extension_manager, $phpbb_root_path = '', phpbb_cache_driver_interface $cache = null, $php_ext = '.php', $cache_name = '_ext_finder') + public function __construct(phpbb_extension_manager $extension_manager, phpbb_filesystem $filesystem, $phpbb_root_path = '', phpbb_cache_driver_interface $cache = null, $php_ext = '.php', $cache_name = '_ext_finder') { $this->extension_manager = $extension_manager; + $this->filesystem = $filesystem; $this->phpbb_root_path = $phpbb_root_path; $this->cache = $cache; $this->php_ext = $php_ext; @@ -227,7 +230,7 @@ class phpbb_extension_finder */ protected function sanitise_directory($directory) { - $directory = preg_replace('#(?:^|/)\./#', '/', $directory); + $directory = $this->filesystem->clean_path($directory); $dir_len = strlen($directory); if ($dir_len > 1 && $directory[$dir_len - 1] === '/') diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php index 44a30c6280..de9a3937c3 100644 --- a/phpBB/includes/extension/manager.php +++ b/phpBB/includes/extension/manager.php @@ -44,13 +44,14 @@ class phpbb_extension_manager * @param phpbb_db_driver $db A database connection * @param phpbb_config $config phpbb_config * @param phpbb_db_migrator $migrator + * @param phpbb_filesystem $filesystem * @param string $extension_table The name of the table holding extensions * @param string $phpbb_root_path Path to the phpbb includes directory. * @param string $php_ext php file extension * @param phpbb_cache_driver_interface $cache A cache instance or null * @param string $cache_name The name of the cache variable, defaults to _ext */ - public function __construct(ContainerInterface $container, phpbb_db_driver $db, phpbb_config $config, phpbb_db_migrator $migrator, $extension_table, $phpbb_root_path, $php_ext = '.php', phpbb_cache_driver_interface $cache = null, $cache_name = '_ext') + public function __construct(ContainerInterface $container, phpbb_db_driver $db, phpbb_config $config, phpbb_db_migrator $migrator, phpbb_filesystem $filesystem, $extension_table, $phpbb_root_path, $php_ext = '.php', phpbb_cache_driver_interface $cache = null, $cache_name = '_ext') { $this->container = $container; $this->phpbb_root_path = $phpbb_root_path; @@ -58,6 +59,7 @@ class phpbb_extension_manager $this->config = $config; $this->migrator = $migrator; $this->cache = $cache; + $this->filesystem = $filesystem; $this->php_ext = $php_ext; $this->extension_table = $extension_table; $this->cache_name = $cache_name; @@ -510,7 +512,7 @@ class phpbb_extension_manager */ public function get_finder() { - return new phpbb_extension_finder($this, $this->phpbb_root_path, $this->cache, $this->php_ext, $this->cache_name . '_finder'); + return new phpbb_extension_finder($this, $this->filesystem, $this->phpbb_root_path, $this->cache, $this->php_ext, $this->cache_name . '_finder'); } /** diff --git a/phpBB/includes/filesystem.php b/phpBB/includes/filesystem.php new file mode 100644 index 0000000000..27cab48fb0 --- /dev/null +++ b/phpBB/includes/filesystem.php @@ -0,0 +1,52 @@ +<?php +/** +* +* @package phpBB3 +* @copyright (c) 2013 phpBB Group +* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 +* +*/ +/** +* @ignore +*/ +if (!defined('IN_PHPBB')) +{ + exit; +} + +/** +* A class with various functions that are related to paths, files and the filesystem +* @package phpBB3 +*/ +class phpbb_filesystem +{ + /** + * Eliminates useless . and .. components from specified path. + * + * @param string $path Path to clean + * @return string Cleaned path + */ + public function clean_path($path) + { + $exploded = explode('/', $path); + $filtered = array(); + foreach ($exploded as $part) + { + if ($part === '.' && !empty($filtered)) + { + continue; + } + + if ($part === '..' && !empty($filtered) && $filtered[sizeof($filtered) - 1] !== '..') + { + array_pop($filtered); + } + else + { + $filtered[] = $part; + } + } + $path = implode('/', $filtered); + return $path; + } +} diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 6b5d7bd1df..3a650b37fc 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -1049,31 +1049,33 @@ else /** * Eliminates useless . and .. components from specified path. * +* Deprecated, use filesystem class instead +* * @param string $path Path to clean * @return string Cleaned path +* +* @deprecated */ function phpbb_clean_path($path) { - $exploded = explode('/', $path); - $filtered = array(); - foreach ($exploded as $part) - { - if ($part === '.' && !empty($filtered)) - { - continue; - } + global $phpbb_container; - if ($part === '..' && !empty($filtered) && $filtered[sizeof($filtered) - 1] !== '..') - { - array_pop($filtered); - } - else + if ($phpbb_container) + { + $phpbb_filesystem = $phpbb_container->get('filesystem'); + } + else + { + // The container is not yet loaded, use a new instance + if (!class_exists('phpbb_filesystem')) { - $filtered[] = $part; + global $phpbb_root_path, $phpEx; + require($phpbb_root_path . 'includes/filesystem.' . $phpEx); } + $phpbb_filesystem = new phpbb_filesystem(); } - $path = implode('/', $filtered); - return $path; + + return $phpbb_filesystem->clean_path($path); } // functions used for building option fields diff --git a/phpBB/includes/functions_messenger.php b/phpBB/includes/functions_messenger.php index 821f0d970d..e580f6b675 100644 --- a/phpBB/includes/functions_messenger.php +++ b/phpBB/includes/functions_messenger.php @@ -209,7 +209,7 @@ class messenger if (!isset($this->tpl_msg[$template_lang . $template_file])) { $style_resource_locator = new phpbb_style_resource_locator(); - $style_path_provider = new phpbb_style_extension_path_provider($phpbb_extension_manager, new phpbb_style_path_provider()); + $style_path_provider = new phpbb_style_extension_path_provider($phpbb_extension_manager, new phpbb_style_path_provider(), $phpbb_root_path); $tpl = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator, new phpbb_template_context(), $phpbb_extension_manager); $style = new phpbb_style($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator, $style_path_provider, $tpl); diff --git a/phpBB/includes/style/extension_path_provider.php b/phpBB/includes/style/extension_path_provider.php index 6976a45ed0..ec1d85f821 100644 --- a/phpBB/includes/style/extension_path_provider.php +++ b/phpBB/includes/style/extension_path_provider.php @@ -40,17 +40,22 @@ class phpbb_style_extension_path_provider extends phpbb_extension_provider imple */ protected $base_path_provider; + /** @var string */ + protected $phpbb_root_path; + /** * Constructor stores extension manager * * @param phpbb_extension_manager $extension_manager phpBB extension manager * @param phpbb_style_path_provider $base_path_provider A simple path provider * to provide paths to be located in extensions + * @param string $phpbb_root_path phpBB root path */ - public function __construct(phpbb_extension_manager $extension_manager, phpbb_style_path_provider $base_path_provider) + public function __construct(phpbb_extension_manager $extension_manager, phpbb_style_path_provider $base_path_provider, $phpbb_root_path) { parent::__construct($extension_manager); $this->base_path_provider = $base_path_provider; + $this->phpbb_root_path = $phpbb_root_path; } /** @@ -91,10 +96,23 @@ class phpbb_style_extension_path_provider extends phpbb_extension_provider imple $directories['style'][] = $path; if ($path && !phpbb_is_absolute($path)) { + // Remove phpBB root path from the style path, + // so the finder is able to find extension styles, + // when the root path is not ./ + if (strpos($path, $this->phpbb_root_path) === 0) + { + $path = substr($path, strlen($this->phpbb_root_path)); + } + $result = $finder->directory('/' . $this->ext_dir_prefix . $path) ->get_directories(true, false, true); foreach ($result as $ext => $ext_path) { + // Make sure $ext_path has no ending slash + if (substr($ext_path, -1) === '/') + { + $ext_path = substr($ext_path, 0, -1); + } $directories[$ext][] = $ext_path; } } diff --git a/phpBB/includes/user.php b/phpBB/includes/user.php index 4477c98097..5530fe3f03 100644 --- a/phpBB/includes/user.php +++ b/phpBB/includes/user.php @@ -215,7 +215,7 @@ class phpbb_user extends phpbb_session if (!$this->style) { - trigger_error('STYLE_NOT_FOUND', E_USER_ERROR); + trigger_error('NO_STYLE_DATA', E_USER_ERROR); } // Now parse the cfg file and cache it diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index c2db496437..c1d6ef4af3 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -455,6 +455,7 @@ $lang = array_merge($lang, array( 'NO_POSTS_TIME_FRAME' => 'No posts exist inside this topic for the selected time frame.', 'NO_FEED_ENABLED' => 'Feeds are not available on this board.', 'NO_FEED' => 'The requested feed is not available.', + 'NO_STYLE_DATA' => 'Could not get style data', 'NO_SUBJECT' => 'No subject specified', // Used for posts having no subject defined but displayed within management pages. 'NO_SUCH_SEARCH_MODULE' => 'The specified search backend doesn’t exist.', 'NO_SUPPORTED_AUTH_METHODS' => 'No supported authentication methods.', @@ -639,7 +640,6 @@ $lang = array_merge($lang, array( 'STATISTICS' => 'Statistics', 'START_WATCHING_FORUM' => 'Subscribe forum', 'START_WATCHING_TOPIC' => 'Subscribe topic', - 'STYLE_NOT_FOUND' => 'Could not get style data', 'STOP_WATCHING_FORUM' => 'Unsubscribe forum', 'STOP_WATCHING_TOPIC' => 'Unsubscribe topic', 'SUBFORUM' => 'Subforum', diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php index 0d7fc2f839..ae4099e6f8 100644 --- a/tests/dbal/migrator_test.php +++ b/tests/dbal/migrator_test.php @@ -60,6 +60,7 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case $this->db, $this->config, $this->migrator, + new phpbb_filesystem(), 'phpbb_ext', dirname(__FILE__) . '/../../phpBB/', '.php', diff --git a/tests/extension/ext/bar/styles/prosilver/template/foobar_body.html b/tests/extension/ext/bar/styles/prosilver/template/foobar_body.html new file mode 100644 index 0000000000..00c2a84a18 --- /dev/null +++ b/tests/extension/ext/bar/styles/prosilver/template/foobar_body.html @@ -0,0 +1 @@ +bertie rules! diff --git a/tests/extension/finder_test.php b/tests/extension/finder_test.php index c96b11a73c..4c99ba6343 100644 --- a/tests/extension/finder_test.php +++ b/tests/extension/finder_test.php @@ -6,6 +6,7 @@ * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 * */ +require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; class phpbb_extension_finder_test extends phpbb_test_case { @@ -66,7 +67,7 @@ class phpbb_extension_finder_test extends phpbb_test_case public function test_prefix_get_directories() { $dirs = $this->finder - ->prefix('t') + ->prefix('ty') ->get_directories(); sort($dirs); @@ -142,13 +143,28 @@ class phpbb_extension_finder_test extends phpbb_test_case ); } + public function test_uncleansub_directory_get_classes() + { + $classes = $this->finder + ->directory('/sub/../sub/type') + ->get_classes(); + + sort($classes); + $this->assertEquals( + array( + 'phpbb_ext_foo_sub_type_alternative', + ), + $classes + ); + } + /** * These do not work because of changes with PHPBB3-11386 * They do not seem neccessary to me, so I am commenting them out for now public function test_get_classes_create_cache() { $cache = new phpbb_mock_cache; - $finder = new phpbb_extension_finder($this->extension_manager, dirname(__FILE__) . '/', $cache, '.php', '_custom_cache_name'); + $finder = new phpbb_extension_finder($this->extension_manager, new phpbb_filesystem(), dirname(__FILE__) . '/', $cache, '.php', '_custom_cache_name'); $files = $finder->suffix('_class.php')->get_files(); $expected_files = array( @@ -188,6 +204,7 @@ class phpbb_extension_finder_test extends phpbb_test_case $finder = new phpbb_extension_finder( $this->extension_manager, + new phpbb_filesystem(), dirname(__FILE__) . '/', new phpbb_mock_cache(array( '_ext_finder' => array( diff --git a/tests/extension/manager_test.php b/tests/extension/manager_test.php index 1f311116f4..d6bcb97586 100644 --- a/tests/extension/manager_test.php +++ b/tests/extension/manager_test.php @@ -112,6 +112,7 @@ class phpbb_extension_manager_test extends phpbb_database_test_case $db, $config, $migrator, + new phpbb_filesystem(), 'phpbb_ext', dirname(__FILE__) . '/', '.' . $php_ext, diff --git a/tests/extension/metadata_manager_test.php b/tests/extension/metadata_manager_test.php index 081a32e277..df7817b479 100644 --- a/tests/extension/metadata_manager_test.php +++ b/tests/extension/metadata_manager_test.php @@ -64,6 +64,7 @@ class metadata_manager_test extends phpbb_database_test_case $this->db, $this->config, $this->migrator, + new phpbb_filesystem(), 'phpbb_ext', $this->phpbb_root_path, $this->phpEx, diff --git a/tests/extension/style_path_provider_test.php b/tests/extension/style_path_provider_test.php new file mode 100644 index 0000000000..e1021c20ac --- /dev/null +++ b/tests/extension/style_path_provider_test.php @@ -0,0 +1,50 @@ +<?php +/** +* +* @package testing +* @copyright (c) 2013 phpBB Group +* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 +* +*/ +require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; + +class phpbb_extension_style_path_provider_test extends phpbb_test_case +{ + protected $relative_root_path; + protected $root_path; + + public function setUp() + { + $this->relative_root_path = './'; + $this->root_path = dirname(__FILE__) . '/'; + } + + public function test_find() + { + $phpbb_style_path_provider = new phpbb_style_path_provider(); + $phpbb_style_path_provider->set_styles(array($this->relative_root_path . 'styles/prosilver')); + $phpbb_style_extension_path_provider = new phpbb_style_extension_path_provider(new phpbb_mock_extension_manager( + $this->root_path, + array( + 'foo' => array( + 'ext_name' => 'foo', + 'ext_active' => '1', + 'ext_path' => 'ext/foo/', + ), + 'bar' => array( + 'ext_name' => 'bar', + 'ext_active' => '1', + 'ext_path' => 'ext/bar/', + ), + )), $phpbb_style_path_provider, $this->relative_root_path); + + $this->assertEquals(array( + 'style' => array( + $this->relative_root_path . 'styles/prosilver', + ), + 'bar' => array( + $this->root_path . 'ext/bar/styles/prosilver', + ), + ), $phpbb_style_extension_path_provider->find()); + } +} diff --git a/tests/extension/subdir/style_path_provider_test.php b/tests/extension/subdir/style_path_provider_test.php new file mode 100644 index 0000000000..1b5ce62e5f --- /dev/null +++ b/tests/extension/subdir/style_path_provider_test.php @@ -0,0 +1,18 @@ +<?php +/** +* +* @package testing +* @copyright (c) 2013 phpBB Group +* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 +* +*/ +require_once dirname(__FILE__) . '/../style_path_provider_test.php'; + +class phpbb_extension_subdir_style_path_provider_test extends phpbb_extension_style_path_provider_test +{ + public function setUp() + { + $this->relative_root_path = '../'; + $this->root_path = dirname(__FILE__) . '/../'; + } +} diff --git a/tests/functions/clean_path_test.php b/tests/filesystem/clean_path_test.php index bcbe9838d9..50951fc88c 100644 --- a/tests/functions/clean_path_test.php +++ b/tests/filesystem/clean_path_test.php @@ -7,11 +7,17 @@ * */ -require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; - -class phpbb_clean_path_test extends phpbb_test_case +class phpbb_filesystem_clean_path_test extends phpbb_test_case { - public function clean_path_test_data() + protected $filesystem; + + public function setUp() + { + parent::setUp(); + $this->filesystem = new phpbb_filesystem(); + } + + public function clean_path_data() { return array( array('foo', 'foo'), @@ -33,12 +39,10 @@ class phpbb_clean_path_test extends phpbb_test_case } /** - * @dataProvider clean_path_test_data + * @dataProvider clean_path_data */ public function test_clean_path($input, $expected) { - $output = phpbb_clean_path($input); - - $this->assertEquals($expected, $output); + $this->assertEquals($expected, $this->filesystem->clean_path($input)); } } diff --git a/tests/mock/extension_manager.php b/tests/mock/extension_manager.php index fdda4cbadc..954f2bf1c4 100644 --- a/tests/mock/extension_manager.php +++ b/tests/mock/extension_manager.php @@ -14,5 +14,6 @@ class phpbb_mock_extension_manager extends phpbb_extension_manager $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = '.php'; $this->extensions = $extensions; + $this->filesystem = new phpbb_filesystem(); } } diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 15c063ec02..b9647e4742 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -153,6 +153,7 @@ class phpbb_functional_test_case extends phpbb_test_case $db, $config, $migrator, + new phpbb_filesystem(), self::$config['table_prefix'] . 'ext', dirname(__FILE__) . '/', '.' . $php_ext, |