[ticket/15531] Log malformed BBCodes

PHPBB3-15531
author: JoshyPHP <s9e.dev@gmail.com> 2018-01-22 03:34:47 +0100
committer: JoshyPHP <s9e.dev@gmail.com> 2018-01-22 03:34:47 +0100
commit: 531d9dfa1f0dddfe765a92ca40c77015091ecc5e (patch)
tree: 071817862fb1559727816d8c18c3fc5eccbe1dab
parent: d0143bec8830e709047443f73f79c0dce438e3fc (diff)
download: forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.tar
forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.tar.gz
forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.tar.bz2
forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.tar.xz
forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.zip
6 files changed, 84 insertions, 15 deletions
diff --git a/phpBB/config/default/container/services_text_formatter.yml b/phpBB/config/default/container/services_text_formatter.yml
index 74624ea4e4..07087cd4a9 100644
--- a/phpBB/config/default/container/services_text_formatter.yml
+++ b/phpBB/config/default/container/services_text_formatter.yml
@@ -39,6 +39,7 @@ services:
             - '@dispatcher'
             - '@config'
             - '@text_formatter.s9e.link_helper'
+            - '@log'
             - '%text_formatter.cache.dir%'
             - '%text_formatter.cache.parser.key%'
             - '%text_formatter.cache.renderer.key%'
diff --git a/phpBB/language/en/acp/common.php b/phpBB/language/en/acp/common.php
index de5039f047..0d5f6fee25 100644
--- a/phpBB/language/en/acp/common.php
+++ b/phpBB/language/en/acp/common.php
@@ -552,6 +552,7 @@ $lang = array_merge($lang, array(
 	'LOG_BBCODE_ADD'		=> '<strong>Added new BBCode</strong><br />» %s',
 	'LOG_BBCODE_EDIT'		=> '<strong>Edited BBCode</strong><br />» %s',
 	'LOG_BBCODE_DELETE'		=> '<strong>Deleted BBCode</strong><br />» %s',
+	'LOG_BBCODE_CONFIGURATION_ERROR'	=> '<strong>Error while configuring BBCode</strong>: %1$s<br />» %2$s',
 
 	'LOG_BOT_ADDED'		=> '<strong>New bot added</strong><br />» %s',
 	'LOG_BOT_DELETE'	=> '<strong>Deleted bot</strong><br />» %s',
diff --git a/phpBB/phpbb/textformatter/s9e/factory.php b/phpBB/phpbb/textformatter/s9e/factory.php
index 1e85856898..2b11a79192 100644
--- a/phpBB/phpbb/textformatter/s9e/factory.php
+++ b/phpBB/phpbb/textformatter/s9e/factory.php
@@ -13,6 +13,7 @@
 
 namespace phpbb\textformatter\s9e;
 
+use Exception;
 use s9e\TextFormatter\Configurator;
 use s9e\TextFormatter\Configurator\Items\AttributeFilters\RegexpFilter;
 use s9e\TextFormatter\Configurator\Items\UnsafeTemplate;
@@ -132,6 +133,11 @@ class factory implements \phpbb\textformatter\cache_interface
 	protected $dispatcher;
 
 	/**
+	* @var \phpbb\log\log_interface
+	*/
+	protected $log;
+
+	/**
 	* Constructor
 	*
 	* @param \phpbb\textformatter\data_access $data_access
@@ -139,11 +145,12 @@ class factory implements \phpbb\textformatter\cache_interface
 	* @param \phpbb\event\dispatcher_interface $dispatcher
 	* @param \phpbb\config\config $config
 	* @param \phpbb\textformatter\s9e\link_helper $link_helper
+	* @param \phpbb\log\log_interface $log
 	* @param string $cache_dir          Path to the cache dir
 	* @param string $cache_key_parser   Cache key used for the parser
 	* @param string $cache_key_renderer Cache key used for the renderer
 	*/
-	public function __construct(\phpbb\textformatter\data_access $data_access, \phpbb\cache\driver\driver_interface $cache, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\config\config $config, \phpbb\textformatter\s9e\link_helper $link_helper, $cache_dir, $cache_key_parser, $cache_key_renderer)
+	public function __construct(\phpbb\textformatter\data_access $data_access, \phpbb\cache\driver\driver_interface $cache, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\config\config $config, \phpbb\textformatter\s9e\link_helper $link_helper, \phpbb\log\log_interface $log, $cache_dir, $cache_key_parser, $cache_key_renderer)
 	{
 		$this->link_helper = $link_helper;
 		$this->cache = $cache;
@@ -153,6 +160,7 @@ class factory implements \phpbb\textformatter\cache_interface
 		$this->config = $config;
 		$this->data_access = $data_access;
 		$this->dispatcher = $dispatcher;
+		$this->log = $log;
 	}
 
 	/**
@@ -272,7 +280,7 @@ class factory implements \phpbb\textformatter\cache_interface
 		// Add default BBCodes
 		foreach ($this->get_default_bbcodes($configurator) as $bbcode)
 		{
-			$configurator->BBCodes->addCustom($bbcode['usage'], new UnsafeTemplate($bbcode['template']));
+			$this->add_bbcode($configurator, $bbcode['usage'], $bbcode['template']);
 		}
 		if (isset($configurator->tags['QUOTE']))
 		{
@@ -299,17 +307,7 @@ class factory implements \phpbb\textformatter\cache_interface
 				},
 				$row['bbcode_tpl']
 			);
-
-			try
-			{
-				$configurator->BBCodes->addCustom($row['bbcode_match'], new UnsafeTemplate($tpl));
-			}
-			catch (\Exception $e)
-			{
-				/**
-				* @todo log an error?
-				*/
-			}
+			$this->add_bbcode($configurator, $row['bbcode_match'], $tpl);
 		}
 
 		// Load smilies
@@ -419,6 +417,26 @@ class factory implements \phpbb\textformatter\cache_interface
 	}
 
 	/**
+	* Add a BBCode to given configurator
+	*
+	* @param  Configurator $configurator
+	* @param  string       $usage
+	* @param  string       $template
+	* @return void
+	*/
+	protected function add_bbcode(Configurator $configurator, $usage, $template)
+	{
+		try
+		{
+			$configurator->BBCodes->addCustom($usage, new UnsafeTemplate($template));
+		}
+		catch (Exception $e)
+		{
+			$this->log->add('critical', null, null, 'LOG_BBCODE_CONFIGURATION_ERROR', false, [$usage, $e->getMessage()]);
+		}
+	}
+
+	/**
 	* Configure the Autolink / Autoemail plugins used to linkify text
 	*
 	* @param  \s9e\TextFormatter\Configurator $configurator
diff --git a/tests/test_framework/phpbb_test_case_helpers.php b/tests/test_framework/phpbb_test_case_helpers.php
index 7fb9a740b8..c792976b1e 100644
--- a/tests/test_framework/phpbb_test_case_helpers.php
+++ b/tests/test_framework/phpbb_test_case_helpers.php
@@ -385,7 +385,7 @@ class phpbb_test_case_helpers
 		$mb = $this->test_case->getMockBuilder('phpbb\\textformatter\\data_access');
 		$mb->setMethods(array('get_bbcodes', 'get_censored_words', 'get_smilies', 'get_styles'));
 		$mb->setConstructorArgs(array(
-			$this->test_case->getMock('phpbb\\db\\driver\\driver'),
+			$this->test_case->getMockBuilder('phpbb\\db\\driver\\driver')->getMock(),
 			'phpbb_bbcodes',
 			'phpbb_smilies',
 			'phpbb_styles',
@@ -489,8 +489,11 @@ class phpbb_test_case_helpers
 			$request = new phpbb_mock_request;
 		}
 
+		// Get a log interface
+		$log = ($container->has('log')) ? $container->get('log') : $this->test_case->getMockBuilder('phpbb\\log\\log_interface')->getMock();
+
 		// Create and register the text_formatter.s9e.factory service
-		$factory = new \phpbb\textformatter\s9e\factory($dal, $cache, $dispatcher, $config, new \phpbb\textformatter\s9e\link_helper, $cache_dir, $cache_key_parser, $cache_key_renderer);
+		$factory = new \phpbb\textformatter\s9e\factory($dal, $cache, $dispatcher, $config, new \phpbb\textformatter\s9e\link_helper, $log, $cache_dir, $cache_key_parser, $cache_key_renderer);
 		$container->set('text_formatter.s9e.factory', $factory);
 
 		// Create a user if none was provided, and add the common lang strings
diff --git a/tests/text_formatter/s9e/factory_test.php b/tests/text_formatter/s9e/factory_test.php
index d35330a975..0d780a19a9 100644
--- a/tests/text_formatter/s9e/factory_test.php
+++ b/tests/text_formatter/s9e/factory_test.php
@@ -56,6 +56,7 @@ class phpbb_textformatter_s9e_factory_test extends phpbb_database_test_case
 			$this->dispatcher,
 			new \phpbb\config\config(array('allowed_schemes_links' => 'http,https,ftp')),
 			new \phpbb\textformatter\s9e\link_helper,
+			$this->getMockBuilder('phpbb\\log\\log_interface')->getMock(),
 			$this->get_cache_dir(),
 			'_foo_parser',
 			'_foo_renderer'
@@ -264,6 +265,23 @@ class phpbb_textformatter_s9e_factory_test extends phpbb_database_test_case
 	}
 
 	/**
+	* @testdox Logs malformed BBCodes
+	*/
+	public function test_malformed_bbcodes()
+	{
+		$log = $this->getMockBuilder('phpbb\\log\\log_interface')->getMock();
+		$log->expects($this->once())
+			->method('add')
+			->with('critical', null, null, 'LOG_BBCODE_CONFIGURATION_ERROR', false, ['[x !x]{TEXT}[/x]', 'Cannot interpret the BBCode definition']);
+
+		$container = new phpbb_mock_container_builder;
+		$container->set('log', $log);
+
+		$fixture   = __DIR__ . '/fixtures/malformed_bbcode.xml';
+		$this->get_test_case_helpers()->set_s9e_services($container, $fixture);
+	}
+
+	/**
 	* @testdox get_configurator() triggers events before and after configuration
 	*/
 	public function test_configure_events()
diff --git a/tests/text_formatter/s9e/fixtures/malformed_bbcode.xml b/tests/text_formatter/s9e/fixtures/malformed_bbcode.xml
new file mode 100644
index 0000000000..7e7aa1a39c
--- /dev/null
+++ b/tests/text_formatter/s9e/fixtures/malformed_bbcode.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dataset>
+	<table name="phpbb_bbcodes">
+		<column>bbcode_id</column>
+		<column>bbcode_tag</column>
+		<column>bbcode_helpline</column>
+		<column>display_on_posting</column>
+		<column>bbcode_match</column>
+		<column>bbcode_tpl</column>
+		<column>first_pass_match</column>
+		<column>first_pass_replace</column>
+		<column>second_pass_match</column>
+		<column>second_pass_replace</column>
+
+		<row>
+			<value>13</value>
+			<value>x</value>
+			<value></value>
+			<value>1</value>
+			<value>[x !x]{TEXT}[/x]</value>
+			<value>...</value>
+			<value/>
+			<value/>
+			<value/>
+			<value/>
+		</row>
+	</table>
+</dataset>
author	JoshyPHP <s9e.dev@gmail.com>	2018-01-22 03:34:47 +0100
committer	JoshyPHP <s9e.dev@gmail.com>	2018-01-22 03:34:47 +0100
commit	531d9dfa1f0dddfe765a92ca40c77015091ecc5e (patch)
tree	071817862fb1559727816d8c18c3fc5eccbe1dab
parent	d0143bec8830e709047443f73f79c0dce438e3fc (diff)
download	forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.tar forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.tar.gz forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.tar.bz2 forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.tar.xz forums-531d9dfa1f0dddfe765a92ca40c77015091ecc5e.zip