From 78349ed80f24cb61ad05f997e97d805cc5b0409f Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Fri, 11 Dec 2015 21:31:27 +0100 Subject: [PATCH 1/5] [ticket/14306] Automatically enable a safe mode when container building fails PHPBB3-14306 --- phpBB/adm/style/overall_header.html | 20 +++-- phpBB/includes/functions_acp.php | 4 +- phpBB/phpbb/di/container_builder.php | 125 +++++++++++++++------------ 3 files changed, 85 insertions(+), 64 deletions(-) diff --git a/phpBB/adm/style/overall_header.html b/phpBB/adm/style/overall_header.html index ada88edff2..dbf27eb942 100644 --- a/phpBB/adm/style/overall_header.html +++ b/phpBB/adm/style/overall_header.html @@ -49,7 +49,7 @@ function marklist(id, name, state) } var rb = parent.getElementsByTagName('input'); - + for (var r = 0; r < rb.length; r++) { if (rb[r].name.substr(0, name.length) == name) @@ -103,7 +103,7 @@ function popup(url, width, height, name)

{L_ADMIN_INDEX} • {L_FORUM_INDEX}

{L_SKIP}

- +
    @@ -120,7 +120,7 @@ function popup(url, width, height, name) - +
@@ -129,13 +129,13 @@ function popup(url, width, height, name)
- +
+ +
+

{L_CONTAINER_EXCEPTION}

+

{L_CONTAINER_EXCEPTION_DETAIL}

+

Exception message: {{ CONTAINER_EXCEPTION.getMessage() }}

+ 
{{ CONTAINER_EXCEPTION.getTraceAsString() }}
+
+ diff --git a/phpBB/includes/functions_acp.php b/phpBB/includes/functions_acp.php index 4a52657023..803e4f5e54 100644 --- a/phpBB/includes/functions_acp.php +++ b/phpBB/includes/functions_acp.php @@ -26,7 +26,7 @@ function adm_page_header($page_title) { global $config, $db, $user, $template; global $phpbb_root_path, $phpbb_admin_path, $phpEx, $SID, $_SID; - global $phpbb_dispatcher; + global $phpbb_dispatcher, $phpbb_container; if (defined('HEADER_INC')) { @@ -105,6 +105,8 @@ function adm_page_header($page_title) 'S_CONTENT_ENCODING' => 'UTF-8', 'S_CONTENT_FLOW_BEGIN' => ($user->lang['DIRECTION'] == 'ltr') ? 'left' : 'right', 'S_CONTENT_FLOW_END' => ($user->lang['DIRECTION'] == 'ltr') ? 'right' : 'left', + + 'CONTAINER_EXCEPTION' => $phpbb_container->hasParameter('container_exception') ? $phpbb_container->getParameter('container_exception') : false, )); // An array of http headers that phpbb will set. The following event may override these. diff --git a/phpBB/phpbb/di/container_builder.php b/phpBB/phpbb/di/container_builder.php index 8f175c966c..95d6483e34 100644 --- a/phpBB/phpbb/di/container_builder.php +++ b/phpBB/phpbb/di/container_builder.php @@ -90,7 +90,7 @@ class container_builder * * @var array */ - protected $custom_parameters = null; + protected $custom_parameters = []; /** * @var \phpbb\config_php_file @@ -126,67 +126,80 @@ class container_builder */ public function get_container() { - $container_filename = $this->get_container_filename(); - $config_cache = new ConfigCache($container_filename, defined('DEBUG')); - if ($this->use_cache && $config_cache->isFresh()) - { - require($config_cache->getPath()); - $this->container = new \phpbb_cache_container(); - } - else - { - $this->container_extensions = array(new extension\core($this->get_config_path())); - - if ($this->use_extensions) + try { + $container_filename = $this->get_container_filename(); + $config_cache = new ConfigCache($container_filename, defined('DEBUG')); + if ($this->use_cache && $config_cache->isFresh()) { - $this->load_extensions(); + require($config_cache->getPath()); + $this->container = new \phpbb_cache_container(); } - - // Inject the config - if ($this->config_php_file) + else { - $this->container_extensions[] = new extension\config($this->config_php_file); - } + $this->container_extensions = array(new extension\core($this->get_config_path())); - $this->container = $this->create_container($this->container_extensions); - - // Easy collections through tags - $this->container->addCompilerPass(new pass\collection_pass()); - - // Event listeners "phpBB style" - $this->container->addCompilerPass(new RegisterListenersPass('dispatcher', 'event.listener_listener', 'event.listener')); - - // Event listeners "Symfony style" - $this->container->addCompilerPass(new RegisterListenersPass('dispatcher')); - - if ($this->use_extensions) - { - $this->register_ext_compiler_pass(); - } - - $filesystem = new filesystem(); - $loader = new YamlFileLoader($this->container, new FileLocator($filesystem->realpath($this->get_config_path()))); - $loader->load($this->container->getParameter('core.environment') . '/config.yml'); - - $this->inject_custom_parameters(); - - if ($this->compile_container) - { - $this->container->compile(); - - if ($this->use_cache) + if ($this->use_extensions) { - $this->dump_container($config_cache); + $this->load_extensions(); + } + + // Inject the config + if ($this->config_php_file) + { + $this->container_extensions[] = new extension\config($this->config_php_file); + } + + $this->container = $this->create_container($this->container_extensions); + + // Easy collections through tags + $this->container->addCompilerPass(new pass\collection_pass()); + + // Event listeners "phpBB style" + $this->container->addCompilerPass(new RegisterListenersPass('dispatcher', 'event.listener_listener', 'event.listener')); + + // Event listeners "Symfony style" + $this->container->addCompilerPass(new RegisterListenersPass('dispatcher')); + + if ($this->use_extensions) + { + $this->register_ext_compiler_pass(); + } + + $filesystem = new filesystem(); + $loader = new YamlFileLoader($this->container, new FileLocator($filesystem->realpath($this->get_config_path()))); + $loader->load($this->container->getParameter('core.environment') . '/config.yml'); + + $this->inject_custom_parameters(); + + if ($this->compile_container) + { + $this->container->compile(); + + if ($this->use_cache) + { + $this->dump_container($config_cache); + } } } - } - if ($this->compile_container && $this->config_php_file) + if ($this->compile_container && $this->config_php_file) + { + $this->container->set('config.php', $this->config_php_file); + } + + return $this->container; + } + catch (\Exception $e) { - $this->container->set('config.php', $this->config_php_file); + return $this + ->without_extensions() + ->without_cache() + ->with_custom_parameters(array_merge($this->custom_parameters, [ + 'container_exception' => $e, + ])) + ->get_container() + ; } - - return $this->container; } /** @@ -451,13 +464,11 @@ class container_builder */ protected function inject_custom_parameters() { - if ($this->custom_parameters !== null) + foreach ($this->custom_parameters as $key => $value) { - foreach ($this->custom_parameters as $key => $value) - { - $this->container->setParameter($key, $value); - } + $this->container->setParameter($key, $value); } + } /** From 761fa9da52e92efafa6839938113ddb55cd85f17 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Tue, 15 Dec 2015 20:13:59 +0100 Subject: [PATCH 2/5] [ticket/14306] Doesn't try to build a "safe" container in the dev env PHPBB3-14306 --- phpBB/phpbb/di/container_builder.php | 30 +++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/phpBB/phpbb/di/container_builder.php b/phpBB/phpbb/di/container_builder.php index 95d6483e34..6e6fb5c7fb 100644 --- a/phpBB/phpbb/di/container_builder.php +++ b/phpBB/phpbb/di/container_builder.php @@ -113,7 +113,7 @@ class container_builder * @param string $phpbb_root_path Path to the phpbb includes directory. * @param string $php_ext php file extension */ - function __construct($phpbb_root_path, $php_ext) + public function __construct($phpbb_root_path, $php_ext) { $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; @@ -191,14 +191,26 @@ class container_builder } catch (\Exception $e) { - return $this - ->without_extensions() - ->without_cache() - ->with_custom_parameters(array_merge($this->custom_parameters, [ - 'container_exception' => $e, - ])) - ->get_container() - ; + // Don't try to recover if we are in the development environment + if ($this->get_environment() === 'development') { + throw $e; + } + + try + { + return $this + ->without_extensions() + ->without_cache() + ->with_custom_parameters(array_merge($this->custom_parameters, [ + 'container_exception' => $e, + ])) + ->get_container(); + } + catch (\Exception $_) + { + // Rethrow the original exception if it's still failing + throw $e; + } } } From 7f8b6c02c6380d44d2bc3c402cfa5e9953d4819b Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Tue, 15 Dec 2015 20:18:55 +0100 Subject: [PATCH 3/5] [ticket/14306] Update the error message PHPBB3-14306 --- phpBB/adm/style/overall_header.html | 3 +-- phpBB/language/en/acp/common.php | 3 +++ phpBB/phpbb/di/container_builder.php | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/phpBB/adm/style/overall_header.html b/phpBB/adm/style/overall_header.html index dbf27eb942..4212f01f15 100644 --- a/phpBB/adm/style/overall_header.html +++ b/phpBB/adm/style/overall_header.html @@ -149,8 +149,7 @@ function popup(url, width, height, name)

{L_CONTAINER_EXCEPTION}

-

{L_CONTAINER_EXCEPTION_DETAIL}

-

Exception message: {{ CONTAINER_EXCEPTION.getMessage() }}

+

{L_EXCEPTION}{L_COLON} {{ CONTAINER_EXCEPTION.getMessage() }}

{{ CONTAINER_EXCEPTION.getTraceAsString() }}
diff --git a/phpBB/language/en/acp/common.php b/phpBB/language/en/acp/common.php index a1e2d1ef40..24db3f31fe 100644 --- a/phpBB/language/en/acp/common.php +++ b/phpBB/language/en/acp/common.php @@ -225,6 +225,9 @@ $lang = array_merge($lang, array( 'BACK' => 'Back', + 'CONTAINER_EXCEPTION' => 'phpBB encountered an error building the container due to an installed extension. For this reason, all extensions have been temporarily disabled. Please try purging your forum cache. All extensions will automatically be re-enabled once the container error is resolved. If this error continues, please visit phpBB.com for support.', + 'EXCEPTION' => 'Exception', + 'COLOUR_SWATCH' => 'Web-safe colour swatch', 'CONFIG_UPDATED' => 'Configuration updated successfully.', 'CRON_LOCK_ERROR' => 'Could not obtain cron lock.', diff --git a/phpBB/phpbb/di/container_builder.php b/phpBB/phpbb/di/container_builder.php index 6e6fb5c7fb..d90f78c0d9 100644 --- a/phpBB/phpbb/di/container_builder.php +++ b/phpBB/phpbb/di/container_builder.php @@ -192,7 +192,8 @@ class container_builder catch (\Exception $e) { // Don't try to recover if we are in the development environment - if ($this->get_environment() === 'development') { + if ($this->get_environment() === 'development') + { throw $e; } From 6594ef8b1e0b63f911fbcc2b8859fcfb09977e2f Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Sat, 9 Jan 2016 15:32:11 +0100 Subject: [PATCH 4/5] [ticket/14306] CS and correctly handle exception loop PHPBB3-14306 --- phpBB/adm/style/overall_header.html | 8 ++++---- phpBB/phpbb/di/container_builder.php | 14 ++++++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/phpBB/adm/style/overall_header.html b/phpBB/adm/style/overall_header.html index 4212f01f15..3feb94f89c 100644 --- a/phpBB/adm/style/overall_header.html +++ b/phpBB/adm/style/overall_header.html @@ -146,10 +146,10 @@ function popup(url, width, height, name)
- + {% if CONTAINER_EXCEPTION !== false %}
-

{L_CONTAINER_EXCEPTION}

-

{L_EXCEPTION}{L_COLON} {{ CONTAINER_EXCEPTION.getMessage() }}

+

{{ lang('CONTAINER_EXCEPTION') }}

+

{{ lang('EXCEPTION') }}{{ lang('COLON') }} {{ CONTAINER_EXCEPTION.getMessage() }}

{{ CONTAINER_EXCEPTION.getTraceAsString() }}
- + {% endif %} diff --git a/phpBB/phpbb/di/container_builder.php b/phpBB/phpbb/di/container_builder.php index d90f78c0d9..433847b285 100644 --- a/phpBB/phpbb/di/container_builder.php +++ b/phpBB/phpbb/di/container_builder.php @@ -107,6 +107,9 @@ class container_builder */ private $container_extensions; + /** @var \Exception */ + private $build_exception; + /** * Constructor * @@ -126,7 +129,8 @@ class container_builder */ public function get_container() { - try { + try + { $container_filename = $this->get_container_filename(); $config_cache = new ConfigCache($container_filename, defined('DEBUG')); if ($this->use_cache && $config_cache->isFresh()) @@ -197,8 +201,10 @@ class container_builder throw $e; } - try + if ($this->build_exception === null) { + $this->build_exception = $e; + return $this ->without_extensions() ->without_cache() @@ -207,10 +213,10 @@ class container_builder ])) ->get_container(); } - catch (\Exception $_) + else { // Rethrow the original exception if it's still failing - throw $e; + throw $this->build_exception; } } } From 1e0340b0cf7cc34094e03985457f1aed2f60da36 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Sat, 9 Jan 2016 18:03:21 +0100 Subject: [PATCH 5/5] =?UTF-8?q?[ticket/14306]=C2=A0Improves=20Error=20disp?= =?UTF-8?q?lay?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PHPBB3-14306 --- phpBB/adm/style/overall_header.html | 2 +- phpBB/language/en/acp/common.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/adm/style/overall_header.html b/phpBB/adm/style/overall_header.html index 3feb94f89c..9266372ab8 100644 --- a/phpBB/adm/style/overall_header.html +++ b/phpBB/adm/style/overall_header.html @@ -148,7 +148,7 @@ function popup(url, width, height, name)
{% if CONTAINER_EXCEPTION !== false %}
-

{{ lang('CONTAINER_EXCEPTION') }}

+

{{ lang('CONTAINER_EXCEPTION') }}


{{ lang('EXCEPTION') }}{{ lang('COLON') }} {{ CONTAINER_EXCEPTION.getMessage() }}

{{ CONTAINER_EXCEPTION.getTraceAsString() }}
diff --git a/phpBB/language/en/acp/common.php b/phpBB/language/en/acp/common.php index 24db3f31fe..4a70aafc6f 100644 --- a/phpBB/language/en/acp/common.php +++ b/phpBB/language/en/acp/common.php @@ -225,7 +225,7 @@ $lang = array_merge($lang, array( 'BACK' => 'Back', - 'CONTAINER_EXCEPTION' => 'phpBB encountered an error building the container due to an installed extension. For this reason, all extensions have been temporarily disabled. Please try purging your forum cache. All extensions will automatically be re-enabled once the container error is resolved. If this error continues, please visit phpBB.com for support.', + 'CONTAINER_EXCEPTION' => 'phpBB encountered an error building the container due to an installed extension. For this reason, all extensions have been temporarily disabled. Please try purging your forum cache. All extensions will automatically be re-enabled once the container error is resolved. If this error continues, please visit phpBB.com for support.', 'EXCEPTION' => 'Exception', 'COLOUR_SWATCH' => 'Web-safe colour swatch',