merged branch schmittjoh/httpUtilFixes (PR #1739)

Commits ------- eae6a77 fixed wrong case d0a175b fixes #1659 f300ede fixes several bugs a4f05ac added some tests Discussion ---------- Http util fixes Fixes several bugs in the http utils. Please don't add anymore features without sufficient tests. Especially for the Security\Http namespace, regressions are very likely otherwise. --------------------------------------------------------------------------- by fabpot at 2011/07/19 22:37:26 -0700 Tests do not pass for me: There were 2 errors: 1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #0 ('en') InvalidArgumentException: The current node list is empty. .../src/Symfony/Component/DomCrawler/Crawler.php:604 .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:16 2) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #1 ('de') InvalidArgumentException: The current node list is empty. .../src/Symfony/Component/DomCrawler/Crawler.php:604 .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:16 -- There were 4 failures: 1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResource with data set #0 ('en') Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -http://localhost/en/login +http://localhost/login .../src/Symfony/Bundle/Securitybundle/Tests/Functional/WebTestCase.php:22 .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:38 2) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResource with data set #1 ('de') Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -http://localhost/de/login +http://localhost/login .../src/Symfony/Bundle/Securitybundle/Tests/Functional/WebTestCase.php:22 .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:38 3) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResourceWithForward with data set #0 ('en') HTTP/1.0 302 Found Cache-Control: no-cache Content-Length: 299 Content-Type: text/html; charset=UTF-8 Date: Wed, 20 Jul 2011 05:36:27 GMT Location: http://localhost/login Set-Cookie: PHPSESSID=11c9c6a7e7620e13bddef223a5ba46d9; path=/; domain= <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <meta http-equiv="refresh" content="1;url=http://localhost/login" /> </head> <body> Redirecting to <a href="http://localhost/login">http://localhost/login</a>. </body> </html> Failed asserting that <integer:0> matches expected <integer:1>. .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:50 4) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResourceWithForward with data set #1 ('de') HTTP/1.0 302 Found Cache-Control: no-cache Content-Length: 299 Content-Type: text/html; charset=UTF-8 Date: Wed, 20 Jul 2011 05:36:28 GMT Location: http://localhost/login Set-Cookie: PHPSESSID=2bbe63786a088471ade3717917f4ba4f; path=/; domain= <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <meta http-equiv="refresh" content="1;url=http://localhost/login" /> </head> <body> Redirecting to <a href="http://localhost/login">http://localhost/login</a>. </body> </html> Failed asserting that <integer:0> matches expected <integer:1>. .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:50 --------------------------------------------------------------------------- by schmittjoh at 2011/07/19 23:47:29 -0700 I fixed a wrong case, but I couldn't reproduce the other errors (tested on Ubuntu). My guess is that the temporary directory on your machine couldn't be deleted for some reason, and the test runs with the configuration of some of the previous tests. --------------------------------------------------------------------------- by fabpot at 2011/07/20 00:28:41 -0700 That does not make any difference for me. For instance, in `LocalizedRoutesAsPathTest::testLoginLogoutProcedure()`, the first request to `'/'.$locale.'/login'` returns the following Response: <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <meta http-equiv="refresh" content="1;url=http://localhost/login" /> </head> <body> Redirecting to <a href="http://localhost/login">http://localhost/login</a>. </body> </html> --------------------------------------------------------------------------- by schmittjoh at 2011/07/20 00:31:34 -0700 That's weird, did you make sure that the temporary directory does not exist? ``rm -Rf /tmp/StandardFormLogin/`` On Wed, Jul 20, 2011 at 9:28 AM, fabpot < reply@reply.github.com>wrote: > That does not make any difference for me. For instance, in > `LocalizedRoutesAsPathTest::testLoginLogoutProcedure()`, the first request > to `'/'.$locale.'/login'` returns the following Response: > > <html> > <head> > <meta http-equiv="Content-Type" content="text/html; > charset=utf-8" /> > <meta http-equiv="refresh" content="1;url= > http://localhost/login" /> > </head> > <body> > Redirecting to <a href="http://localhost/login"> > http://localhost/login</a>. > </body> > </html> > > -- > Reply to this email directly or view it on GitHub: > https://github.com/symfony/symfony/pull/1739#issuecomment-1613504 > --------------------------------------------------------------------------- by fabpot at 2011/07/20 00:33:40 -0700 Yes, I've just checked and the directory does not exist. --------------------------------------------------------------------------- by schmittjoh at 2011/07/20 00:39:55 -0700 Sorry, I can't reproduce it on Ubuntu and unless someone wants to sponsor me a Mac, there is not much I can do.
author: Fabien Potencier <fabien.potencier@gmail.com> 2011-07-22 14:45:54 +0200
committer: Fabien Potencier <fabien.potencier@gmail.com> 2011-07-22 14:45:54 +0200
commit: d345e021e13d38d6cdf1340c59cd02ce6971216d (patch)
tree: 388a0ade88b8979c1961ee00b36209ff60eca608 /Http
parent: ba039320edf278a60887c3345a4737b09249cbd4 (diff)
parent: b005f5ce6e2b6aefbe0ae8569b2b8e63dc9a2fa9 (diff)
download: symfony-security-d345e021e13d38d6cdf1340c59cd02ce6971216d.zip
symfony-security-d345e021e13d38d6cdf1340c59cd02ce6971216d.tar.gz
symfony-security-d345e021e13d38d6cdf1340c59cd02ce6971216d.tar.bz2
2 files changed, 47 insertions, 15 deletions
diff --git a/Http/Firewall/ExceptionListener.php b/Http/Firewall/ExceptionListener.php
index c757390..46ffde8 100644
--- a/Http/Firewall/ExceptionListener.php
+++ b/Http/Firewall/ExceptionListener.php
@@ -76,6 +76,11 @@ class ExceptionListener
         $exception = $event->getException();
         $request = $event->getRequest();
 
+        // determine the actual cause for the exception
+        while (null !== $previous = $exception->getPrevious()) {
+            $exception = $previous;
+        }
+
         if ($exception instanceof AuthenticationException) {
             if (null !== $this->logger) {
                 $this->logger->info(sprintf('Authentication exception occurred; redirecting to authentication entry point (%s)', $exception->getMessage()));
diff --git a/Http/HttpUtils.php b/Http/HttpUtils.php
index 51168cc..3eccb41 100644
--- a/Http/HttpUtils.php
+++ b/Http/HttpUtils.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Http;
 
+use Symfony\Component\Security\Core\SecurityContextInterface;
+
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\Routing\RouterInterface;
@@ -45,22 +47,10 @@ class HttpUtils
      */
     public function createRedirectResponse(Request $request, $path, $status = 302)
     {
-        if (0 === strpos($path, '/')) {
+        if ('/' === $path[0]) {
             $path = $request->getUriForPath($path);
         } elseif (0 !== strpos($path, 'http')) {
-            // hack (don't have a better solution for now)
-            $context = $this->router->getContext();
-            try {
-                $parameters = $this->router->match($request->getPathInfo());
-            } catch (\Exception $e) {
-            }
-
-            if (isset($parameters['_locale'])) {
-                $context->setParameter('_locale', $parameters['_locale']);
-            } elseif ($session = $request->getSession()) {
-                $context->setParameter('_locale', $session->getLocale());
-            }
-
+            $this->resetLocale($request);
             $path = $this->generateUrl($path, true);
         }
 
@@ -78,10 +68,26 @@ class HttpUtils
     public function createRequest(Request $request, $path)
     {
         if ($path && '/' !== $path[0] && 0 !== strpos($path, 'http')) {
+            $this->resetLocale($request);
             $path = $this->generateUrl($path, true);
         }
 
-        return Request::create($path, 'get', array(), $request->cookies->all(), array(), $request->server->all());
+        $newRequest = Request::create($path, 'get', array(), $request->cookies->all(), array(), $request->server->all());
+        if ($session = $request->getSession()) {
+            $newRequest->setSession($session);
+        }
+
+        if ($request->attributes->has(SecurityContextInterface::AUTHENTICATION_ERROR)) {
+            $newRequest->attributes->set(SecurityContextInterface::AUTHENTICATION_ERROR, $request->attributes->get(SecurityContextInterface::AUTHENTICATION_ERROR));
+        }
+        if ($request->attributes->has(SecurityContextInterface::ACCESS_DENIED_ERROR)) {
+            $newRequest->attributes->set(SecurityContextInterface::ACCESS_DENIED_ERROR, $request->attributes->get(SecurityContextInterface::ACCESS_DENIED_ERROR));
+        }
+        if ($request->attributes->has(SecurityContextInterface::LAST_USERNAME)) {
+            $newRequest->attributes->set(SecurityContextInterface::LAST_USERNAME, $request->attributes->get(SecurityContextInterface::LAST_USERNAME));
+        }
+
+        return $newRequest;
     }
 
     /**
@@ -107,6 +113,27 @@ class HttpUtils
         return $path === $request->getPathInfo();
     }
 
+    // hack (don't have a better solution for now)
+    private function resetLocale(Request $request)
+    {
+        $context = $this->router->getContext();
+        if ($context->getParameter('_locale')) {
+            return;
+        }
+
+        try {
+            $parameters = $this->router->match($request->getPathInfo());
+
+            if (isset($parameters['_locale'])) {
+                $context->setParameter('_locale', $parameters['_locale']);
+            } elseif ($session = $request->getSession()) {
+                $context->setParameter('_locale', $session->getLocale());
+            }
+        } catch (\Exception $e) {
+            // let's hope user doesn't use the locale in the path
+        }
+    }
+
     private function generateUrl($route, $absolute = false)
     {
         if (null === $this->router) {
author	Fabien Potencier <fabien.potencier@gmail.com>	2011-07-22 14:45:54 +0200
committer	Fabien Potencier <fabien.potencier@gmail.com>	2011-07-22 14:45:54 +0200
commit	d345e021e13d38d6cdf1340c59cd02ce6971216d (patch)
tree	388a0ade88b8979c1961ee00b36209ff60eca608 /Http
parent	ba039320edf278a60887c3345a4737b09249cbd4 (diff)
parent	b005f5ce6e2b6aefbe0ae8569b2b8e63dc9a2fa9 (diff)
download	symfony-security-d345e021e13d38d6cdf1340c59cd02ce6971216d.zip symfony-security-d345e021e13d38d6cdf1340c59cd02ce6971216d.tar.gz symfony-security-d345e021e13d38d6cdf1340c59cd02ce6971216d.tar.bz2