Fixes #4555: adds "id" to manifest and, if present, assert that direc…

…tory name matches.

Also documents $plugin in some views to help IDEs with code comprehension

engine/classes/ElggPluginManifest.php

@@ -19,6 +19,8 @@ class ElggPluginManifest {
 
 	/**
 	 * The parser object
+	 *
+	 * @var ElggPluginManifestParser18
 	 */
 	protected $parser;
 
@@ -123,6 +125,8 @@ class ElggPluginManifest {
 	 * @param mixed  $manifest  A string, XmlElement, or path of a manifest file.
 	 * @param string $plugin_id Optional ID of the owning plugin. Used to
 	 *                          fill in some values automatically.
+	 *
+	 * @throws PluginException
 	 */
 	public function __construct($manifest, $plugin_id = null) {
 		if ($plugin_id) {
@@ -133,15 +137,19 @@ public function __construct($manifest, $plugin_id = null) {
 		if ($manifest instanceof XmlElement) {
 			$manifest_obj = $manifest;
 		} else {
+			$raw_xml = '';
 			if (substr(trim($manifest), 0, 1) == '<') {
 				// this is a string
 				$raw_xml = $manifest;
 			} elseif (is_file($manifest)) {
 				// this is a file
 				$raw_xml = file_get_contents($manifest);
 			}
-
-			$manifest_obj = xml_to_object($raw_xml);
+			if ($raw_xml) {
+				$manifest_obj = xml_to_object($raw_xml);
+			} else {
+				$manifest_obj = null;
+			}
 		}
 
 		if (!$manifest_obj) {
@@ -179,8 +187,6 @@ public function __construct($manifest, $plugin_id = null) {
 			throw new PluginException(elgg_echo('PluginException:ParserError',
 						array($this->apiVersion, $this->getPluginID())));
 		}
-
-		return true;
 	}
 
 	/**
@@ -236,6 +242,20 @@ public function getName() {
 		return $name;
 	}
 
+	/**
+	 * Return the plugin ID required by the author. If getPluginID() does
+	 * not match this, the plugin should not be started.
+	 *
+	 * @return string|bool false if not defined
+	 */
+	public function getID() {
+		$id = $this->parser->getAttribute('id');
+		if (is_string($id)) {
+			return trim($id);
+		}
+		return false;
+	}
+
 
 	/**
 	 * Return the description
@@ -264,7 +284,7 @@ public function getBlurb() {
 	/**
 	 * Returns the license
 	 *
-	 * @return sting
+	 * @return string
 	 */
 	public function getLicense() {
 		// license vs licence.  Use license.
@@ -490,6 +510,7 @@ private function normalizeDep($dep) {
 				break;
 		}
 
+		// @todo $struct may not have been defined...
 		$normalized_dep = $this->buildStruct($struct, $dep);
 
 		// normalize comparison operators
@@ -599,8 +620,8 @@ protected function buildStruct(array $struct, array $array) {
 	 * localization is found, returns the category with _ and - converted to ' '
 	 * and then ucwords()'d.
 	 *
-	 * @param str $category The category as defined in the manifest.
-	 * @return str A human-readable category
+	 * @param string $category The category as defined in the manifest.
+	 * @return string A human-readable category
 	 */
 	static public function getFriendlyCategory($category) {
 		$cat_raw_string = "admin:plugins:category:$category";

engine/classes/ElggPluginManifestParser18.php

@@ -13,7 +13,7 @@ class ElggPluginManifestParser18 extends ElggPluginManifestParser {
 	 * @var array
 	 */
 	protected $validAttributes = array(
-		'name', 'author', 'version', 'blurb', 'description',
+		'name', 'author', 'version', 'blurb', 'description', 'id',
 		'website', 'copyright', 'license', 'requires', 'suggests',
 		'screenshot', 'category', 'conflicts', 'provides',
 		'activate_on_install'
@@ -31,7 +31,9 @@ class ElggPluginManifestParser18 extends ElggPluginManifestParser {
 	/**
 	 * Parse a manifest object from 1.8 and later
 	 *
-	 * @return void
+	 * @return bool
+	 *
+	 * @throws PluginException
 	 */
 	public function parse() {
 		$parsed = array();
@@ -43,6 +45,7 @@ public function parse() {
 				case 'name':
 				case 'author':
 				case 'version':
+				case 'id':
 				case 'website':
 				case 'copyright':
 				case 'license':

engine/classes/ElggPluginPackage.php

@@ -162,10 +162,16 @@ public function __construct($plugin, $validate = true) {
 	 * @return bool
 	 */
 	public function isValid() {
-		if (isset($this->valid)) {
-			return $this->valid;
+		if (!isset($this->valid)) {
+			$this->valid = $this->validate();
 		}
+		return $this->valid;
+	}
 
+	/**
+	 * @return bool
+	 */
+	private function validate() {
 		// check required files.
 		$have_req_files = true;
 		foreach ($this->requiredFiles as $file) {
@@ -179,21 +185,45 @@ public function isValid() {
 
 		// check required files
 		if (!$have_req_files) {
-			return $this->valid = false;
+			return false;
 		}
 
 		// check for valid manifest.
 		if (!$this->loadManifest()) {
-			return $this->valid = false;
+			return false;
+		}
+
+		// is plugin directory named correctly?
+		if (!$this->isNamedCorrectly()) {
+			return false;
 		}
 
 		// can't require or conflict with yourself or something you provide.
 		// make sure provides are all valid.
-		if (!$this->isSaneDeps()) {
-			return $this->valid = false;
+		if (!$this->hasSaneDependencies()) {
+			return false;
 		}
 
-		return $this->valid = true;
+		return true;
+	}
+
+	/**
+	 * Check that the plugin is installed in the directory with name specified
+	 * in the manifest's "id" element.
+	 *
+	 * @return bool
+	 */
+	private function isNamedCorrectly() {
+		$manifest = $this->getManifest();
+		if ($manifest) {
+			$required_id = $manifest->getID();
+			if (!empty($required_id) && ($required_id !== $this->id)) {
+				$this->errorMsg =
+					elgg_echo('ElggPluginPackage:InvalidPlugin:InvalidId', array($required_id));
+				return false;
+			}
+		}
+		return true;
 	}
 
 	/**
@@ -207,7 +237,7 @@ public function isValid() {
 	 *
 	 * @return bool
 	 */
-	private function isSaneDeps() {
+	private function hasSaneDependencies() {
 		// protection against plugins with no manifest file
 		if (!$this->getManifest()) {
 			return false;

engine/lib/elgglib.php

@@ -1573,7 +1573,7 @@ function elgg_http_url_is_identical($url1, $url2, $ignore_params = array('offset
  * @param bool   $strict  Return array key if it's set, even if empty. If false,
  *                        return $default if the array key is unset or empty.
  *
- * @return void
+ * @return mixed
  * @since 1.8.0
  */
 function elgg_extract($key, array $array, $default = NULL, $strict = true) {

languages/en.php

@@ -76,6 +76,7 @@
 	'ElggPlugin:NoPluginPackagePackage' => 'Missing ElggPluginPackage for plugin ID %s (guid %s)',
 
 	'ElggPluginPackage:InvalidPlugin:MissingFile' => 'Missing file %s in package',
+	'ElggPluginPackage:InvalidPlugin:InvalidId' => 'This plugin\'s directory must be renamed to "%s" as specified in its manifest.',
 	'ElggPluginPackage:InvalidPlugin:InvalidDependency' => 'Invalid dependency type "%s"',
 	'ElggPluginPackage:InvalidPlugin:InvalidProvides' => 'Invalid provides type "%s"',
 	'ElggPluginPackage:InvalidPlugin:CircularDep' => 'Invalid %s dependency "%s" in plugin %s.  Plugins cannot conflict with or require something they provide!',

views/default/object/plugin/full.php

@@ -13,6 +13,8 @@
  */
 
 $plugin = $vars['entity'];
+/* @var ElggPlugin $plugin */
+
 $reordering = elgg_extract('display_reordering', $vars, false);
 $priority = $plugin->getPriority();
 $active = $plugin->isActive();

views/default/object/plugin/invalid.php

@@ -11,6 +11,7 @@
  */
 
 $plugin = $vars['entity'];
+/* @var ElggPlugin $plugin */
 
 $id = $plugin->getID();
 $path = htmlspecialchars($plugin->getPath());