Add ValidatingArrayLoader and more validation for the validate command

src/Composer/Command/ValidateCommand.php

@@ -17,6 +17,8 @@ use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Output\OutputInterface;
 use Composer\Json\JsonFile;
 use Composer\Json\JsonValidationException;
+use Composer\Package\Loader\ValidatingArrayLoader;
+use Composer\Package\Loader\ArrayLoader;
 use Composer\Util\RemoteFilesystem;
 use Composer\Util\SpdxLicenseIdentifier;
 
@@ -109,7 +111,7 @@ EOT
             $warnings[] = 'No license specified, it is recommended to do so';
         }
 
-        if (preg_match('{[A-Z]}', $manifest['name'])) {
+        if (!empty($manifest['name']) && preg_match('{[A-Z]}', $manifest['name'])) {
             $suggestName = preg_replace('{(?:([a-z])([A-Z])|([A-Z])([A-Z][a-z]))}', '\\1\\3-\\2\\4', $manifest['name']);
             $suggestName = strtolower($suggestName);
 
@@ -120,6 +122,20 @@ EOT
             );
         }
 
+        // TODO validate package repositories' packages using the same technique as below
+        try {
+            $loader = new ValidatingArrayLoader(new ArrayLoader(), false);
+            if (!isset($manifest['version'])) {
+                $manifest['version'] = '1.0.0';
+            }
+            if (!isset($manifest['name'])) {
+                $manifest['version'] = 'dummy/dummy';
+            }
+            $loader->load($manifest);
+        } catch (\Exception $e) {
+            $errors = array_merge($errors, explode("\n", $e->getMessage()));
+        }
+
         // output errors/warnings
         if (!$errors && !$publishErrors && !$warnings) {
             $output->writeln('<info>' . $file . ' is valid</info>');

src/Composer/Package/Loader/ArrayLoader.php

@@ -19,7 +19,7 @@ use Composer\Package\Version\VersionParser;
  * @author Konstantin Kudryashiv <ever.zet@gmail.com>
  * @author Jordi Boggiano <j.boggiano@seld.be>
  */
-class ArrayLoader
+class ArrayLoader implements LoaderInterface
 {
     protected $versionParser;
 
@@ -31,7 +31,7 @@ class ArrayLoader
         $this->versionParser = $parser;
     }
 
-    public function load($config)
+    public function load(array $config)
     {
         if (!isset($config['name'])) {
             throw new \UnexpectedValueException('Unknown package has no name defined ('.json_encode($config).').');
@@ -82,8 +82,8 @@ class ArrayLoader
             $package->setHomepage($config['homepage']);
         }
 
-        if (!empty($config['keywords'])) {
+        if (!empty($config['keywords']) && is_array($config['keywords'])) {
-            $package->setKeywords(is_array($config['keywords']) ? $config['keywords'] : array($config['keywords']));
+            $package->setKeywords($config['keywords']);
         }
 
         if (!empty($config['license'])) {

src/Composer/Package/Loader/JsonLoader.php

@@ -17,8 +17,19 @@ use Composer\Json\JsonFile;
 /**
  * @author Konstantin Kudryashiv <ever.zet@gmail.com>
  */
-class JsonLoader extends ArrayLoader
+class JsonLoader
 {
+    private $loader;
+
+    public function __construct(LoaderInterface $loader)
+    {
+        $this->loader = $loader;
+    }
+
+    /**
+     * @param  string|JsonFile $json A filename, json string or JsonFile instance to load the package from
+     * @return \Composer\Package\PackageInterface
+     */
     public function load($json)
     {
         if ($json instanceof JsonFile) {
@@ -29,6 +40,6 @@ class JsonLoader extends ArrayLoader
             $config = JsonFile::parseJson($json);
         }
 
-        return parent::load($config);
+        return $this->loader->load($config);
     }
 }

src/Composer/Package/Loader/LoaderInterface.php

@@ -0,0 +1,29 @@
+<?php
+
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ *     Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Package\Loader;
+
+/**
+ * Defines a loader that takes an array to create package instances
+ *
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+interface LoaderInterface
+{
+    /**
+     * Converts a package from an array to a real instance
+     *
+     * @param array $package Package config
+     * @return \Composer\Package\PackageInterface
+     */
+    public function load(array $package);
+}

src/Composer/Package/Loader/RootPackageLoader.php

@@ -40,7 +40,7 @@ class RootPackageLoader extends ArrayLoader
         parent::__construct($parser);
     }
 
-    public function load($config)
+    public function load(array $config)
     {
         if (!isset($config['name'])) {
             $config['name'] = '__root__';

src/Composer/Package/Loader/ValidatingArrayLoader.php

@@ -0,0 +1,279 @@
+<?php
+
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ *     Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Package\Loader;
+
+use Composer\Package;
+use Composer\Package\Version\VersionParser;
+
+/**
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+class ValidatingArrayLoader implements LoaderInterface
+{
+    private $loader;
+    private $versionParser;
+    private $ignoreErrors;
+    private $errors = array();
+
+    public function __construct(LoaderInterface $loader, $ignoreErrors = true, VersionParser $parser = null)
+    {
+        $this->loader = $loader;
+        $this->ignoreErrors = $ignoreErrors;
+        if (!$parser) {
+            $parser = new VersionParser();
+        }
+        $this->versionParser = $parser;
+    }
+
+    public function load(array $config)
+    {
+        $this->config = $config;
+
+        $this->validateRegex('name', '[A-Za-z0-9][A-Za-z0-9_.-]*/[A-Za-z0-9][A-Za-z0-9_.-]*', true);
+
+        if (!empty($config['version'])) {
+            try {
+                $this->versionParser->normalize($config['version']);
+            } catch (\Exception $e) {
+                unset($this->config['version']);
+                $this->errors[] = 'version : invalid value ('.$config['version'].'): '.$e->getMessage();
+            }
+        }
+
+        $this->validateRegex('type', '[a-z0-9-]+');
+        $this->validateString('target-dir');
+        $this->validateArray('extra');
+        $this->validateFlatArray('bin');
+        $this->validateArray('scripts'); // TODO validate event names & listener syntax
+        $this->validateString('description');
+        $this->validateUrl('homepage');
+        $this->validateFlatArray('keywords', '[A-Za-z0-9 -]+');
+
+        if (isset($config['license'])) {
+            if (is_string($config['license'])) {
+                $this->validateRegex('license', '[A-Za-z0-9+. ()-]+');
+            } else {
+                $this->validateFlatArray('license', '[A-Za-z0-9+. ()-]+');
+            }
+        }
+
+        $this->validateString('time');
+        if (!empty($this->config['time'])) {
+            try {
+                $date = new \DateTime($config['time']);
+            } catch (\Exception $e) {
+                $this->errors[] = 'time : invalid value ('.$this->config['time'].'): '.$e->getMessage();
+                unset($this->config['time']);
+            }
+        }
+
+        $this->validateArray('authors');
+        if (!empty($this->config['authors'])) {
+            foreach ($this->config['authors'] as $key => $author) {
+                if (isset($author['homepage']) && !$this->filterUrl($author['homepage'])) {
+                    $this->errors[] = 'authors.'.$key.'.homepage : invalid value, must be a valid http/https URL';
+                    unset($this->config['authors'][$key]['homepage']);
+                }
+                if (isset($author['email']) && !filter_var($author['email'], FILTER_VALIDATE_EMAIL)) {
+                    $this->errors[] = 'authors.'.$key.'.email : invalid value, must be a valid email address';
+                    unset($this->config['authors'][$key]['email']);
+                }
+                if (isset($author['name']) && !is_string($author['name'])) {
+                    $this->errors[] = 'authors.'.$key.'.name : invalid value, must be a string';
+                    unset($this->config['authors'][$key]['name']);
+                }
+                if (isset($author['role']) && !is_string($author['role'])) {
+                    $this->errors[] = 'authors.'.$key.'.role : invalid value, must be a string';
+                    unset($this->config['authors'][$key]['role']);
+                }
+            }
+            if (empty($this->config['authors'])) {
+                unset($this->config['authors']);
+            }
+        }
+
+        $this->validateArray('support');
+        if (!empty($this->config['support'])) {
+            if (isset($this->config['support']['email']) && !filter_var($this->config['support']['email'], FILTER_VALIDATE_EMAIL)) {
+                $this->errors[] = 'support.email : invalid value, must be a valid email address';
+                unset($this->config['support']['email']);
+            }
+
+            if (isset($this->config['support']['irc'])
+                && (!filter_var($this->config['support']['irc'], FILTER_VALIDATE_URL) || !preg_match('{^irc://}iu', $this->config['support']['irc']))
+            ) {
+                $this->errors[] = 'support.irc : invalid value, must be ';
+                unset($this->config['support']['irc']);
+            }
+
+            foreach (array('issues', 'forum', 'wiki', 'source') as $key) {
+                if (isset($this->config['support'][$key]) && !$this->filterUrl($this->config['support'][$key])) {
+                    $this->errors[] = 'support.'.$key.' : invalid value, must be a valid http/https URL';
+                    unset($this->config['support'][$key]);
+                }
+            }
+            if (empty($this->config['support'])) {
+                unset($this->config['support']);
+            }
+        }
+
+        // TODO validate require/require-dev/replace/provide
+        // TODO validate suggest
+        // TODO validate autoload
+        // TODO validate minimum-stability
+
+        // TODO validate dist
+        // TODO validate source
+
+        // TODO validate repositories
+
+        $this->validateFlatArray('include-path');
+
+        // branch alias validation
+        if (isset($this->config['extra']['branch-alias'])) {
+            if (!is_array($this->config['extra']['branch-alias'])) {
+                $this->errors[] = 'extra.branch-alias : must be an array of versions => aliases';
+            } else {
+                foreach ($this->config['extra']['branch-alias'] as $sourceBranch => $targetBranch) {
+                    // ensure it is an alias to a -dev package
+                    if ('-dev' !== substr($targetBranch, -4)) {
+                        $this->errors[] = 'extra.branch-alias.'.$sourceBranch.' : the target branch ('.$targetBranch.') must end in -dev';
+                        unset($this->config['extra']['branch-alias'][$sourceBranch]);
+
+                        continue;
+                    }
+
+                    // normalize without -dev and ensure it's a numeric branch that is parseable
+                    $validatedTargetBranch = $this->versionParser->normalizeBranch(substr($targetBranch, 0, -4));
+                    if ('-dev' !== substr($validatedTargetBranch, -4)) {
+                        $this->errors[] = 'extra.branch-alias.'.$sourceBranch.' : the target branch ('.$targetBranch.') must be a parseable number like 2.0-dev';
+                        unset($this->config['extra']['branch-alias'][$sourceBranch]);
+                    }
+                }
+            }
+        }
+
+        if ($this->errors && !$this->ignoreErrors) {
+            throw new \Exception(implode("\n", $this->errors));
+        }
+
+        $package = $this->loader->load($this->config);
+        $this->errors = array();
+        unset($this->config);
+
+        return $package;
+    }
+
+    private function validateRegex($property, $regex, $mandatory = false)
+    {
+        if (!$this->validateString($property, $mandatory)) {
+            return false;
+        }
+
+        if (!preg_match('{^'.$regex.'$}u', $this->config[$property])) {
+            $this->errors[] = $property.' : invalid value, must match '.$regex;
+            unset($this->config[$property]);
+
+            return false;
+        }
+
+        return true;
+    }
+
+    private function validateString($property, $mandatory = false)
+    {
+        if (isset($this->config[$property]) && !is_string($this->config[$property])) {
+            $this->errors[] = $property.' : should be a string, '.gettype($this->config[$property]).' given';
+            unset($this->config[$property]);
+
+            return false;
+        }
+
+        if (!isset($this->config[$property]) || trim($this->config[$property]) === '') {
+            if ($mandatory) {
+                $this->errors[] = $property.' : must be present';
+            }
+            unset($this->config[$property]);
+
+            return false;
+        }
+
+        return true;
+    }
+
+    private function validateArray($property, $mandatory = false)
+    {
+        if (isset($this->config[$property]) && !is_array($this->config[$property])) {
+            $this->errors[] = $property.' : should be an array, '.gettype($this->config[$property]).' given';
+            unset($this->config[$property]);
+
+            return false;
+        }
+
+        if (!isset($this->config[$property]) || !count($this->config[$property])) {
+            if ($mandatory) {
+                $this->errors[] = $property.' : must be present and contain at least one element';
+            }
+            unset($this->config[$property]);
+
+            return false;
+        }
+
+        return true;
+    }
+
+    private function validateFlatArray($property, $regex = null, $mandatory = false)
+    {
+        if (!$this->validateArray($property, $mandatory)) {
+            return false;
+        }
+
+        $pass = true;
+        foreach ($this->config[$property] as $key => $value) {
+            if (!is_string($value) && !is_numeric($value)) {
+                $this->errors[] = $property.'.'.$key.' : must be a string or int, '.gettype($value).' given';
+                unset($this->config[$property][$key]);
+                $pass = false;
+
+                continue;
+            }
+
+            if ($regex && !preg_match('{^'.$regex.'$}u', $value)) {
+                $this->errors[] = $property.'.'.$key.' : invalid value, must match '.$regex;
+                unset($this->config[$property][$key]);
+                $pass = false;
+            }
+        }
+
+        return $pass;
+    }
+
+    private function validateUrl($property, $mandatory = false)
+    {
+        if (!$this->validateString($property, $mandatory)) {
+            return false;
+        }
+
+        if (!$this->filterUrl($this->config[$property])) {
+            $this->errors[] = $property.' : invalid value, must be a valid http/https URL';
+            unset($this->config[$property]);
+
+            return false;
+        }
+    }
+
+    private function filterUrl($value)
+    {
+        return filter_var($value, FILTER_VALIDATE_URL) && preg_match('{^https?://}iu', $value);
+    }
+}

tests/Composer/Test/Installer/InstallerInstallerTest.php

@@ -16,6 +16,7 @@ use Composer\Composer;
 use Composer\Config;
 use Composer\Installer\InstallerInstaller;
 use Composer\Package\Loader\JsonLoader;
+use Composer\Package\Loader\ArrayLoader;
 use Composer\Package\PackageInterface;
 
 class InstallerInstallerTest extends \PHPUnit_Framework_TestCase
@@ -28,7 +29,7 @@ class InstallerInstallerTest extends \PHPUnit_Framework_TestCase
 
     protected function setUp()
     {
-        $loader = new JsonLoader();
+        $loader = new JsonLoader(new ArrayLoader());
         $this->packages = array();
         for ($i = 1; $i <= 4; $i++) {
             $this->packages[] = $loader->load(__DIR__.'/Fixtures/installer-v'.$i.'/composer.json');

tests/Composer/Test/Package/Loader/ValidatingArrayLoaderTest.php

@@ -0,0 +1,222 @@
+<?php
+
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ *     Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Test\Package\Loader;
+
+use Composer\Package;
+use Composer\Package\Loader\ValidatingArrayLoader;
+
+class ValidatingArrayLoaderTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @dataProvider successProvider
+     */
+    public function testLoadSuccess($config)
+    {
+        $internalLoader = $this->getMock('Composer\Package\Loader\LoaderInterface');
+        $internalLoader
+            ->expects($this->once())
+            ->method('load')
+            ->with($config);
+
+        $loader = new ValidatingArrayLoader($internalLoader, false);
+        $loader->load($config);
+    }
+
+    public function successProvider()
+    {
+        return array(
+            array( // minimal
+                array(
+                    'name' => 'foo/bar',
+                ),
+            ),
+            array( // complete
+                array(
+                    'name' => 'foo/bar',
+                    'description' => 'Foo bar',
+                    'version' => '1.0.0',
+                    'type' => 'library',
+                    'keywords' => array('a', 'b'),
+                    'homepage' => 'https://foo.com',
+                    'time' => '2010-10-10T10:10:10+00:00',
+                    'license' => 'MIT',
+                    'authors' => array(
+                        array(
+                            'name' => 'Alice',
+                            'email' => 'alice@example.org',
+                            'role' => 'Lead',
+                            'homepage' => 'http://example.org',
+                        ),
+                        array(
+                            'name' => 'Bob',
+                            'homepage' => 'http://example.com',
+                        ),
+                    ),
+                    'support' => array(
+                        'email' => 'mail@example.org',
+                        'issues' => 'http://example.org/',
+                        'forum' => 'http://example.org/',
+                        'wiki' => 'http://example.org/',
+                        'source' => 'http://example.org/',
+                        'irc' => 'irc://example.org/example',
+                    ),
+                    'require' => array(
+                        'a/b' => '1.*',
+                        'example' => '>2.0-dev,<2.4-dev',
+                    ),
+                    'require-dev' => array(
+                        'a/b' => '1.*',
+                        'example' => '>2.0-dev,<2.4-dev',
+                    ),
+                    'conflict' => array(
+                        'a/b' => '1.*',
+                        'example' => '>2.0-dev,<2.4-dev',
+                    ),
+                    'replace' => array(
+                        'a/b' => '1.*',
+                        'example' => '>2.0-dev,<2.4-dev',
+                    ),
+                    'provide' => array(
+                        'a/b' => '1.*',
+                        'example' => '>2.0-dev,<2.4-dev',
+                    ),
+                    'suggest' => array(
+                        'foo/bar' => 'Foo bar is very useful',
+                    ),
+                    'autoload' => array(
+                        'psr-0' => array(
+                            'Foo\\Bar' => 'src/',
+                            '' => 'fallback/libs/',
+                        ),
+                        'classmap' => array(
+                            'dir/',
+                            'dir2/file.php',
+                        ),
+                        'files' => array(
+                            'functions.php',
+                        ),
+                    ),
+                    'include-path' => array(
+                        'lib/',
+                    ),
+                    'target-dir' => 'Foo/Bar',
+                    'minimum-stability' => 'dev',
+                    'repositories' => array(
+                        array(
+                            'type' => 'composer',
+                            'url' => 'http://packagist.org/',
+                        )
+                    ),
+                    'config' => array(
+                        'bin-dir' => 'bin',
+                        'vendor-dir' => 'vendor',
+                        'process-timeout' => 10000,
+                    ),
+                    'scripts' => array(
+                        'post-update-cmd' => 'Foo\\Bar\\Baz::doSomething',
+                        'post-install-cmd' => array(
+                            'Foo\\Bar\\Baz::doSomething',
+                        ),
+                    ),
+                    'extra' => array(
+                        'random' => array('stuff' => array('deeply' => 'nested')),
+                    ),
+                    'bin' => array(
+                        'bin/foo',
+                        'bin/bar',
+                    ),
+                ),
+            ),
+            array( // test as array
+                array(
+                    'name' => 'foo/bar',
+                    'license' => array('MIT', 'WTFPL'),
+                ),
+            ),
+        );
+    }
+
+    /**
+     * @dataProvider failureProvider
+     */
+    public function testLoadFailureThrowsException($config, $expectedErrors)
+    {
+        $internalLoader = $this->getMock('Composer\Package\Loader\LoaderInterface');
+        $loader = new ValidatingArrayLoader($internalLoader, false);
+        try {
+            $loader->load($config);
+            $this->fail('Expected exception to be thrown');
+        } catch (\Exception $e) {
+            $errors = explode("\n", $e->getMessage());
+            sort($expectedErrors);
+            sort($errors);
+            $this->assertEquals($expectedErrors, $errors);
+        }
+    }
+
+    /**
+     * @dataProvider failureProvider
+     */
+    public function testLoadSkipsInvalidDataWhenIgnoringErrors($config)
+    {
+        $internalLoader = $this->getMock('Composer\Package\Loader\LoaderInterface');
+        $internalLoader
+            ->expects($this->once())
+            ->method('load')
+            ->with(array('name' => 'a/b'));
+
+        $loader = new ValidatingArrayLoader($internalLoader, true);
+        $config['name'] = 'a/b';
+        $loader->load($config);
+    }
+
+    public function failureProvider()
+    {
+        return array(
+            array(
+                array(
+                    'name' => 'foo',
+                ),
+                array(
+                    'name : invalid value, must match [A-Za-z0-9][A-Za-z0-9_.-]*/[A-Za-z0-9][A-Za-z0-9_.-]*'
+                )
+            ),
+            array(
+                array(
+                    'name' => 'foo/bar',
+                    'homepage' => 'foo:bar',
+                ),
+                array(
+                    'homepage : invalid value, must be a valid http/https URL'
+                )
+            ),
+            array(
+                array(
+                    'name' => 'foo/bar',
+                    'support' => array(
+                        'source' => 'foo:bar',
+                        'forum' => 'foo:bar',
+                        'issues' => 'foo:bar',
+                        'wiki' => 'foo:bar',
+                    ),
+                ),
+                array(
+                    'support.source : invalid value, must be a valid http/https URL',
+                    'support.forum : invalid value, must be a valid http/https URL',
+                    'support.issues : invalid value, must be a valid http/https URL',
+                    'support.wiki : invalid value, must be a valid http/https URL',
+                )
+            ),
+        );
+    }
+}