From 0d5f7e29050cf161dd620b4fa6d004d2cc7a81dc Mon Sep 17 00:00:00 2001 From: flip111 Date: Sat, 12 Dec 2015 17:45:35 +0100 Subject: [PATCH 1/5] Update FileSystem * Fixes edge case on windows where PHP does not generate a PHP Warning but instead returns a wrong result https://bugs.php.net/bug.php?id=71103 * Improved error reporting on `unlink` used in `remove()` --- .../Component/Filesystem/Filesystem.php | 27 +++++++++++++++-- .../Filesystem/Tests/FilesystemTest.php | 29 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Filesystem/Filesystem.php b/src/Symfony/Component/Filesystem/Filesystem.php index 1b6eaa68cd..8b41db93b8 100644 --- a/src/Symfony/Component/Filesystem/Filesystem.php +++ b/src/Symfony/Component/Filesystem/Filesystem.php @@ -100,6 +100,10 @@ class Filesystem public function exists($files) { foreach ($this->toIterator($files) as $file) { + if ('\\' === DIRECTORY_SEPARATOR AND strlen($file) > 258) { + throw new IOException(sprintf('Could not check if file exist because path length exceeds 258 characters for file "%s"', $file)); + } + if (!file_exists($file)) { return false; } @@ -139,7 +143,7 @@ class Filesystem $files = iterator_to_array($this->toIterator($files)); $files = array_reverse($files); foreach ($files as $file) { - if (!file_exists($file) && !is_link($file)) { + if (!$this->exists($file) && !is_link($file)) { continue; } @@ -157,7 +161,8 @@ class Filesystem } } else { if (true !== @unlink($file)) { - throw new IOException(sprintf('Failed to remove file %s', $file)); + $error = error_get_last(); + throw new IOException(sprintf('Failed to remove file "%s": %s.', $file, $error['message'])); } } } @@ -253,7 +258,7 @@ class Filesystem public function rename($origin, $target, $overwrite = false) { // we check that target does not exist - if (!$overwrite && is_readable($target)) { + if (!$overwrite && $this->isReadable($target)) { throw new IOException(sprintf('Cannot rename because the target "%s" already exist.', $target)); } @@ -262,6 +267,22 @@ class Filesystem } } + /** + * Tells whether a file exists and is readable. + * + * @param string $filename Path to the file. + * + * @throws IOException When windows path is longer than 258 characters + */ + private function isReadable($filename) + { + if ('\\' === DIRECTORY_SEPARATOR AND strlen($filename) > 258) { + throw new IOException(sprintf('Could not check if file is readable because path length exceeds 258 characters for file "%s"', $filename)); + } + + return is_readable($filename); + } + /** * Creates a symbolic link or copy a directory. * diff --git a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php index b57610cb81..d6c27d1f77 100644 --- a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php +++ b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php @@ -19,6 +19,7 @@ use Symfony\Component\Filesystem\Filesystem; class FilesystemTest extends \PHPUnit_Framework_TestCase { private $umask; + private $longPathNamesWindows = array(); /** * @var string @@ -56,6 +57,12 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase protected function tearDown() { + if (!empty($this->longPathNamesWindows)) { + foreach ($this->longPathNamesWindows as $path) { + exec('DEL '.$path); + } + } + $this->filesystem->remove($this->workspace); umask($this->umask); } @@ -354,6 +361,28 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase $this->assertTrue($this->filesystem->exists($basePath.'folder')); } + /** + * @expectedException \Symfony\Component\Filesystem\Exception\IOException + */ + public function testFilesExistsFails() + { + if ('\\' !== DIRECTORY_SEPARATOR) { + $this->markTestSkipped('Test covers edge case on Windows only.'); + } + + $basePath = $this->workspace.'\\directory\\'; + + $oldPath = getcwd(); + mkdir($basePath); + chdir($basePath); + $file = str_repeat('T', 259 - strlen($basePath)); + $path = $basePath.$file; + exec('TYPE NUL >>'.$file); // equivalent of touch, we can not use the php touch() here because it suffers from the same limitation + $this->longPathNamesWindows[] = $path; // save this so we can clean up later + chdir($oldPath); + $this->filesystem->exists($path); + } + public function testFilesExistsTraversableObjectOfFilesAndDirectories() { $basePath = $this->workspace.DIRECTORY_SEPARATOR; From 0fe3088dea6f0210efc13cc1f9a0581192f560a3 Mon Sep 17 00:00:00 2001 From: Toni Uebernickel Date: Wed, 27 Jan 2016 13:18:03 +0100 Subject: [PATCH 2/5] register commands from kernel when accessing list --- .../FrameworkBundle/Console/Application.php | 38 ++++++-- .../Tests/Console/ApplicationTest.php | 86 ++++++++++++++++++- src/Symfony/Component/Console/Application.php | 8 +- 3 files changed, 118 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Application.php b/src/Symfony/Bundle/FrameworkBundle/Console/Application.php index 6b02d3c5d2..3ecdb0837b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Application.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Application.php @@ -11,14 +11,14 @@ namespace Symfony\Bundle\FrameworkBundle\Console; -use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\Console\Application as BaseApplication; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; -use Symfony\Component\HttpKernel\KernelInterface; -use Symfony\Component\HttpKernel\Kernel; +use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\HttpKernel\Bundle\Bundle; +use Symfony\Component\HttpKernel\Kernel; +use Symfony\Component\HttpKernel\KernelInterface; /** * Application. @@ -69,12 +69,6 @@ class Application extends BaseApplication { $this->kernel->boot(); - if (!$this->commandsRegistered) { - $this->registerCommands(); - - $this->commandsRegistered = true; - } - $container = $this->kernel->getContainer(); foreach ($this->all() as $command) { @@ -96,8 +90,34 @@ class Application extends BaseApplication return parent::doRun($input, $output); } + /** + * {@inheritdoc} + */ + public function get($name) + { + $this->registerCommands(); + + return parent::get($name); + } + + /** + * {@inheritdoc} + */ + public function all($namespace = null) + { + $this->registerCommands(); + + return parent::all($namespace); + } + protected function registerCommands() { + if ($this->commandsRegistered) { + return; + } + + $this->commandsRegistered = true; + foreach ($this->kernel->getBundles() as $bundle) { if ($bundle instanceof Bundle) { $bundle->registerCommands($this); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php index 42fcb6f597..dbc8fb868b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php @@ -11,8 +11,9 @@ namespace Symfony\Bundle\FrameworkBundle\Tests\Console; -use Symfony\Bundle\FrameworkBundle\Tests\TestCase; use Symfony\Bundle\FrameworkBundle\Console\Application; +use Symfony\Bundle\FrameworkBundle\Tests\TestCase; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Console\Tester\ApplicationTester; @@ -38,6 +39,89 @@ class ApplicationTest extends TestCase $application = new Application($kernel); $application->doRun(new ArrayInput(array('list')), new NullOutput()); + + // Calling twice: registration should only be done once. + $application->doRun(new ArrayInput(array('list')), new NullOutput()); + } + + public function testBundleCommandsAreRetrievable() + { + $bundle = $this->getMock('Symfony\Component\HttpKernel\Bundle\Bundle'); + $bundle->expects($this->once())->method('registerCommands'); + + $kernel = $this->getMock('Symfony\Component\HttpKernel\KernelInterface'); + $kernel + ->expects($this->any()) + ->method('getBundles') + ->will($this->returnValue(array($bundle))) + ; + + $application = new Application($kernel); + $application->all(); + + // Calling twice: registration should only be done once. + $application->all(); + } + + public function testBundleSingleCommandIsRetrievable() + { + $bundle = $this->getMock('Symfony\Component\HttpKernel\Bundle\Bundle'); + $bundle->expects($this->once())->method('registerCommands'); + + $kernel = $this->getMock('Symfony\Component\HttpKernel\KernelInterface'); + $kernel + ->expects($this->any()) + ->method('getBundles') + ->will($this->returnValue(array($bundle))) + ; + + $application = new Application($kernel); + + $command = new Command('example'); + $application->add($command); + + $this->assertSame($command, $application->get('example')); + } + + public function testBundleCommandCanBeFound() + { + $bundle = $this->getMock('Symfony\Component\HttpKernel\Bundle\Bundle'); + $bundle->expects($this->once())->method('registerCommands'); + + $kernel = $this->getMock('Symfony\Component\HttpKernel\KernelInterface'); + $kernel + ->expects($this->any()) + ->method('getBundles') + ->will($this->returnValue(array($bundle))) + ; + + $application = new Application($kernel); + + $command = new Command('example'); + $application->add($command); + + $this->assertSame($command, $application->find('example')); + } + + public function testBundleCommandCanBeFoundByAlias() + { + $bundle = $this->getMock('Symfony\Component\HttpKernel\Bundle\Bundle'); + $bundle->expects($this->once())->method('registerCommands'); + + $kernel = $this->getMock('Symfony\Component\HttpKernel\KernelInterface'); + $kernel + ->expects($this->any()) + ->method('getBundles') + ->will($this->returnValue(array($bundle))) + ; + + $application = new Application($kernel); + + $command = new Command('example'); + $command->setAliases(array('alias')); + $application->add($command); + + $this->assertSame($command, $application->find('alias')); } public function testBundleCommandsHaveRightContainer() diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php index 78051c37ec..7a5b87ffbe 100644 --- a/src/Symfony/Component/Console/Application.php +++ b/src/Symfony/Component/Console/Application.php @@ -436,7 +436,7 @@ class Application public function getNamespaces() { $namespaces = array(); - foreach ($this->commands as $command) { + foreach ($this->all() as $command) { $namespaces = array_merge($namespaces, $this->extractAllNamespaces($command->getName())); foreach ($command->getAliases() as $alias) { @@ -530,7 +530,7 @@ class Application // name $commands = array(); - foreach ($this->commands as $command) { + foreach ($this->all() as $command) { $extractedNamespace = $this->extractNamespace($command->getName()); if ($extractedNamespace === $namespace || !empty($namespace) && 0 === strpos($extractedNamespace, $namespace) @@ -556,7 +556,7 @@ class Application // aliases $aliases = array(); - foreach ($this->commands as $command) { + foreach ($this->all() as $command) { foreach ($command->getAliases() as $alias) { $extractedNamespace = $this->extractNamespace($alias); if ($extractedNamespace === $namespace @@ -1028,7 +1028,7 @@ class Application return $item->getName(); }; - return $this->findAlternatives($name, $this->commands, $abbrevs, $callback); + return $this->findAlternatives($name, $this->all(), $abbrevs, $callback); } /** From 59944896f28c11bf9d94e03b6f04cea33426eda8 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 18 Feb 2016 14:55:37 +0100 Subject: [PATCH 3/5] documented the $url parameter better --- src/Symfony/Component/HttpFoundation/RedirectResponse.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpFoundation/RedirectResponse.php b/src/Symfony/Component/HttpFoundation/RedirectResponse.php index e7f47aa259..18d5794c0b 100644 --- a/src/Symfony/Component/HttpFoundation/RedirectResponse.php +++ b/src/Symfony/Component/HttpFoundation/RedirectResponse.php @@ -23,7 +23,8 @@ class RedirectResponse extends Response /** * Creates a redirect response so that it conforms to the rules defined for a redirect status code. * - * @param string $url The URL to redirect to + * @param string $url The URL to redirect to. The URL should be a full URL, with schema etc., + * but practically every browser redirects on paths only as well * @param int $status The status code (302 by default) * @param array $headers The headers (Location is always set to the given URL) * From 1f22290d8cc4fe73849c882cd742b4c958586480 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 18 Feb 2016 16:12:50 +0100 Subject: [PATCH 4/5] fixed CS --- src/Symfony/Component/Filesystem/Filesystem.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Filesystem/Filesystem.php b/src/Symfony/Component/Filesystem/Filesystem.php index 1b706ea068..b1b8052254 100644 --- a/src/Symfony/Component/Filesystem/Filesystem.php +++ b/src/Symfony/Component/Filesystem/Filesystem.php @@ -100,7 +100,7 @@ class Filesystem public function exists($files) { foreach ($this->toIterator($files) as $file) { - if ('\\' === DIRECTORY_SEPARATOR AND strlen($file) > 258) { + if ('\\' === DIRECTORY_SEPARATOR && strlen($file) > 258) { throw new IOException(sprintf('Could not check if file exist because path length exceeds 258 characters for file "%s"', $file)); } @@ -276,7 +276,7 @@ class Filesystem */ private function isReadable($filename) { - if ('\\' === DIRECTORY_SEPARATOR AND strlen($filename) > 258) { + if ('\\' === DIRECTORY_SEPARATOR && strlen($filename) > 258) { throw new IOException(sprintf('Could not check if file is readable because path length exceeds 258 characters for file "%s"', $filename)); } From a02967c9eee2e846d03c7a22bd5194706ab60646 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 18 Feb 2016 16:45:01 +0100 Subject: [PATCH 5/5] fixed CS --- src/Symfony/Bundle/FrameworkBundle/Console/Application.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Application.php b/src/Symfony/Bundle/FrameworkBundle/Console/Application.php index 3ecdb0837b..ddbbf8eb37 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Application.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Application.php @@ -11,14 +11,14 @@ namespace Symfony\Bundle\FrameworkBundle\Console; +use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\Console\Application as BaseApplication; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; -use Symfony\Component\DependencyInjection\ContainerAwareInterface; -use Symfony\Component\HttpKernel\Bundle\Bundle; -use Symfony\Component\HttpKernel\Kernel; use Symfony\Component\HttpKernel\KernelInterface; +use Symfony\Component\HttpKernel\Kernel; +use Symfony\Component\HttpKernel\Bundle\Bundle; /** * Application.