Merge pull request #204 from dryabov/patch-71

Improve validation of XML manifests

administrator/components/com_jedchecker/language/en-GB/en-GB.com_jedchecker.ini

@@ -117,10 +117,14 @@ COM_JEDCHECKER_MANIFEST_MENU_UNUSED_ATTRIBUTE="Menu item attribute '%s' is not u
 COM_JEDCHECKER_MANIFEST_MISSED_METHOD_UPGRADE="Without the method="upgrade" attribute the extension package cannot be upgraded"
 COM_JEDCHECKER_MANIFEST_MISSED_ATTRIBUTE="The node <%1$s> doesn't contain required '%2$s' attribute"
 COM_JEDCHECKER_MANIFEST_UNKNOWN_ATTRIBUTE_VALUE="The node <%1$s> has attribute '%2$s' with unknown value "%3$s""
+COM_JEDCHECKER_MANIFEST_MULTIPLE_ATTRIBUTES="The <files> node contains multiple elements with '%s' attribute"
+COM_JEDCHECKER_MANIFEST_MODULE_ELEMENT_MISMATCH="The value of <element> node doesn't match value of 'module' attribute in the <files> elements"
+COM_JEDCHECKER_MANIFEST_MISSED_ELEMENT_ATTRIBUTE="The <files> node doesn't contain element with '%s' attribute"
 COM_JEDCHECKER_XML_FILES="XML Files references"
 COM_JEDCHECKER_XML_FILES_DESC="Check for incorrect files and folders references in the XML manifest"
 COM_JEDCHECKER_XML_FILES_FILE_NOT_FOUND="File not found: %s"
 COM_JEDCHECKER_XML_FILES_FOLDER_NOT_FOUND="Folder not found: %s"
+COM_JEDCHECKER_XML_FILES_EMPTY_LIST="The node %s is empty"
 COM_JEDCHECKER_LANG="Language files"
 COM_JEDCHECKER_LANG_DESC="Validates language files"
 COM_JEDCHECKER_LANG_INCORRECT_EOL="Incorrect end-of-line character found. Convert file to Unix EOL (\n) format."

administrator/components/com_jedchecker/libraries/rules/xmlfiles.php

@@ -105,7 +105,7 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		$xml = simplexml_load_file($file);
 
 		// Failed to parse the xml file.
-		// Assume that this is not a extension manifest
+		// Assume that this is not an extension manifest
 		if (!$xml)
 		{
 			return false;
@@ -125,6 +125,7 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->files))
 		{
 			$node = $xml->files;
+			$this->checkNotEmpty($node);
 
 			// Get path to site files from "folder" attribute
 			$sitedir = $this->getSourceFolder($node);
@@ -138,6 +139,8 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->media))
 		{
 			$node = $xml->media;
+			$this->checkNotEmpty($node);
+
 			$dir = $this->getSourceFolder($node);
 
 			$this->checkFiles($node->filename, $dir);
@@ -149,6 +152,8 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->fonts))
 		{
 			$node = $xml->fonts;
+			$this->checkNotEmpty($node);
+
 			$dir = $this->getSourceFolder($node);
 
 			$this->checkFiles($node->filename, $dir);
@@ -160,6 +165,8 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->languages))
 		{
 			$node = $xml->languages;
+			$this->checkNotEmpty($node);
+
 			$dir = $this->getSourceFolder($node);
 
 			$this->checkFiles($node->language, $dir);
@@ -171,6 +178,7 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->administration->files))
 		{
 			$node = $xml->administration->files;
+			$this->checkNotEmpty($node);
 
 			// Get path to admin files from "folder" attribute
 			$admindir = $this->getSourceFolder($node);
@@ -184,6 +192,8 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->administration->media))
 		{
 			$node = $xml->administration->media;
+			$this->checkNotEmpty($node);
+
 			$dir = $this->getSourceFolder($node);
 
 			$this->checkFiles($node->filename, $dir);
@@ -195,6 +205,8 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->administration->languages))
 		{
 			$node = $xml->administration->languages;
+			$this->checkNotEmpty($node);
+
 			$dir = $this->getSourceFolder($node);
 
 			$this->checkFiles($node->language, $dir);
@@ -205,6 +217,8 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->fileset->files))
 		{
 			$node = $xml->fileset->files;
+			$this->checkNotEmpty($node);
+
 			$dir = $this->getSourceFolder($node);
 
 			$this->checkFiles($node->filename, $dir);
@@ -216,6 +230,8 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		if (isset($xml->api->files))
 		{
 			$node = $xml->api->files;
+			$this->checkNotEmpty($node);
+
 			$dir = $this->getSourceFolder($node);
 
 			$this->checkFiles($node->filename, $dir);
@@ -347,6 +363,27 @@ class JedcheckerRulesXMLFiles extends JEDcheckerRule
 		return '';
 	}
 
+	/**
+	 * Check list of files/folders is not empty
+	 *
+	 * @param   SimpleXMLElement  $node  Node to check
+	 *
+	 * @return  void
+	 */
+	protected function checkNotEmpty($node)
+	{
+		if (count($node->children()) === 0) {
+			$path = array();
+
+			foreach ($node->xpath("ancestor-or-self::*") as $p)
+			{
+				$path[] = $p->getName();
+			}
+
+			$this->warnings[] = JText::sprintf('COM_JEDCHECKER_XML_FILES_EMPTY_LIST', implode('/', $path));
+		}
+	}
+
 	/**
 	 * Check files exist
 	 *

administrator/components/com_jedchecker/libraries/rules/xmlmanifest.php

@@ -132,7 +132,7 @@ class JedcheckerRulesXMLManifest extends JEDcheckerRule
 		$xml = simplexml_load_file($file);
 
 		// Failed to parse the xml file.
-		// Assume that this is not a extension manifest
+		// Assume that this is not an extension manifest
 		if (!$xml)
 		{
 			return false;
@@ -178,12 +178,67 @@ class JedcheckerRulesXMLManifest extends JEDcheckerRule
 				{
 					$this->report->addError($file, JText::sprintf('COM_JEDCHECKER_MANIFEST_UNKNOWN_ATTRIBUTE_VALUE', $xml->getName(), 'client', $client));
 				}
+
+				if ($type === 'module')
+				{
+					// Either <element> or "module" attribute (once only) should be present
+					$elements = $this->collectElements($xml->files, $type);
+
+					if (count($elements) >= 2)
+					{
+						$this->report->addWarning($file, JText::sprintf('COM_JEDCHECKER_MANIFEST_MULTIPLE_ATTRIBUTES', 'module'));
+					}
+
+					if (isset($xml->element))
+					{
+						$element = (string) $xml->element;
+
+						if (count($elements) && $elements[0] !== $element)
+						{
+							$this->report->addWarning($file, JText::_('COM_JEDCHECKER_MANIFEST_MODULE_ELEMENT_MISMATCH'));
+						}
+					}
+					else
+					{
+						if (count($elements) === 0)
+						{
+							$this->report->addError($file, JText::sprintf('COM_JEDCHECKER_MANIFEST_MISSED_ELEMENT_ATTRIBUTE', 'module'));
+						}
+					}
+				}
+
+				break;
+
+			case 'plugin':
+				// "plugin" attribute (once only) should be present
+				$elements = $this->collectElements($xml->files, $type);
+
+				if (count($elements) >= 2)
+				{
+					$this->report->addWarning($file, JText::sprintf('COM_JEDCHECKER_MANIFEST_MULTIPLE_ATTRIBUTES', 'plugin'));
+				}
+
+				if (count($elements) === 0)
+				{
+					$this->report->addError($file, JText::sprintf('COM_JEDCHECKER_MANIFEST_MISSED_ELEMENT_ATTRIBUTE', 'plugin'));
+				}
+
 				break;
 
 			case 'package':
 				// Check type-specific attributes
-				foreach ($xml->files->file as $item)
+				foreach ($xml->files->children() as $item)
 				{
+					if (!isset($item['type']))
+					{
+						$this->report->addError($file, JText::sprintf('COM_JEDCHECKER_MANIFEST_MISSED_ATTRIBUTE', $item->getName(), 'type'));
+					}
+
+					if (!isset($item['id']))
+					{
+						$this->report->addError($file, JText::sprintf('COM_JEDCHECKER_MANIFEST_MISSED_ATTRIBUTE', $item->getName(), 'id'));
+					}
+
 					switch ((string) $item['type'])
 					{
 						case 'plugin':
@@ -431,4 +486,29 @@ class JedcheckerRulesXMLManifest extends JEDcheckerRule
 			}
 		}
 	}
+
+	/**
+	 * Collect values of $type attribute from all children
+	 * @param   SimpleXMLElement  $node  XML node <files>
+	 * @param   string            $type  Extension's type (plugin or module)
+	 *
+	 * @return array List of found attributes
+	 */
+	protected function collectElements($node, $type)
+	{
+		$elements = array();
+
+		if (isset($node))
+		{
+			foreach ($node->children() as $child)
+			{
+				if (isset($child[$type]))
+				{
+					$elements[] = (string) $child[$type];
+				}
+			}
+		}
+
+		return $elements;
+	}
 }

administrator/components/com_jedchecker/libraries/rules/xmlmanifest/dtd_module.json

@@ -87,6 +87,9 @@
     "files:file": [
       "module"
     ],
+    "folder": [
+      "module"
+    ],
     "languages": [
       "folder"
     ],

administrator/components/com_jedchecker/libraries/rules/xmlmanifest/dtd_package.json

@@ -75,6 +75,12 @@
       "id",
       "type"
     ],
+    "folder": [
+      "client",
+      "group",
+      "id",
+      "type"
+    ],
     "languages": [
       "folder"
     ],

administrator/components/com_jedchecker/libraries/rules/xmlmanifest/dtd_plugin.json

@@ -88,6 +88,9 @@
     "files:file": [
       "plugin"
     ],
+    "folder": [
+      "plugin"
+    ],
     "languages": [
       "folder"
     ],