[ticket/17135] Fix code flaws

PHPBB-17135
This commit is contained in:
rxu 2024-06-23 14:55:01 +07:00
parent 3fddff240c
commit 0c720eaf7c
No known key found for this signature in database
GPG key ID: 955F0567380E586A
13 changed files with 249 additions and 107 deletions

View file

@ -16,9 +16,7 @@ services:
- '@assets.bag'
- '@config'
- '@dispatcher'
- '@ext.manager'
- '@language'
- '@log'
- '@messenger.queue'
- '@path_helper'
- '@request'
@ -27,6 +25,8 @@ services:
- '@user'
- '%core.root_path%'
- '%core.template.cache_path%'
- '@?ext.manager'
- '@?log'
messenger.method.email:
class: phpbb\messenger\method\email

View file

@ -217,7 +217,8 @@ class acp_email
);
extract($phpbb_dispatcher->trigger_event('core.acp_email_send_before', compact($vars)));
$messenger = (\phpbb\di\service_collection) $phpbb_container->get('messenger.method_collection');
/** @var \phpbb\di\service_collection */
$messenger = $phpbb_container->get('messenger.method_collection');
$messenger_collection_iterator = $messenger->getIterator();
for ($i = 0, $size = count($email_list); $i < $size; $i++)
{

View file

@ -198,7 +198,9 @@ class acp_inactive
{
// Send the messages
$usernames = $user_ids = array();
$messenger = (\phpbb\di\service_collection) $phpbb_container->get('messenger.method_collection');
/** @var \phpbb\di\service_collection */
$messenger = $phpbb_container->get('messenger.method_collection');
$messenger_collection_iterator = $messenger->getIterator();
do

View file

@ -131,7 +131,7 @@ class ucp_activate
$phpbb_notifications = $phpbb_container->get('notification_manager');
$phpbb_notifications->delete_notifications('notification.type.admin_activate_user', $user_row['user_id']);
$messenger = (\phpbb\di\service_collection) $phpbb_container->get('messenger.method_collection');
$messenger = $phpbb_container->get('messenger.method_collection');
$messenger_collection_iterator = $messenger->getIterator();
foreach ($messenger_collection_iterator as $messenger_method)
{

View file

@ -458,6 +458,7 @@ class ucp_register
if ($config['email_enable'])
{
/** var \phpbb\messenger\method\email */
$email_method = $phpbb_container->get('messenger.method.email');
$email_method->set_use_queue(false);
$email_method->template($email_template, $data['lang']);
@ -490,8 +491,9 @@ class ucp_register
* @var string server_url Server URL
* @var int user_id New user ID
* @var string user_actkey User activation key
* @var messenger messenger phpBB Messenger
* @var \phpbb\messenger\method\email email_method phpBB email notification method
* @since 3.2.4-RC1
* @changed 4.0.0-a1 Added vars: email_method. Removed vars: messenger.
*/
$vars = array(
'user_row',
@ -501,7 +503,7 @@ class ucp_register
'server_url',
'user_id',
'user_actkey',
'messenger',
'email_method',
);
extract($phpbb_dispatcher->trigger_event('core.ucp_register_welcome_email_before', compact($vars)));

View file

@ -900,7 +900,7 @@ switch ($mode)
case 'contactadmin':
case 'email':
$messenger = (\phpbb\di\service_collection) $phpbb_container->get('messenger.method_collection');
$messenger = $phpbb_container->get('messenger.method_collection');
$user_id = $request->variable('u', 0);
$topic_id = $request->variable('t', 0);

View file

@ -94,9 +94,7 @@ abstract class base
* @param assets_bag $assets_bag
* @param config $config
* @param dispatcher $dispatcher
* @param manager $ext_manager
* @param language $language
* @param log_interface $log
* @param queue $queue
* @param path_helper $path_helper
* @param request $request
@ -105,14 +103,14 @@ abstract class base
* @param user $user
* @param string $phpbb_root_path
* @param string $template_cache_path
* @param manager $ext_manager
* @param log_interface $log
*/
public function __construct(
assets_bag $assets_bag,
config $config,
dispatcher $dispatcher,
manager $ext_manager,
language $language,
log_interface $log,
queue $queue,
path_helper $path_helper,
request $request,
@ -120,7 +118,9 @@ abstract class base
lexer $twig_lexer,
user $user,
$phpbb_root_path,
$template_cache_path
$template_cache_path,
?manager $ext_manager = null,
?log_interface $log = null
)
{
$this->assets_bag = $assets_bag;
@ -198,32 +198,6 @@ abstract class base
$this->subject = $subject;
}
/**
* Adds antiabuse headers
*
* @param config $config Config object
* @param user $user User object
* @return void
*/
abstract public function anti_abuse_headers(config $config, user $user): void;
/**
* Set up extra headers
*
* @param string $header_name Email header name
* @param string $header_value Email header body
* @return void
*/
abstract public function header(string $header_name, string $header_value): void;
/**
* Set the reply to address
*
* @param string $address Email "Reply to" address
* @return void
*/
abstract public function reply_to($address): void;
/**
* Send out messages
*
@ -444,7 +418,10 @@ abstract class base
$type = strtoupper($this->get_queue_object_name());
$calling_page = html_entity_decode($this->request->server('PHP_SELF'), ENT_COMPAT);
$message = '<strong>' . $type . '</strong><br><em>' . htmlspecialchars($calling_page, ENT_COMPAT) . '</em><br><br>' . $msg . '<br>';
$this->log->add('critical', $this->user->data['user_id'], $this->user->ip, 'LOG_ERROR_' . $type, false, [$message]);
if ($this->log)
{
$this->log->add('critical', $this->user->data['user_id'], $this->user->ip, 'LOG_ERROR_' . $type, false, [$message]);
}
}
/**

View file

@ -26,11 +26,11 @@ class email extends base
{
/** @var array */
private const PRIORITY_MAP = [
Email::PRIORITY_HIGHEST => 'Highest',
Email::PRIORITY_HIGH => 'High',
Email::PRIORITY_NORMAL => 'Normal',
Email::PRIORITY_LOW => 'Low',
Email::PRIORITY_LOWEST => 'Lowest',
symfony_email::PRIORITY_HIGHEST => 'Highest',
symfony_email::PRIORITY_HIGH => 'High',
symfony_email::PRIORITY_NORMAL => 'Normal',
symfony_email::PRIORITY_LOW => 'Low',
symfony_email::PRIORITY_LOWEST => 'Lowest',
];
/**
@ -40,7 +40,7 @@ class email extends base
*/
protected $dsn = '';
/** @var Email */
/** @var symfony_email */
protected $email;
/** @var Address */
@ -53,13 +53,13 @@ class email extends base
* @var int
*
* Possible values are:
* Email::PRIORITY_HIGHEST
* Email::PRIORITY_HIGH
* Email::PRIORITY_NORMAL
* Email::PRIORITY_LOW
* Email::PRIORITY_LOWEST
* symfony_email::PRIORITY_HIGHEST
* symfony_email::PRIORITY_HIGH
* symfony_email::PRIORITY_NORMAL
* symfony_email::PRIORITY_LOW
* symfony_email::PRIORITY_LOWEST
*/
protected $mail_priority = Email::PRIORITY_NORMAL;
protected $mail_priority = symfony_email::PRIORITY_NORMAL;
/** @var \phpbb\messenger\queue */
protected $queue;
@ -102,7 +102,7 @@ class email extends base
$this->email = new symfony_email();
$this->headers = $this->email->getHeaders();
$this->subject = $this->msg = '';
$this->mail_priority = Email::PRIORITY_NORMAL;
$this->mail_priority = symfony_email::PRIORITY_NORMAL;
$this->additional_headers = [];
$this->use_queue = true;
@ -254,16 +254,16 @@ class email extends base
* Set the email priority
*
* Possible values are:
* Email::PRIORITY_HIGHEST = 1
* Email::PRIORITY_HIGH = 2
* Email::PRIORITY_NORMAL = 3
* Email::PRIORITY_LOW = 4
* Email::PRIORITY_LOWEST = 5
* symfony_email::PRIORITY_HIGHEST = 1
* symfony_email::PRIORITY_HIGH = 2
* symfony_email::PRIORITY_NORMAL = 3
* symfony_email::PRIORITY_LOW = 4
* symfony_email::PRIORITY_LOWEST = 5
*
* @param int $priority Email priority level
* @return void
*/
public function set_mail_priority(int $priority = Email::PRIORITY_NORMAL): void
public function set_mail_priority(int $priority = symfony_email::PRIORITY_NORMAL): void
{
$this->email->priority($priority);
}
@ -291,7 +291,7 @@ class email extends base
$this->email->priority($this->mail_priority);
$phpbb_headers = [
$headers = [
'Return-Path' => new Address($this->config['board_email']),
'Sender' => new Address($this->config['board_email']),
'X-MSMail-Priority' => self::PRIORITY_MAP[$this->mail_priority],
@ -301,25 +301,23 @@ class email extends base
];
// Add additional headers
$phpbb_headers = array_merge($phpbb_headers, $this->additional_headers);
$headers = array_merge($headers, $this->additional_headers);
foreach ($phpbb_headers as $header => $value)
{
$this->headers->addHeader($header, $value);
}
$headers = $this->headers;
/**
* Event to modify email header entries
*
* @event core.modify_email_headers
* @var Headers headers Array containing email header entries
* @var array headers Array containing email header entries
* @since 3.1.11-RC1
* @changed 4.0.0-a1 'headers' var type changed from array to \Symfony\Component\Mime\Header\Headers
*/
$vars = ['headers'];
extract($this->dispatcher->trigger_event('core.modify_email_headers', compact($vars)));
$this->headers = $headers;
foreach ($headers as $header => $value)
{
$this->headers->addHeader($header, $value);
}
}
/**
@ -566,7 +564,9 @@ class email extends base
'email' => $this->email,
]);
}
$this->reset();
// Reset the object
$this->init();
return true;
}

View file

@ -359,13 +359,12 @@ class jabber extends base
return is_resource($this->connection) && !feof($this->connection);
}
/**
* Initiates login (using data from contructor, after calling connect())
*
* @return bool|void
* @return bool|null
*/
public function login(): bool|void
public function login(): bool|null
{
if (empty($this->features))
{
@ -410,7 +409,7 @@ class jabber extends base
/**
* {@inheritDoc}
*/
public function init(): void
public function reset(): void
{
$this->subject = $this->msg = '';
$this->additional_headers = $this->to = [];
@ -484,7 +483,7 @@ class jabber extends base
/**
* {@inheritDoc}
*/
public function send(): void
public function send(): bool
{
$this->prepare_message();
@ -666,9 +665,9 @@ class jabber extends base
/**
* Initiates account registration (based on data used for contructor)
*
* @return bool|void
* @return bool|null
*/
public function register(): bool|void
public function register(): bool|null
{
if (!isset($this->session['id']) || isset($this->session['jid']))
{
@ -711,9 +710,9 @@ class jabber extends base
* This handles all the different XML elements
*
* @param array $xml
* @return bool|void
* @return bool|null
*/
function response(array $xml): bool|void
function response(array $xml): bool|null
{
if (!is_array($xml) || !count($xml))
{
@ -728,20 +727,17 @@ class jabber extends base
{
$this->response(array($key => $value));
}
return;
return true;
}
else
else if (is_array(reset($xml)) && count(reset($xml)) > 1)
{
// or even multiple elements of the same type?
// array('message' => array(0 => ..., 1 => ...))
if (is_array(reset($xml)) && count(reset($xml)) > 1)
foreach (reset($xml) as $value)
{
foreach (reset($xml) as $value)
{
$this->response(array(key($xml) => array(0 => $value)));
}
return;
$this->response(array(key($xml) => array(0 => $value)));
}
return true;
}
switch (key($xml))
@ -1012,6 +1008,7 @@ class jabber extends base
$message['subject'] = $xml['message'][0]['#']['subject'][0]['#'];
}
$this->session['messages'][] = $message;
return true;
break;
default:
@ -1123,7 +1120,7 @@ class jabber extends base
* @param array $data Data array
* @return string
*/
public function implode_data(arary $data): string
public function implode_data(array $data): string
{
$return = array();
foreach ($data as $key => $value)
@ -1141,7 +1138,7 @@ class jabber extends base
* @param string $data Data string
* @param string|int|bool $skip_white New XML parser option value
* @param string $encoding Encoding value
* @return string
* @return array
*/
function xmlize(string $data, string|int|bool $skip_white = 1, string $encoding = 'UTF-8'): array
{

View file

@ -114,7 +114,7 @@ abstract class messenger_base extends \phpbb\notification\method\base
/** @psalm-suppress InvalidTemplateParam */
$messenger_collection_iterator = $this->messenger->getIterator();
while ($messenger_collection_iterator as $messenger_method)
foreach ($messenger_collection_iterator as $messenger_method)
{
if ($messenger_method->get_id() == $notify_method || $notify_method == NOTIFY_BOTH)
{

View file

@ -34,7 +34,7 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
{
global $auth, $db, $cache, $config, $user, $phpbb_dispatcher, $phpbb_container, $phpbb_root_path, $phpEx;
$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
$phpbb_dispatcher = new \phpbb\event\dispatcher();
$phpbb_container = new phpbb_mock_container_builder();
$phpbb_container->set('cache.driver', new phpbb_mock_cache());
$phpbb_container->set('notification_manager', new phpbb_mock_notification_manager());
@ -108,8 +108,7 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
$assets_bag = new \phpbb\template\assets_bag();
$phpbb_container->set('assets.bag', $assets_bag);
$dispatcher = new \phpbb\event\dispatcher();
$phpbb_container->set('dispatcher', $dispatcher);
$phpbb_container->set('dispatcher', $phpbb_dispatcher);
$core_cache_dir = $phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT . '/';
$phpbb_container->setParameter('core.cache_dir', $core_cache_dir);
@ -121,7 +120,7 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
$messenger_method_collection->add('messenger.method.email');
$phpbb_container->set('messenger.method_collection', $messenger_method_collection);
$messenger_queue = new \phpbb\messenger\queue($config, $dispatcher, $messenger_method_collection, $core_messenger_queue_file);
$messenger_queue = new \phpbb\messenger\queue($config, $phpbb_dispatcher, $messenger_method_collection, $core_messenger_queue_file);
$phpbb_container->set('messenger.queue', $messenger_queue);
$request = new phpbb_mock_request;
@ -139,9 +138,33 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
);
$phpbb_container->set('path_helper', $phpbb_path_helper);
$extension_manager = new phpbb_mock_extension_manager(
$factory = new \phpbb\db\tools\factory();
$db_doctrine = $this->new_doctrine_dbal();
$db_tools = $factory->get($db_doctrine);
$migrator = new \phpbb\db\migrator(
$phpbb_container,
$config,
$db,
$db_tools,
'phpbb_migrations',
$phpbb_root_path,
$this->php_ext,
'phpbb_',
self::get_core_tables(),
[],
new \phpbb\db\migration\helper()
);
$phpbb_container->set('migrator', $migrator);
$finder_factory = new \phpbb\finder\factory(null, false, $phpbb_root_path, $this->php_ext);
$extension_manager = new \phpbb\extension\manager(
$phpbb_container,
$db,
$config,
$finder_factory,
'phpbb_ext',
__DIR__ . '/',
[]
new \phpbb\cache\service(new phpbb_mock_cache(), $config, $db, $phpbb_dispatcher, $phpbb_root_path, $this->php_ext)
);
$phpbb_container->set('ext.manager', $extension_manager);
@ -159,7 +182,7 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
$cache_path,
null,
new \phpbb\template\twig\loader(''),
$dispatcher,
$phpbb_dispatcher,
[
'cache' => false,
'debug' => false,
@ -167,7 +190,7 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
'autoescape' => false,
]
);
$twig_extension = new \phpbb\template\twig\extension($context, $twig, $lang);
$twig_extension = new \phpbb\template\twig\extension($context, $twig, $this->language);
$phpbb_container->set('template.twig.extensions.phpbb', $twig_extension);
$twig_extensions_collection = new \phpbb\di\service_collection($phpbb_container);
@ -178,10 +201,21 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
$twig_lexer = new \phpbb\template\twig\lexer($twig);
$phpbb_container->set('template.twig.lexer', $twig_lexer);
$this->email = new \phpbb\messenger\method\phpbb_email(
$assets_bag, $this->config, $dispatcher, $this->language, $log, $request, $user, $messenger_queue,
$phpbb_path_helper, $extension_manager, $twig_extensions_collection, $twig_lexer,
$cache_path, $phpbb_root_path
$this->email = new \phpbb\messenger\method\email(
$assets_bag,
$this->config,
$phpbb_dispatcher,
$this->language,
$messenger_queue,
$phpbb_path_helper,
$request,
$twig_extensions_collection,
$twig_lexer,
$user,
$phpbb_root_path,
$cache_path,
$extension_manager,
$this->log
);
$phpbb_container->set('messenger.method.email', $this->email);

View file

@ -123,10 +123,21 @@ class phpbb_email_parsing_test extends phpbb_test_case
$messenger_queue = new \phpbb\messenger\queue($config, $dispatcher, $messenger_method_collection, $core_messenger_queue_file);
$phpbb_container->set('messenger.queue', $messenger_queue);
$this->email = new \phpbb\messenger\method\phpbb_email(
$assets_bag, $config, $dispatcher, $lang, $log, $request, $user, $messenger_queue,
$phpbb_path_helper, $extension_manager, $twig_extensions_collection, $twig_lexer,
$cache_path, $phpbb_root_path
$this->email = new \phpbb\messenger\method\email(
$assets_bag,
$config,
$dispatcher,
$lang,
$messenger_queue,
$phpbb_path_helper,
$request,
$twig_extensions_collection,
$twig_lexer,
$user,
$phpbb_root_path,
$cache_path,
$extension_manager,
$log
);
$phpbb_container->set('messenger.method.email', $this->email);

View file

@ -324,6 +324,123 @@ class phpbb_functional_test_case extends phpbb_test_case
return $extension_manager;
}
protected static function get_messenger_method_email($container)
{
global $phpbb_root_path, $phpEx;
$config = new \phpbb\config\config(
[
'version' => PHPBB_VERSION,
'email_enable' => false,
'email_package_size' => 0,
'smtp_delivery' => 0,
'default_lang' => 'en',
]
);
$lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx);
$lang = new \phpbb\language\language($lang_loader);
$user = new \phpbb\user($lang, '\phpbb\datetime');
$container->set('user', $user);
$container->set('language', $lang);
$assets_bag = new \phpbb\template\assets_bag();
$container->set('assets.bag', $assets_bag);
$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
$container->set('dispatcher', $phpbb_dispatcher);
$core_cache_dir = $phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT . '/';
$container->setParameter('core.cache_dir', $core_cache_dir);
$core_messenger_queue_file = $core_cache_dir . 'queue.' . $phpEx;
$container->setParameter('core.messenger_queue_file', $core_messenger_queue_file);
$messenger_method_collection = new \phpbb\di\service_collection($container);
$messenger_method_collection->add('messenger.method.email');
$container->set('messenger.method_collection', $messenger_method_collection);
$messenger_queue = new \phpbb\messenger\queue($config, $phpbb_dispatcher, $messenger_method_collection, $core_messenger_queue_file);
$container->set('messenger.queue', $messenger_queue);
$request = new phpbb_mock_request;
$container->set('request', $request);
$symfony_request = new \phpbb\symfony_request(
$request
);
$phpbb_path_helper = new \phpbb\path_helper(
$symfony_request,
$request,
$phpbb_root_path,
$phpEx
);
$container->set('path_helper', $phpbb_path_helper);
$dbms = self::$config['dbms'];
$db = new $dbms();
$db->sql_connect(self::$config['dbhost'], self::$config['dbuser'], self::$config['dbpasswd'], self::$config['dbname'], self::$config['dbport']);
$extension_manager = new phpbb_mock_extension_manager($phpbb_root_path, [], $container);
$container->set('ext.manager', $extension_manager);
$context = new \phpbb\template\context();
$cache_path = $phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT . '/twig';
$container->setParameter('core.template.cache_path', $cache_path);
$filesystem = new \phpbb\filesystem\filesystem();
$container->set('filesystem', $filesystem);
$twig = new \phpbb\template\twig\environment(
$assets_bag,
$config,
$filesystem,
$phpbb_path_helper,
$cache_path,
null,
new \phpbb\template\twig\loader(''),
$phpbb_dispatcher,
[
'cache' => false,
'debug' => false,
'auto_reload' => true,
'autoescape' => false,
]
);
$twig_extension = new \phpbb\template\twig\extension($context, $twig, $lang);
$container->set('template.twig.extensions.phpbb', $twig_extension);
$twig_extensions_collection = new \phpbb\di\service_collection($container);
$twig_extensions_collection->add('template.twig.extensions.phpbb');
$container->set('template.twig.extensions.collection', $twig_extensions_collection);
$twig->addExtension($twig_extension);
$twig_lexer = new \phpbb\template\twig\lexer($twig);
$container->set('template.twig.lexer', $twig_lexer);
$auth = new \phpbb\auth\auth();
$log = new \phpbb\log\log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE);
$container->set('log', $log);
$email_method = new \phpbb\messenger\method\email(
$assets_bag,
$config,
$phpbb_dispatcher,
$lang,
$messenger_queue,
$phpbb_path_helper,
$request,
$twig_extensions_collection,
$twig_lexer,
$user,
$phpbb_root_path,
$cache_path,
$extension_manager,
$log
);
$container->set('messenger.method.email', $email_method);
}
protected static function install_board()
{
global $phpbb_root_path, $phpEx;
@ -374,6 +491,7 @@ class phpbb_functional_test_case extends phpbb_test_case
$container->set('installer.install_finish.notify_user', new phpbb_mock_null_installer_task());
$container->register('installer.install_finish.install_extensions')->setSynthetic(true);
$container->set('installer.install_finish.install_extensions', new phpbb_mock_null_installer_task());
self::get_messenger_method_email($container);
$container->compile();
$language = $container->get('language');