Permalink
Browse files

Merge pull request #267 from mrclay/4555-manifest-id

Fixes #4555: adds "id" to manifest and, if present, require dir name to match
  • Loading branch information...
2 parents 36ff8be + 8d55a9f commit 029109b7e9c6e8610d172f827716863aba65b8c5 @cash cash committed Jun 30, 2012
@@ -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,16 @@ 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 empty string if not empty/not defined
+ */
+ public function getID() {
+ return trim((string) $this->parser->getAttribute('id'));
+ }
+
/**
* Return the description
@@ -264,7 +280,7 @@ public function getBlurb() {
/**
* Returns the license
*
- * @return sting
+ * @return string
*/
public function getLicense() {
// license vs licence. Use license.
@@ -504,6 +520,7 @@ private function normalizeDep($dep) {
break;
}
+ // @todo $struct may not have been defined...
$normalized_dep = $this->buildStruct($struct, $dep);
// normalize comparison operators
@@ -613,8 +630,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";
@@ -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':
@@ -162,38 +162,60 @@ 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) {
if (!is_readable($this->path . $file)) {
- $have_req_files = false;
$this->errorMsg =
elgg_echo('ElggPluginPackage:InvalidPlugin:MissingFile', array($file));
- break;
+ return false;
}
}
- // check required files
- if (!$have_req_files) {
- return $this->valid = false;
- }
-
// check for valid manifest.
if (!$this->loadManifest()) {
- return $this->valid = false;
+ return false;
+ }
+
+ 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,14 +229,16 @@ public function isValid() {
*
* @return bool
*/
- private function isSaneDeps() {
+ private function hasSaneDependencies() {
// protection against plugins with no manifest file
if (!$this->getManifest()) {
return false;
}
+ // @todo these two vars are unused; should they be?
$conflicts = $this->getManifest()->getConflicts();
$requires = $this->getManifest()->getRequires();
+
$provides = $this->getManifest()->getProvides();
foreach ($provides as $provide) {
View
@@ -1575,7 +1575,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) {
View
@@ -77,6 +77,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" to match the ID 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!',
@@ -12,7 +12,9 @@
* @subpackage Plugins
*/
+/* @var ElggPlugin $plugin */
$plugin = $vars['entity'];
+
$reordering = elgg_extract('display_reordering', $vars, false);
$priority = $plugin->getPriority();
$active = $plugin->isActive();
@@ -10,6 +10,7 @@
* @subpackage Plugins
*/
+/* @var ElggPlugin $plugin */
$plugin = $vars['entity'];
$id = $plugin->getID();

0 comments on commit 029109b

Please sign in to comment.