diff --git a/phpBB/includes/acp/acp_board.php b/phpBB/includes/acp/acp_board.php index 8e571de379..968c9762cc 100644 --- a/phpBB/includes/acp/acp_board.php +++ b/phpBB/includes/acp/acp_board.php @@ -510,6 +510,29 @@ class acp_board } } + if ($mode == 'avatar' && $cfg_array['allow_avatar_upload']) + { + // If avatar uploading is enabled but the path setting is empty, + // config variable validation is bypassed. Catch the case here + if (!$cfg_array['avatar_path']) + { + $error[] = $language->lang('AVATAR_NO_UPLOAD_PATH'); + } + else if (!$submit) + { + $filesystem = $phpbb_container->get('filesystem'); + $avatar_path_exists = $filesystem->exists($phpbb_root_path . $cfg_array['avatar_path']); + $avatar_path_writable = $filesystem->is_writable($phpbb_root_path . $cfg_array['avatar_path']); + + // Not existing or writable path will be caught on submit by validate_config_vars(). + // Display the warning if the directory was changed on the server afterwards + if (!$avatar_path_exists || !$avatar_path_writable) + { + $error[] = $language->lang('AVATAR_NO_UPLOAD_DIR'); + } + } + } + // We validate the complete config if wished validate_config_vars($display_vars['vars'], $cfg_array, $error); diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index 3f8117b7af..fefb7dafb6 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -110,6 +110,8 @@ $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 new file mode 100644 index 0000000000..254459ad33 --- /dev/null +++ b/tests/functional/acp_avatar_settings_test.php @@ -0,0 +1,83 @@ + +* @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()); + + $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=avatar&sid=' . $this->sid); + + // Test empty avatar storage path while avatar uploading is enabled - invalid + $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ + 'config[avatar_path]' => '' + ]); + $crawler = self::submit($form); + $this->assertContainsLang('WARNING', $crawler->filter('div[class="errorbox"] > h3')->text()); + $this->assertContainsLang('AVATAR_NO_UPLOAD_PATH', $crawler->filter('div[class="errorbox"] > p')->text()); + + // Test avatar upload path 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_board&mode=avatar&sid=' . $this->sid); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); + $values = $form->getValues(); + $avatar_upload_path = $values['config[avatar_path]']; + $filesystem = new \phpbb\filesystem\filesystem; + // Make the directory not writable + global $phpbb_root_path; + $filesystem->chmod($phpbb_root_path . $avatar_upload_path, 444); + $this->assertFalse($filesystem->is_writable($phpbb_root_path . $avatar_upload_path)); + + // Visit Avatar ACP settings again - warning should be displayed + $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=avatar&sid=' . $this->sid); + $this->assertContainsLang('WARNING', $crawler->filter('div[class="errorbox"] > h3')->text()); + $this->assertContainsLang('AVATAR_NO_UPLOAD_DIR', $crawler->filter('div[class="errorbox"] > p')->text()); + + // Restore default state + $filesystem->chmod($phpbb_root_path . $avatar_upload_path, 777); + $this->assertTrue($filesystem->is_writable($phpbb_root_path . $avatar_upload_path)); + + $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=avatar&sid=' . $this->sid); + $this->assertNotContainsLang('AVATAR_NO_UPLOAD_DIR', $this->get_content()); + $this->assertNotContainsLang('AVATAR_NO_UPLOAD_PATH', $this->get_content()); + } + } +}