Merge pull request #69 from senky/ad-code-analysis

Ad snippet analysis

adm/style/manage_ads.html

@@ -42,7 +42,27 @@ <h3>{{ lang('WARNING') }}</h3>
 			</dl>
 			<dl>
 				<dt><label for="ad_code">{{ lang('AD_CODE') ~ lang('COLON') }}</label><br /><span>{{ lang('AD_CODE_EXPLAIN') }}</span></dt>
-				<dd><textarea id="ad_code" name="ad_code" rows="20" cols="60" style="width: 95%;">{{ AD_CODE }}</textarea></dd>
+				<dd>
+					<textarea id="ad_code" name="ad_code" rows="20" cols="60" style="width: 95%;">{{ AD_CODE }}</textarea>
+					<input type="submit" class="button2" id="analyse_ad_code" name="analyse_ad_code" value="{{ lang('ANALYSE_AD_CODE') }}">
+					<div class="analyser-results">
+						{% if loops.analyser_results_notice or loops.analyser_results_warning %}
+							{% if loops.analyser_results_notice %}
+								{% for notice in loops.analyser_results_notice %}
+									<p class="warningbox">{{ notice.MESSAGE }}</p>
+								{% endfor %}
+							{% endif %}
+
+							{% if loops.analyser_results_warning %}
+								{% for warning in loops.analyser_results_warning %}
+									<p class="errorbox">{{ warning.MESSAGE }}</p>
+								{% endfor %}
+							{% endif %}
+						{% elseif CODE_ANALYSED %}
+							<p class="successbox">{{ lang('EVERYTHING_OK') }}</p>
+						{% endif %}
+					</div>
+				</dd>
 			</dl>
 		</fieldset>
 		<fieldset>

analyser/manager.php

@@ -0,0 +1,79 @@
+<?php
+/**
+ *
+ * Advertisement management. An extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2017 phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ */
+
+namespace phpbb\ads\analyser;
+
+class manager
+{
+	/** @var array Ad code analysis tests */
+	protected $tests;
+
+	/** @var \phpbb\request\request */
+	protected $request;
+
+	/** @var \phpbb\template\template */
+	protected $template;
+
+	/** @var \phpbb\language\language */
+	protected $lang;
+
+	/**
+	 * Construct an ad code analysis manager object
+	 *
+	 * @param	array						$tests		Ad code analysis tests passed via the service container
+	 * @param	\phpbb\request\request		$request	Request object
+	 * @param	\phpbb\template\template	$template	Template object
+	 * @param	\phpbb\language\language	$lang		Language object
+	 */
+	public function __construct($tests, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\language\language $lang)
+	{
+		$this->tests = $tests;
+		$this->request = $request;
+		$this->template = $template;
+		$this->lang = $lang;
+	}
+
+	/**
+	 * Test the ad code for potential problems.
+	 *
+	 * @param	string	$ad_code	Advertisement code
+	 */
+	public function run($ad_code)
+	{
+		$results = array();
+		foreach ($this->tests as $test)
+		{
+			$result = $test->run($ad_code);
+			if ($result !== false)
+			{
+				$results[] = $result;
+			}
+		}
+
+		$this->assign_template_vars($results);
+	}
+
+	/**
+	 * Assign analyser results to template variables.
+	 *
+	 * @param	array	$results	Analyser results
+	 */
+	protected function assign_template_vars($results)
+	{
+		foreach ($results as $result)
+		{
+			$this->template->assign_block_vars('analyser_results_' . $result['severity'], array(
+				'MESSAGE'	=> $this->lang->lang($result['message']),
+			));
+		}
+
+		$this->template->assign_var('CODE_ANALYSED', true);
+	}
+}

analyser/test/alert.php

@@ -0,0 +1,35 @@
+<?php
+/**
+ *
+ * Advertisement management. An extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2017 phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ */
+
+namespace phpbb\ads\analyser\test;
+
+class alert implements test_interface
+{
+	/**
+	 * {@inheritDoc}
+	 *
+	 * Javascript alert() test.
+	 * This test checks for the presence of alert() in an ad code.
+	 * There is no reason why ad would trigger alert, so it's
+	 * categorized as warning.
+	 */
+	public function run($ad_code)
+	{
+		if (preg_match('/alert\s*\(/U', $ad_code))
+		{
+			return array(
+				'severity'	=> 'warning',
+				'message'	=> 'ALERT_USAGE',
+			);
+		}
+
+		return false;
+	}
+}

analyser/test/location_href.php

@@ -0,0 +1,35 @@
+<?php
+/**
+ *
+ * Advertisement management. An extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2017 phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ */
+
+namespace phpbb\ads\analyser\test;
+
+class location_href implements test_interface
+{
+	/**
+	 * {@inheritDoc}
+	 *
+	 * Javascript redirect using window.location.href test.
+	 * This test checks for the presence of redirect in an ad code.
+	 * There is no reason why ad would redirect user to another page,
+	 * so it's categorized as warning.
+	 */
+	public function run($ad_code)
+	{
+		if (preg_match('/location\.href(\s)*=/U', $ad_code))
+		{
+			return array(
+				'severity'	=> 'warning',
+				'message'	=> 'LOCATION_CHANGE',
+			);
+		}
+
+		return false;
+	}
+}

analyser/test/script_without_async.php

@@ -0,0 +1,41 @@
+<?php
+/**
+ *
+ * Advertisement management. An extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2017 phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ */
+
+namespace phpbb\ads\analyser\test;
+
+class script_without_async implements test_interface
+{
+	/**
+	 * {@inheritDoc}
+	 *
+	 * Synchronously loaded scripts test.
+	 * This test looks for scripts that aren't using `async` attribute
+	 * to load itself asynchronously. Such scripts slow down page rendering
+	 * time and should be made asynchronous.
+	 */
+	public function run($ad_code)
+	{
+		if (preg_match_all('/&lt;script(.*)src(.*)&gt;/U', $ad_code, $matches))
+		{
+			foreach ($matches[1] as $match)
+			{
+				if (!preg_match('/ async/', $match))
+				{
+					return array(
+						'severity'	=> 'notice',
+						'message'	=> 'SCRIPT_WITHOUT_ASYNC',
+					);
+				}
+			}
+		}
+
+		return false;
+	}
+}

analyser/test/test_interface.php

@@ -0,0 +1,25 @@
+<?php
+/**
+ *
+ * Advertisement management. An extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2017 phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ */
+
+namespace phpbb\ads\analyser\test;
+
+/**
+ * Interface for ad code analysis tests
+ */
+interface test_interface
+{
+	/**
+	 * Test ad code for potential problems.
+	 *
+	 * @param	string	$ad_code	Advertisement code
+	 * @return	mixed	List of notices and warnings or false when there are none.
+	 */
+	public function run($ad_code);
+}

analyser/test/untrusted_connection.php

@@ -0,0 +1,48 @@
+<?php
+/**
+ *
+ * Advertisement management. An extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2017 phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ */
+
+namespace phpbb\ads\analyser\test;
+
+class untrusted_connection implements test_interface
+{
+	/** @var \phpbb\request\request */
+	protected $request;
+
+	/**
+	 * Construct an ad code analysis manager object
+	 *
+	 * @param \phpbb\request\request $request Request object
+	 */
+	public function __construct(\phpbb\request\request $request)
+	{
+		$this->request = $request;
+	}
+
+	/**
+	 * {@inheritDoc}
+	 *
+	 * Untrusted connection test.
+	 * When board runs on HTTPS and ad tries to load a file from
+	 * HTTP source, browser throws a warning. We should prevent that.
+	 */
+	public function run($ad_code)
+	{
+		$is_https = $this->request->server('HTTPS', false);
+		if ($is_https && preg_match('/http[^s]/', $ad_code))
+		{
+			return array(
+				'severity'	=> 'warning',
+				'message'	=> 'UNSECURE_CONNECTION',
+			);
+		}
+
+		return false;
+	}
+}

config/analyser.yml

@@ -0,0 +1,38 @@
+services:
+    phpbb.ads.analyser.manager:
+        class: phpbb\ads\analyser\manager
+        arguments:
+            - '@phpbb.ads.analyser.test_collection'
+            - '@request'
+            - '@template'
+            - '@language'
+
+# ----- Analyser tests -----
+    phpbb.ads.analyser.test_collection:
+        class: phpbb\di\service_collection
+        arguments:
+            - '@service_container'
+        tags:
+            - { name: service_collection, tag: phpbb.ads.analyser.test }
+
+    phpbb.ads.analyser.test.alert:
+        class: phpbb\ads\analyser\test\alert
+        tags:
+            - { name: phpbb.ads.analyser.test }
+
+    phpbb.ads.analyser.test.location_href:
+        class: phpbb\ads\analyser\test\location_href
+        tags:
+            - { name: phpbb.ads.analyser.test }
+
+    phpbb.ads.analyser.test.script_without_async:
+        class: phpbb\ads\analyser\test\script_without_async
+        tags:
+            - { name: phpbb.ads.analyser.test }
+
+    phpbb.ads.analyser.test.untrusted_connection:
+        class: phpbb\ads\analyser\test\untrusted_connection
+        arguments:
+            - '@request'
+        tags:
+            - { name: phpbb.ads.analyser.test }

config/services.yml

@@ -1,5 +1,6 @@
 imports:
     - { resource: tables.yml }
+    - { resource: analyser.yml }
     - { resource: location.yml }
 
 services:
@@ -31,6 +32,7 @@ services:
             - '@group_helper'
             - '@phpbb.ads.admin.input'
             - '@phpbb.ads.admin.helper'
+            - '@phpbb.ads.analyser.manager'
             - '%core.root_path%'
             - '%core.php_ext%'