Merge pull request #1519 from cyberalien/ticket/11647

Fix INCLUDEJS URLs handling

phpBB/includes/template/asset.php

@@ -0,0 +1,182 @@
+<?php
+/**
+*
+* @package phpBB3
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+/**
+* @ignore
+*/
+if (!defined('IN_PHPBB'))
+{
+	exit;
+}
+
+class phpbb_template_asset
+{
+	protected $components = array();
+
+	/**
+	* Constructor
+	*
+	* @param string $url URL
+	*/
+	public function __construct($url)
+	{
+		$this->set_url($url);
+	}
+
+	/**
+	* Set URL
+	*
+	* @param string $url URL
+	*/
+	public function set_url($url)
+	{
+		if (version_compare(PHP_VERSION, '5.4.7') < 0 && substr($url, 0, 2) === '//')
+		{
+			// Workaround for PHP 5.4.6 and older bug #62844 - add fake scheme and then remove it
+			$this->components = parse_url('http:' . $url);
+			$this->components['scheme'] = '';
+			return;
+		}
+		$this->components = parse_url($url);
+	}
+
+	/**
+	* Convert URL components into string
+	*
+	* @param array $components URL components
+	* @return string URL
+	*/
+	protected function join_url($components)
+	{
+		$path = '';
+		if (isset($components['scheme']))
+		{
+			$path = $components['scheme'] === '' ? '//' : $components['scheme'] . '://';
+		}
+
+		if (isset($components['user']) || isset($components['pass']))
+		{
+			if ($path === '' && !isset($components['port']))
+			{
+				$path = '//';
+			}
+			$path .= $components['user'];
+			if (isset($components['pass']))
+			{
+				$path .= ':' . $components['pass'];
+			}
+			$path .= '@';
+		}
+
+		if (isset($components['host']))
+		{
+			if ($path === '' && !isset($components['port']))
+			{
+				$path = '//';
+			}
+			$path .= $components['host'];
+			if (isset($components['port']))
+			{
+				$path .= ':' . $components['port'];
+			}
+		}
+
+		if (isset($components['path']))
+		{
+			$path .= $components['path'];
+		}
+
+		if (isset($components['query']))
+		{
+			$path .= '?' . $components['query'];
+		}
+
+		if (isset($components['fragment']))
+		{
+			$path .= '#' . $components['fragment'];
+		}
+
+		return $path;
+	}
+
+	/**
+	* Get URL
+	*
+	* @return string URL
+	*/
+	public function get_url()
+	{
+		return $this->join_url($this->components);
+	}
+
+	/**
+	* Checks if URL is local and relative
+	*
+	* @return boolean True if URL is local and relative
+	*/
+	public function is_relative()
+	{
+		if (empty($this->components) || !isset($this->components['path']))
+		{
+			// Invalid URL
+			return false;
+		}
+		return !isset($this->components['scheme']) && !isset($this->components['host']) && substr($this->components['path'], 0, 1) !== '/';
+	}
+
+	/**
+	* Get path component of current URL
+	*
+	* @return string Path
+	*/
+	public function get_path()
+	{
+		return isset($this->components['path']) ? $this->components['path'] : '';
+	}
+
+	/**
+	* Set path component
+	*
+	* @param string $path Path component
+	* @param boolean $urlencode If true, parts of path should be encoded with rawurlencode()
+	*/
+	public function set_path($path, $urlencode = false)
+	{
+		if ($urlencode)
+		{
+			$paths = explode('/', $path);
+			foreach ($paths as &$dir)
+			{
+				$dir = rawurlencode($dir);
+			}
+			$path = implode('/', $paths);
+		}
+		$this->components['path'] = $path;
+	}
+
+	/**
+	* Add assets_version parameter to URL.
+	* Parameter will not be added if assets_version already exists in URL
+	*
+	* @param string $version Version
+	*/
+	public function add_assets_version($version)
+	{
+		if (!isset($this->components['query']))
+		{
+			$this->components['query'] = 'assets_version=' . $version;
+			return;
+		}
+		$query = $this->components['query'];
+		if (!preg_match('/(^|[&;])assets_version=/', $query))
+		{
+			$this->components['query'] = $query . '&amp;assets_version=' . $version;
+		}
+	}
+}

phpBB/includes/template/twig/node/includeasset.php

@@ -33,26 +33,21 @@ class phpbb_template_twig_node_includeasset extends Twig_Node
 			->write("\$asset_file = ")
 			->subcompile($this->getNode('expr'))
 			->raw(";\n")
-			->write("\$argument_string = \$anchor_string = '';\n")
+			->write("\$asset = new phpbb_template_asset(\$asset_file);\n")
-			->write("if ((\$argument_string_start = strpos(\$asset_file, '?')) !== false) {\n")
+			->write("if (\$asset->is_relative()) {\n")
 			->indent()
-				->write("\$argument_string = substr(\$asset_file, \$argument_string_start);\n")
+				->write("\$asset_path = \$asset->get_path();")
-				->write("\$asset_file = substr(\$asset_file, 0, \$argument_string_start);\n")
+				->write("\$local_file = \$this->getEnvironment()->get_phpbb_root_path() . \$asset_path;\n")
-				->write("if ((\$anchor_string_start = strpos(\$argument_string, '#')) !== false) {\n")
+				->write("if (!file_exists(\$local_file)) {\n")
 				->indent()
-					->write("\$anchor_string = substr(\$argument_string, \$anchor_string_start);\n")
+					->write("\$local_file = \$this->getEnvironment()->getLoader()->getCacheKey(\$asset_path);\n")
-					->write("\$argument_string = substr(\$argument_string, 0, \$anchor_string_start);\n")
+					->write("\$asset->set_path(\$local_file, true);\n")
 				->outdent()
+				->write("\$asset->add_assets_version({$config['assets_version']});\n")
+				->write("\$asset_file = \$asset->get_url();\n")
 				->write("}\n")
 			->outdent()
 			->write("}\n")
-			->write("if (strpos(\$asset_file, '//') !== 0 && strpos(\$asset_file, 'http://') !== 0 && strpos(\$asset_file, 'https://') !== 0 && !file_exists(\$asset_file)) {\n")
-			->indent()
-				->write("\$asset_file = \$this->getEnvironment()->getLoader()->getCacheKey(\$asset_file);\n")
-				->write("\$argument_string .= ((\$argument_string) ? '&' : '?') . 'assets_version={$config['assets_version']}';\n")
-			->outdent()
-			->write("}\n")
-			->write("\$asset_file .= \$argument_string . \$anchor_string;\n")
 			->write("\$context['definition']->append('{$this->get_definition_name()}', '")
 		;
 

tests/template/template_includejs_test.php

@@ -26,19 +26,15 @@ class phpbb_template_template_includejs_test extends phpbb_template_template_tes
 			),
 			array(
 				array('TEST' => 2),
-				'<script type="text/javascript" src="' . $this->test_path . '/templates/parent_and_child.js?assets_version=0&assets_version=1"></script>',
+				'<script type="text/javascript" src="' . $this->test_path . '/templates/parent_and_child.js?assets_version=0"></script>',
 			),
 			array(
 				array('TEST' => 3),
-			'<script type="text/javascript" src="' . $this->test_path . '/templates/parent_and_child.js?test=1&assets_version=0&assets_version=1"></script>',
+				'<script type="text/javascript" src="' . $this->test_path . '/templates/parent_and_child.js?test=1&assets_version=0"></script>',
 			),
 			array(
 				array('TEST' => 4),
-				'<script type="text/javascript" src="' . $this->test_path . '/templates/parent_and_child.js?test=1&amp;assets_version=0&assets_version=1"></script>',
+				'<script type="text/javascript" src="' . $this->test_path . '/templates/parent_and_child.js?test=1&amp;assets_version=0"></script>',
-			),
-			array(
-				array('TEST' => 5),
-				'<script type="text/javascript" src="' . $this->test_path . '/templates/parent_and_child.js?test=1;assets_version=0&assets_version=1"></script>',
 			),
 			array(
 				array('TEST' => 6),
@@ -62,19 +58,15 @@ class phpbb_template_template_includejs_test extends phpbb_template_template_tes
 			),
 			array(
 				array('TEST' => 11),
-				'<script type="text/javascript" src="' . $this->test_path . '/templates/child_only.js?test1=1&amp;test2=2&assets_version=1#test3"></script>',
+				'<script type="text/javascript" src="' . $this->test_path . '/templates/child_only.js?test1=1&amp;test2=2&amp;assets_version=1#test3"></script>',
 			),
 			array(
 				array('TEST' => 12),
-				'<script type="text/javascript" src="' . $this->test_path . '/parent_templates/parent_only.js?test1=1&amp;test2=2&assets_version=1#test3"></script>',
+				'<script type="text/javascript" src="' . $this->test_path . '/parent_templates/parent_only.js?test1=1&amp;test2=2&amp;assets_version=1#test3"></script>',
-			),
-			array(
-				array('TEST' => 13),
-				'<script type="text/javascript" src="' . $this->test_path . '/parent_templates/parent_only.js?test1=1;test2=2&assets_version=1#test3"></script>',
 			),
 			array(
 				array('TEST' => 14),
-				'<script type="text/javascript" src="' . $this->test_path . '/parent_templates/parent_only.js?test1=&quot;&assets_version=1#test3"></script>',
+				'<script type="text/javascript" src="' . $this->test_path . '/parent_templates/parent_only.js?test1=&quot;&amp;assets_version=1#test3"></script>',
 			),
 			array(
 				array('TEST' => 15),
@@ -82,12 +74,16 @@ class phpbb_template_template_includejs_test extends phpbb_template_template_tes
 			),
 			array(
 				array('TEST' => 16),
-				'<script type="text/javascript" src="http://phpbb.com/b.js?c=d&assets_version=1#f"></script>',
+				'<script type="text/javascript" src="http://phpbb.com/b.js?c=d&assets_version=2#f"></script>',
 			),
 			array(
 				array('TEST' => 17),
 				'<script type="text/javascript" src="//phpbb.com/b.js"></script>',
 			),
+			array(
+				array('TEST' => 18),
+				'<script type="text/javascript" src="' . $this->test_path . '/templates/parent_and_child.js?test=1&test2=0&amp;assets_version=1"></script>',
+			),
 		);
 	}
 

tests/template/templates/includejs.html

@@ -6,8 +6,6 @@
 	<!-- INCLUDEJS parent_and_child.js?test=1&assets_version=0 -->
 <!-- ELSEIF TEST === 4 -->
 	<!-- INCLUDEJS parent_and_child.js?test=1&amp;assets_version=0 -->
-<!-- ELSEIF TEST === 5 -->
-	<!-- INCLUDEJS parent_and_child.js?test=1;assets_version=0 -->
 <!-- ELSEIF TEST === 6 -->
 	<!-- INCLUDEJS {PARENT} -->
 <!-- ELSEIF TEST === 7 -->
@@ -24,15 +22,15 @@
 	<!-- INCLUDEJS {$TEST} -->
 <!-- ELSEIF TEST === 12 -->
 	<!-- INCLUDEJS parent_only.js?test1=1&amp;test2=2#test3 -->
-<!-- ELSEIF TEST === 13 -->
-	<!-- INCLUDEJS parent_only.js?test1=1;test2=2#test3 -->
 <!-- ELSEIF TEST === 14 -->
 	<!-- INCLUDEJS parent_only.js?test1=&quot;#test3 -->
 <!-- ELSEIF TEST === 15 -->
 	<!-- INCLUDEJS http://phpbb.com/b.js?c=d#f -->
 <!-- ELSEIF TEST === 16 -->
-	<!-- INCLUDEJS http://phpbb.com/b.js?c=d&assets_version=1#f -->
+	<!-- INCLUDEJS http://phpbb.com/b.js?c=d&assets_version=2#f -->
 <!-- ELSEIF TEST === 17 -->
 	<!-- INCLUDEJS //phpbb.com/b.js -->
+<!-- ELSEIF TEST === 18 -->
+	<!-- INCLUDEJS parent_and_child.js?test=1&test2=0 -->
 <!-- ENDIF -->
 {$SCRIPTS}