Old HTML ampersand strict compatibility

- FIX: e107::url() now puts "&" in the query string instead of "&" for compliance with the older,
       looser definition of ambiguous ampersands in the HTML specification.
       Fixes: #4054
- FIX: Typo in comment
- FIX: Clear the core/e107/addons/e_url registry (cache) because if a plugin is installed after that
       cache is initialized, the cache is not updated anymore. The plugin's e_url is therefore not
       loaded, so SEF URLs won't be generated for that plugin until the cache is regenerated.
- NEW: Test for #4054
- FIX: e_pluginTest::testGetFields() expects the initial condition of the "forum" plugin to be
       uninstalled.

e107_handlers/e107_class.php

@@ -3474,7 +3474,7 @@ class e107
 
 		$active = true;
 
-		foreach ($matches[1] as $k => $v) // check if a field value is missing, if so, revent to legacy url.
+		foreach ($matches[1] as $k => $v) // check if a field value is missing, if so, revert to legacy url.
 		{
 			if (!isset($row[$v])) {
 				self::getMessage()->addDebug("Missing value for " . $v . " in " . $plugin . "/e_url.php - '" . $key . "'");
@@ -3489,7 +3489,6 @@ class e107
 			$active = false;
 		}
 
-
 		if (deftrue('e_MOD_REWRITE') && ($active == true) && empty($options['legacy']))  // Search-Engine-Friendly URLs active.
 		{
 			$rawUrl = $tp->simpleParse($tmp[$plugin][$key]['sef'], $row);
@@ -3501,13 +3500,6 @@ class e107
 			} else {
 				$sefUrl = e_HTTP . $rawUrl;
 			}
-
-			// Append the query.
-			if (is_array($options['query']) && !empty($options['query'])) {
-				$sefUrl .= (strpos($sefUrl, '?') !== FALSE ? '&' : '?') . self::httpBuildQuery($options['query']);
-			}
-
-			return $sefUrl . $options['fragment'];
 		} else // Legacy URL.
 		{
 
@@ -3551,15 +3543,15 @@ class e107
 
 				}
 			}
+			$sefUrl = $legacyUrl;
+		}
 
 		// Append the query.
 		if (is_array($options['query']) && !empty($options['query'])) {
-
+			$sefUrl .= (strpos($sefUrl, '?') !== FALSE ? '&' : '?') . self::httpBuildQuery($options['query']);
-				$legacyUrl .= (strpos($legacyUrl, '?') !== FALSE ? '&' : '?') . self::httpBuildQuery($options['query']);
 		}
 
-			return $legacyUrl . $options['fragment'];
+		return $sefUrl . $options['fragment'];
-		}
 	}
 
 
@@ -3631,7 +3623,7 @@ class e107
 			}
 		}
 
-		return implode('&', $params);
+		return implode('&', $params);
 	}
 
 

e107_handlers/plugin_class.php

@@ -140,6 +140,7 @@ class e_plugin
 	{
 		$this->_installed = array();
 		$this->_addons = array();
+		e107::setRegistry('core/e107/addons/e_url');
 
 		$this->_init(true);
 		$this->_initIDs();

e107_tests/tests/unit/e107Test.php

@@ -1,5 +1,5 @@
 <?php
-	/**
+/**
  * e107 website system
  *
  * Copyright (C) 2008-2018 e107 Inc (e107.org)
@@ -9,8 +9,8 @@
  */
 
 
-	class e107Test extends \Codeception\Test\Unit
+class e107Test extends \Codeception\Test\Unit
-	{
+{
 
 	/** @var e107 */
 	private $e107;
@@ -49,7 +49,7 @@
 
 	}
 
-/*
+	/*
 			public function testInitInstall()
 			{
 				$res = null;
@@ -295,7 +295,7 @@
 		$this->assertTrue($res);
 
 	}
-/*
+	/*
 			public function testGetSession()
 			{
 				$res = null;
@@ -685,7 +685,7 @@
 				$res = null;
 				$this->assertTrue($res);
 			}
-*/
+	*/
 	/**
 	 * This test checks getTemplate() use on loading between the core download plugin template and the _blank theme download template
 	 */
@@ -717,7 +717,7 @@
 
 
 	}
-/*
+	/*
 			public function testTemplateWrapper()
 			{
 				$res = null;
@@ -783,7 +783,7 @@
 				$res = null;
 				$this->assertTrue($res);
 			}
-*/
+	*/
 	public function testUrl()
 	{
 		$obj = $this->e107;
@@ -791,12 +791,27 @@
 		$result = $obj::url('news','index', array(), array('mode'=>'full'));
 
 		$this->assertEquals("https://localhost/e107/news", $result);
-		//	var_dump(SITEURL);
-
-
-		//	$this->assertTrue($res);
 	}
-/*
+
+	/**
+	 * @see https://github.com/e107inc/e107/issues/4054
+	 */
+	public function testUrlOptionQueryHasCompliantAmpersand()
+	{
+		$e107 = $this->e107;
+		$e107::getPlugin()->install('forum');
+		$url = $e107::url('forum', 'topic', [], array(
+			'query' => array(
+				'f' => 'post',
+				'id' => 123
+			),
+		));
+		$this->assertEquals(
+			e_PLUGIN_ABS. 'forum/forum_viewtopic.php?f=post&amp;id=123',
+			$url, "Generated href does not match expectation"
+			);
+	}
+	/*
 			public function testRedirect()
 			{
 				$res = null;
@@ -838,7 +853,7 @@
 				$res = null;
 				$this->assertTrue($res);
 			}
-*/
+	*/
 
 	public function testBase64DecodeOnAjaxURL()
 	{
@@ -866,7 +881,7 @@
 		// 	$res = null;
 		// $this->assertTrue($res);
 	}
-/*
+	/*
 			public function testSet_base_path()
 			{
 				$res = null;
@@ -962,7 +977,7 @@
 				$res = null;
 				$this->assertTrue($res);
 			}
-*/
+	*/
 	public function testIsInstalled()
 	{
 		$obj = $this->e107;
@@ -977,7 +992,7 @@
 		// var_dump($result);
 		$this->assertTrue($result);
 	}
-/*
+	/*
 			public function testIni_set()
 			{
 				$res = null;
@@ -1015,5 +1030,5 @@
 			}
 
 
-*/
+	*/
-	}
+}

e107_tests/tests/unit/e_pluginTest.php

@@ -227,6 +227,7 @@
 
 		public function testGetFields()
 		{
+			e107::getPlugin()->uninstall('forum');
 			$result = $this->ep->clearCache()->load('forum')->getFields(true);
 
 		//	print_r($result);