From a0f9f2c53739ef336064e9e3d5ecd0c0d2046418 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Tue, 5 Sep 2017 19:39:30 +0200 Subject: [PATCH 1/8] check permissions if dump target dir is missing `is_dir()` returns `false` if the parent directory misses the executable bit even when the directory itself is present. --- src/Symfony/Component/Filesystem/Filesystem.php | 10 ++++++++++ .../Filesystem/Tests/FilesystemTest.php | 16 ++++++++++++++++ .../Filesystem/Tests/FilesystemTestCase.php | 1 + 3 files changed, 27 insertions(+) diff --git a/src/Symfony/Component/Filesystem/Filesystem.php b/src/Symfony/Component/Filesystem/Filesystem.php index bc2e3dcc2d..bbef48b466 100644 --- a/src/Symfony/Component/Filesystem/Filesystem.php +++ b/src/Symfony/Component/Filesystem/Filesystem.php @@ -532,6 +532,16 @@ class Filesystem $dir = dirname($filename); if (!is_dir($dir)) { + $oldCwd = getcwd(); + + if (!@chdir(dirname($dir))) { + // When the parent directory misses the executable permission bit, we are unable to enter it and thus + // cannot check if the target directory exists. + throw new IOException(sprintf('Unable to detect if the target directory "%s" exists.', $dir)); + } + + chdir($oldCwd); + $this->mkdir($dir); } diff --git a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php index b7bdfac415..44eeceabd9 100644 --- a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php +++ b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php @@ -1102,6 +1102,22 @@ class FilesystemTest extends FilesystemTestCase $this->assertFilePermissions(745, $filename); } + /** + * @expectedException \Symfony\Component\Filesystem\Exception\IOException + * @expectedExceptionMessageRegExp /^Unable to detect if the target directory ".*" exists\.$/ + */ + public function testDumpFailsWithExceptionIfExecutablePermissionsForTheParentDirectoryAreMissing() + { + $this->markAsSkippedIfChmodIsMissing(); + + $target = $this->workspace.DIRECTORY_SEPARATOR.'foo'; + $file = $target.DIRECTORY_SEPARATOR.'foobar'; + mkdir($target); + chmod($this->workspace, 0666); + + $this->filesystem->dumpFile($file, 'baz'); + } + public function testCopyShouldKeepExecutionPermission() { $this->markAsSkippedIfChmodIsMissing(); diff --git a/src/Symfony/Component/Filesystem/Tests/FilesystemTestCase.php b/src/Symfony/Component/Filesystem/Tests/FilesystemTestCase.php index 5586a00547..47598b2942 100644 --- a/src/Symfony/Component/Filesystem/Tests/FilesystemTestCase.php +++ b/src/Symfony/Component/Filesystem/Tests/FilesystemTestCase.php @@ -61,6 +61,7 @@ class FilesystemTestCase extends TestCase $this->longPathNamesWindows = array(); } + chmod($this->workspace, 0777); $this->filesystem->remove($this->workspace); umask($this->umask); } From 73cdb68308670ea6e9caede76738aa31adbcf8b7 Mon Sep 17 00:00:00 2001 From: Yonel Ceruto Date: Wed, 6 Sep 2017 15:05:48 -0400 Subject: [PATCH 2/8] Get KERNEL_CLASS through $_ENV too --- src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php b/src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php index 26781cdf22..af8f4300f2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php +++ b/src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php @@ -106,8 +106,9 @@ abstract class KernelTestCase extends TestCase */ protected static function getKernelClass() { - if (isset($_SERVER['KERNEL_CLASS'])) { - if (!class_exists($class = $_SERVER['KERNEL_CLASS'])) { + if (isset($_SERVER['KERNEL_CLASS']) || isset($_ENV['KERNEL_CLASS'])) { + $class = isset($_SERVER['KERNEL_CLASS']) ? $_SERVER['KERNEL_CLASS'] : $_ENV['KERNEL_CLASS']; + if (!class_exists($class)) { throw new \RuntimeException(sprintf('Class "%s" doesn\'t exist or cannot be autoloaded. Check that the KERNEL_CLASS value in phpunit.xml matches the fully-qualified class name of your Kernel or override the %s::createKernel() method.', $class, static::class)); } From cf11fb9652f6c41c986d740a20429a1523cb65a5 Mon Sep 17 00:00:00 2001 From: Yonel Ceruto Date: Wed, 6 Sep 2017 18:24:46 -0400 Subject: [PATCH 3/8] Get KERNEL_DIR through $_ENV too for KernelTestCase --- src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php b/src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php index 351a8cc80a..5dadca83f2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php +++ b/src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php @@ -105,8 +105,8 @@ abstract class KernelTestCase extends TestCase */ protected static function getKernelClass() { - if (isset($_SERVER['KERNEL_DIR'])) { - $dir = $_SERVER['KERNEL_DIR']; + if (isset($_SERVER['KERNEL_DIR']) || isset($_ENV['KERNEL_DIR'])) { + $dir = isset($_SERVER['KERNEL_DIR']) ? $_SERVER['KERNEL_DIR'] : $_ENV['KERNEL_DIR']; if (!is_dir($dir)) { $phpUnitDir = static::getPhpUnitXmlDir(); From b9c6928d7e3070d6ae07c55505ac7f333f82d98c Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 7 Sep 2017 14:05:56 +0200 Subject: [PATCH 4/8] [HttpKernel] "controller.service_arguments" services should be public --- .../RegisterControllerArgumentLocatorsPass.php | 1 + ...RegisterControllerArgumentLocatorsPassTest.php | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php index 222185f527..9c00f99b7b 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php @@ -50,6 +50,7 @@ class RegisterControllerArgumentLocatorsPass implements CompilerPassInterface foreach ($container->findTaggedServiceIds($this->controllerTag, true) as $id => $tags) { $def = $container->getDefinition($id); + $def->setPublic(true); $class = $def->getClass(); $autowire = $def->isAutowired(); diff --git a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php index 0542698d69..83a07cb466 100644 --- a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php @@ -266,6 +266,21 @@ class RegisterControllerArgumentLocatorsPassTest extends TestCase $locator = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); $this->assertEmpty(array_keys($locator)); } + + public function testControllersAreMadePublic() + { + $container = new ContainerBuilder(); + $resolver = $container->register('argument_resolver.service')->addArgument(array()); + + $container->register('foo', ArgumentWithoutTypeController::class) + ->setPublic(false) + ->addTag('controller.service_arguments'); + + $pass = new RegisterControllerArgumentLocatorsPass(); + $pass->process($container); + + $this->assertTrue($container->getDefinition('foo')->isPublic()); + } } class RegisterTestController From a29e0694de139965a1e8e1f69dd84b46cca8ce89 Mon Sep 17 00:00:00 2001 From: Sergey Linnik Date: Tue, 5 Sep 2017 10:54:44 +0300 Subject: [PATCH 5/8] [Security] Fix exception when use_referer option is true and referer is not set or empty --- .../DefaultAuthenticationSuccessHandler.php | 5 ++--- .../DefaultAuthenticationSuccessHandlerTest.php | 10 ++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php b/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php index 7da6e35572..b7411e2c11 100644 --- a/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php +++ b/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php @@ -118,12 +118,11 @@ class DefaultAuthenticationSuccessHandler implements AuthenticationSuccessHandle return $targetUrl; } - if ($this->options['use_referer']) { - $targetUrl = $request->headers->get('Referer'); + if ($this->options['use_referer'] && $targetUrl = $request->headers->get('Referer')) { if (false !== $pos = strpos($targetUrl, '?')) { $targetUrl = substr($targetUrl, 0, $pos); } - if ($targetUrl !== $this->httpUtils->generateUri($request, $this->options['login_path'])) { + if ($targetUrl && $targetUrl !== $this->httpUtils->generateUri($request, $this->options['login_path'])) { return $targetUrl; } } diff --git a/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php b/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php index b42f840358..a7b8547b6b 100644 --- a/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php @@ -83,6 +83,16 @@ class DefaultAuthenticationSuccessHandlerTest extends TestCase array(), '/', ), + 'target path as referer when referer not set' => array( + Request::create('/'), + array('use_referer' => true), + '/', + ), + 'target path as referer when referer is ?' => array( + Request::create('/', 'GET', array(), array(), array(), array('HTTP_REFERER' => '?')), + array('use_referer' => true), + '/', + ), 'target path should be different than login URL' => array( Request::create('/', 'GET', array(), array(), array(), array('HTTP_REFERER' => 'http://localhost/login')), array('use_referer' => true, 'login_path' => '/login'), From 3eb79e5197c8b4184bf4a250fc04bf4175bf65c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Fri, 8 Sep 2017 15:50:13 +0200 Subject: [PATCH 6/8] [Fabbot] Do not run php-cs-fixer if there are no change in src/ --- .php_cs.dist | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.php_cs.dist b/.php_cs.dist index 140fd32650..6282431a41 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -1,5 +1,9 @@ setRules(array( '@Symfony' => true, From 1605ce1f074d96c3e62eeb297b00a66ce9a28c53 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 8 Sep 2017 16:43:09 +0200 Subject: [PATCH 7/8] [Filesystem] skip tests if not applicable --- src/Symfony/Component/Filesystem/Tests/FilesystemTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php index 44eeceabd9..67ccdd1eb0 100644 --- a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php +++ b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php @@ -1115,6 +1115,10 @@ class FilesystemTest extends FilesystemTestCase mkdir($target); chmod($this->workspace, 0666); + if (false !== @chdir($this->workspace)) { + $this->markTestSkipped('Test skipped as the used PHP version does not prevent entering directories without the required permissions.'); + } + $this->filesystem->dumpFile($file, 'baz'); } From 7793b797afbbe07d34b3ade2a4a368ddc5654af4 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 8 Sep 2017 18:12:06 -0700 Subject: [PATCH 8/8] Revert "bug #24105 [Filesystem] check permissions if dump target dir is missing (xabbuh)" This reverts commit d74144fc0b588bb8d7e0f73e6063e4022946f8e5, reversing changes made to 2b79f484058bd3265a39709e97a8787044b2ae7d. --- .../Component/Filesystem/Filesystem.php | 10 ---------- .../Filesystem/Tests/FilesystemTest.php | 20 ------------------- .../Filesystem/Tests/FilesystemTestCase.php | 1 - 3 files changed, 31 deletions(-) diff --git a/src/Symfony/Component/Filesystem/Filesystem.php b/src/Symfony/Component/Filesystem/Filesystem.php index bbef48b466..bc2e3dcc2d 100644 --- a/src/Symfony/Component/Filesystem/Filesystem.php +++ b/src/Symfony/Component/Filesystem/Filesystem.php @@ -532,16 +532,6 @@ class Filesystem $dir = dirname($filename); if (!is_dir($dir)) { - $oldCwd = getcwd(); - - if (!@chdir(dirname($dir))) { - // When the parent directory misses the executable permission bit, we are unable to enter it and thus - // cannot check if the target directory exists. - throw new IOException(sprintf('Unable to detect if the target directory "%s" exists.', $dir)); - } - - chdir($oldCwd); - $this->mkdir($dir); } diff --git a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php index 67ccdd1eb0..b7bdfac415 100644 --- a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php +++ b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php @@ -1102,26 +1102,6 @@ class FilesystemTest extends FilesystemTestCase $this->assertFilePermissions(745, $filename); } - /** - * @expectedException \Symfony\Component\Filesystem\Exception\IOException - * @expectedExceptionMessageRegExp /^Unable to detect if the target directory ".*" exists\.$/ - */ - public function testDumpFailsWithExceptionIfExecutablePermissionsForTheParentDirectoryAreMissing() - { - $this->markAsSkippedIfChmodIsMissing(); - - $target = $this->workspace.DIRECTORY_SEPARATOR.'foo'; - $file = $target.DIRECTORY_SEPARATOR.'foobar'; - mkdir($target); - chmod($this->workspace, 0666); - - if (false !== @chdir($this->workspace)) { - $this->markTestSkipped('Test skipped as the used PHP version does not prevent entering directories without the required permissions.'); - } - - $this->filesystem->dumpFile($file, 'baz'); - } - public function testCopyShouldKeepExecutionPermission() { $this->markAsSkippedIfChmodIsMissing(); diff --git a/src/Symfony/Component/Filesystem/Tests/FilesystemTestCase.php b/src/Symfony/Component/Filesystem/Tests/FilesystemTestCase.php index 47598b2942..5586a00547 100644 --- a/src/Symfony/Component/Filesystem/Tests/FilesystemTestCase.php +++ b/src/Symfony/Component/Filesystem/Tests/FilesystemTestCase.php @@ -61,7 +61,6 @@ class FilesystemTestCase extends TestCase $this->longPathNamesWindows = array(); } - chmod($this->workspace, 0777); $this->filesystem->remove($this->workspace); umask($this->umask); }