diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 0a934cd325..51a1d9b15f 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -27,6 +27,13 @@ +{% if S_ERROR %} +
+

{{ lang('WARNING') }}

+

{{ ERROR_MSG }}

+
+{% endif %} +
{% for storage in STORAGES %} diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 12a4b45d67..9ba84aafaf 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -42,9 +42,15 @@ class acp_storage /** @var \phpbb\di\service_collection */ protected $storage_collection; + /** @var \phpbb\filesystem\filesystem */ + protected $filesystem; + /** @var string */ public $page_title; + /** @var string */ + public $phpbb_root_path; + /** @var string */ public $tpl_name; @@ -57,15 +63,17 @@ class acp_storage */ public function main($id, $mode) { - global $phpbb_container, $phpbb_dispatcher; + global $phpbb_container, $phpbb_dispatcher, $phpbb_root_path; $this->config = $phpbb_container->get('config'); + $this->filesystem = $phpbb_container->get('filesystem'); $this->lang = $phpbb_container->get('language'); $this->request = $phpbb_container->get('request'); $this->template = $phpbb_container->get('template'); $this->user = $phpbb_container->get('user'); $this->provider_collection = $phpbb_container->get('storage.provider_collection'); $this->storage_collection = $phpbb_container->get('storage.storage_collection'); + $this->phpbb_root_path = $phpbb_root_path; // Add necesary language files $this->lang->add_lang(['acp/storage']); @@ -96,10 +104,10 @@ class acp_storage // Set page title $this->page_title = 'STORAGE_TITLE'; + $messages = []; if ($this->request->is_set_post('submit')) { $modified_storages = []; - $messages = []; if (!check_form_key($form_key)) { @@ -112,6 +120,8 @@ class acp_storage $options = $this->get_provider_options($this->get_current_provider($storage_name)); + $this->validate_path($storage_name, $options, $messages); + $modified = false; // Check if provider have been modified @@ -165,6 +175,11 @@ class acp_storage $storage_stats = []; foreach ($this->storage_collection as $storage) { + $storage_name = $storage->get_name(); + $options = $this->get_provider_options($this->get_current_provider($storage_name)); + + $this->validate_path($storage_name, $options, $messages); + try { $free_space = get_formatted_filesize($storage->free_space()); @@ -182,11 +197,14 @@ class acp_storage ]; } - $this->template->assign_vars(array( + $this->template->assign_vars([ 'STORAGES' => $this->storage_collection, 'STORAGE_STATS' => $storage_stats, 'PROVIDERS' => $this->provider_collection, - )); + + 'ERROR_MSG' => implode('
', $messages), + 'S_ERROR' => !empty($messages), + ]); } /** @@ -336,4 +354,29 @@ class acp_storage $this->config->set('storage\\' . $storage_name . '\\config\\' . $definition, $this->get_new_definition($storage_name, $definition)); } } + + /** + * Validates path + * + * @param string $storage_name Storage name + * @param array $options Storage provider configuration keys + * @param array $messages Error messages array + * @return array $messages Reference to messages array + */ + protected function validate_path($storage_name, $options, &$messages) + { + if ($this->provider_collection->get_by_class($this->get_current_provider($storage_name))->get_name() == 'local' && isset($options['path'])) + { + $path = $this->request->is_set_post('submit') ? $this->get_new_definition($storage_name, 'path') : $this->get_current_definition($storage_name, 'path'); + + if (empty($path)) + { + $messages[] = $this->lang->lang('STORAGE_PATH_NOT_SET', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE')); + } + else if (!$this->filesystem->is_writable($this->phpbb_root_path . $path) || !$this->filesystem->exists($this->phpbb_root_path . $path)) + { + $messages[] = $this->lang->lang('STORAGE_PATH_NOT_EXISTS', $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE')); + } + } + } } diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index 69faa58030..f0da8b3d2f 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -68,4 +68,7 @@ $lang = array_merge($lang, array( 'STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT' => 'Incorrect email for %s of %s.', 'STORAGE_FORM_TYPE_TEXT_TOO_LONG' => 'Text is too long for %s of %s.', 'STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE' => 'Selected value is not available for %s of %s.', + + 'STORAGE_PATH_NOT_EXISTS' => '“%1$s” path does not exist or is not writable.', + 'STORAGE_PATH_NOT_SET' => '“%1$s” path is not set.', )); diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index 6dfac13474..d2086a73e5 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -110,8 +110,6 @@ $lang = array_merge($lang, array( 'AVATAR_NOT_UPLOADED' => 'Avatar could not be uploaded.', 'AVATAR_NO_TEMP_DIR' => 'Temporary folder could not be found or is not writable.', 'AVATAR_NO_SIZE' => 'The width or height of the linked avatar could not be determined. Please enter them manually.', - 'AVATAR_NO_UPLOAD_DIR' => 'Avatar storage path does not exist or is not writable.', - 'AVATAR_NO_UPLOAD_PATH' => 'Avatar uploading is enabled but avatar storage path is not set.', 'AVATAR_PARTIAL_UPLOAD' => 'The specified file was only partially uploaded.', 'AVATAR_PHP_SIZE_NA' => 'The avatar’s filesize is too large.
The maximum allowed filesize set in php.ini could not be determined.', 'AVATAR_PHP_SIZE_OVERRUN' => 'The avatar’s filesize is too large. The maximum allowed upload size is %1$d %2$s.
Please note this is set in php.ini and cannot be overridden.', diff --git a/tests/functional/acp_avatar_settings_test.php b/tests/functional/acp_avatar_settings_test.php deleted file mode 100644 index 220233fb56..0000000000 --- a/tests/functional/acp_avatar_settings_test.php +++ /dev/null @@ -1,45 +0,0 @@ - -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ - -/** -* @group functional -*/ -class phpbb_functional_acp_avatar_settings_test extends phpbb_functional_test_case -{ - public function test_avatar_upload_settings() - { - $this->add_lang(['acp/common', 'acp/board']); - $this->login(); - $this->admin_login(); - - $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=avatar&sid=' . $this->sid); - $this->assertContainsLang('ACP_AVATAR_SETTINGS', $this->get_content()); - $this->assertContainsLang('ACP_AVATAR_SETTINGS_EXPLAIN', $this->get_content()); - - // Test disabling avatar uploading - valid - $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ - 'config[allow_avatar_upload]' => '0' - ]); - $crawler = self::submit($form); - $this->assertContainsLang('CONFIG_UPDATED', $crawler->filter('div[class="successbox"] > p')->text()); - - $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=avatar&sid=' . $this->sid); - - // Test enabling avatar uploading - valid - $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ - 'config[allow_avatar_upload]' => '1' - ]); - $crawler = self::submit($form); - $this->assertContainsLang('CONFIG_UPDATED', $crawler->filter('div[class="successbox"] > p')->text()); - } -} diff --git a/tests/functional/acp_storage_settings_test.php b/tests/functional/acp_storage_settings_test.php new file mode 100644 index 0000000000..b99e3e8fa9 --- /dev/null +++ b/tests/functional/acp_storage_settings_test.php @@ -0,0 +1,93 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +/** +* @group functional +*/ +class phpbb_functional_acp_storage_settings_test extends phpbb_functional_test_case +{ + public function test_storage_settings() + { + $this->add_lang(['common', 'acp/storage']); + $this->login(); + $this->admin_login(); + + $crawler = self::request('GET', 'adm/index.php?i=acp_storage&mode=settings&sid=' . $this->sid); + $this->assertContainsLang('STORAGE_TITLE', $this->get_content()); + $this->assertContainsLang('STORAGE_TITLE_EXPLAIN', $this->get_content()); + + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); + $crawler = self::submit($form); + $this->assertContainsLang('INFORMATION', $crawler->filter('div[class="errorbox"] > h3')->text()); + $this->assertContainsLang('STORAGE_NO_CHANGES', $crawler->filter('div[class="errorbox"] > p')->text()); + + // Test empty storage paths - invalid + $crawler = self::request('GET', 'adm/index.php?i=acp_storage&mode=settings&sid=' . $this->sid); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ + 'attachment[path]' => '', + 'avatar[path]' => '', + 'backup[path]' => '', + ]); + $crawler = self::submit($form); + $this->assertContainsLang('INFORMATION', $crawler->filter('div[class="errorbox"] > h3')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_SET', $this->lang('STORAGE_ATTACHMENT_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_SET', $this->lang('STORAGE_AVATAR_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_SET', $this->lang('STORAGE_BACKUP_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); + + // Test storage paths became not writable on the server afterwards + // Unix tests only + if (!defined('PHP_WINDOWS_VERSION_MAJOR')) + { + $crawler = self::request('GET', 'adm/index.php?i=acp_storage&mode=settings&sid=' . $this->sid); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); + $values = $form->getValues(); + + $attachments_storage_path = $values['attachment[path]']; + $avatar_upload_path = $values['avatar[path]']; + $backup_storage_path = $values['backup[path]']; + + $filesystem = new \phpbb\filesystem\filesystem; + + // Make the directory not writable + global $phpbb_root_path; + $filesystem->chmod($phpbb_root_path . $attachments_storage_path, 444); + $filesystem->chmod($phpbb_root_path . $avatar_upload_path, 444); + $filesystem->chmod($phpbb_root_path . $backup_storage_path, 444); + $this->assertFalse($filesystem->is_writable($phpbb_root_path . $avatar_upload_path)); + + // Visit ACP Storage settings again - warning should be displayed + $crawler = self::request('GET', 'adm/index.php?i=acp_storage&mode=settings&sid=' . $this->sid); + $this->assertContainsLang('WARNING', $crawler->filter('div[class="errorbox"] > h3')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_ATTACHMENT_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_AVATAR_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); + $this->assertStringContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_BACKUP_TITLE')), $crawler->filter('div[class="errorbox"] > p')->text()); + + // Restore default state + $filesystem->chmod($phpbb_root_path . $attachments_storage_path, 777); + $filesystem->chmod($phpbb_root_path . $avatar_upload_path, 777); + $filesystem->chmod($phpbb_root_path . $backup_storage_path, 777); + $this->assertTrue($filesystem->is_writable($phpbb_root_path . $attachments_storage_path)); + $this->assertTrue($filesystem->is_writable($phpbb_root_path . $avatar_upload_path)); + $this->assertTrue($filesystem->is_writable($phpbb_root_path . $backup_storage_path)); + + $crawler = self::request('GET', 'adm/index.php?i=acp_storage&mode=settings&sid=' . $this->sid); + $this->assertStringNotContainsString($this->lang('STORAGE_PATH_NOT_SET', $this->lang('STORAGE_ATTACHMENT_TITLE')), $this->get_content()); + $this->assertStringNotContainsString($this->lang('STORAGE_PATH_NOT_SET', $this->lang('STORAGE_AVATAR_TITLE')), $this->get_content()); + $this->assertStringNotContainsString($this->lang('STORAGE_PATH_NOT_SET', $this->lang('STORAGE_BACKUP_TITLE')), $this->get_content()); + + $this->assertStringNotContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_ATTACHMENT_TITLE')), $this->get_content()); + $this->assertStringNotContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_AVATAR_TITLE')), $this->get_content()); + $this->assertStringNotContainsString($this->lang('STORAGE_PATH_NOT_EXISTS', $this->lang('STORAGE_BACKUP_TITLE')), $this->get_content()); + } + } +}