From 9fcd2f60050f9e3f1e46ee578e9ba77e800df4bb Mon Sep 17 00:00:00 2001 From: Ben Davies Date: Mon, 8 Apr 2013 17:34:47 +0100 Subject: [PATCH 1/9] [HttpFoundation] fixed the creation of sub-requests under some circumstances for IIS --- .../Component/HttpFoundation/Request.php | 7 +- .../HttpFoundation/Tests/RequestTest.php | 89 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index 6da6bc87ed..68af7f1c9b 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -1435,11 +1435,14 @@ class Request { $requestUri = ''; - if ($this->headers->has('X_ORIGINAL_URL') && false !== stripos(PHP_OS, 'WIN')) { + if ($this->headers->has('X_ORIGINAL_URL')) { // IIS with Microsoft Rewrite Module $requestUri = $this->headers->get('X_ORIGINAL_URL'); $this->headers->remove('X_ORIGINAL_URL'); - } elseif ($this->headers->has('X_REWRITE_URL') && false !== stripos(PHP_OS, 'WIN')) { + $this->server->remove('HTTP_X_ORIGINAL_URL'); + $this->server->remove('UNENCODED_URL'); + $this->server->remove('IIS_WasUrlRewritten'); + } elseif ($this->headers->has('X_REWRITE_URL')) { // IIS with ISAPI_Rewrite $requestUri = $this->headers->get('X_REWRITE_URL'); $this->headers->remove('X_REWRITE_URL'); diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php index 38ae748da6..2e6b9d787c 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php @@ -1231,6 +1231,95 @@ class RequestTest extends \PHPUnit_Framework_TestCase // reset Request::setTrustedProxies(array()); } + + /** + * @dataProvider iisRequestUriProvider + */ + public function testIISRequestUri($headers, $server, $expectedRequestUri) + { + $request = new Request(); + $request->headers->replace($headers); + $request->server->replace($server); + + $this->assertEquals($expectedRequestUri, $request->getRequestUri(), '->getRequestUri() is correct'); + + $subRequestUri = '/bar/foo'; + $subRequest = $request::create($subRequestUri, 'get', array(), array(), array(), $request->server->all()); + $this->assertEquals($subRequestUri, $subRequest->getRequestUri(), '->getRequestUri() is correct in sub request'); + } + + public function iisRequestUriProvider() + { + return array( + array( + array( + 'X_ORIGINAL_URL' => '/foo/bar', + ), + array(), + '/foo/bar' + ), + array( + array( + 'X_REWRITE_URL' => '/foo/bar', + ), + array(), + '/foo/bar' + ), + array( + array(), + array( + 'IIS_WasUrlRewritten' => '1', + 'UNENCODED_URL' => '/foo/bar' + ), + '/foo/bar' + ), + array( + array( + 'X_ORIGINAL_URL' => '/foo/bar', + ), + array( + 'HTTP_X_ORIGINAL_URL' => '/foo/bar' + ), + '/foo/bar' + ), + array( + array( + 'X_ORIGINAL_URL' => '/foo/bar', + ), + array( + 'IIS_WasUrlRewritten' => '1', + 'UNENCODED_URL' => '/foo/bar' + ), + '/foo/bar' + ), + array( + array( + 'X_ORIGINAL_URL' => '/foo/bar', + ), + array( + 'HTTP_X_ORIGINAL_URL' => '/foo/bar', + 'IIS_WasUrlRewritten' => '1', + 'UNENCODED_URL' => '/foo/bar' + ), + '/foo/bar' + ), + array( + array(), + array( + 'ORIG_PATH_INFO' => '/foo/bar', + ), + '/foo/bar' + ), + array( + array(), + array( + 'ORIG_PATH_INFO' => '/foo/bar', + 'QUERY_STRING' => 'foo=bar', + ), + '/foo/bar?foo=bar' + ) + ); + } } class RequestContentProxy extends Request From 8a434edd2cfa130b01ebad8af68db5c26321e502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B3th=20G=C3=A1bor?= Date: Wed, 17 Apr 2013 16:22:08 +0200 Subject: [PATCH 2/9] fix a DI circular reference recognition bug --- .../Compiler/CheckCircularReferencesPass.php | 10 ++++++---- .../CheckCircularReferencesPassTest.php | 19 ++++++++++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckCircularReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckCircularReferencesPass.php index 9cb3ff060d..e3db1aabbd 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckCircularReferencesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckCircularReferencesPass.php @@ -56,11 +56,13 @@ class CheckCircularReferencesPass implements CompilerPassInterface private function checkOutEdges(array $edges) { foreach ($edges as $edge) { - $node = $edge->getDestNode(); - $this->currentPath[] = $id = $node->getId(); + $node = $edge->getDestNode(); + $id = $node->getId(); + $searchKey = array_search($id, $this->currentPath); + $this->currentPath[] = $id; - if ($this->currentId === $id) { - throw new ServiceCircularReferenceException($this->currentId, $this->currentPath); + if (false !== $searchKey) { + throw new ServiceCircularReferenceException($id, array_slice($this->currentPath, $searchKey)); } $this->checkOutEdges($node->getOutEdges()); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckCircularReferencesPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckCircularReferencesPassTest.php index 25f816b834..085bc519b7 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckCircularReferencesPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckCircularReferencesPassTest.php @@ -24,7 +24,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; class CheckCircularReferencesPassTest extends \PHPUnit_Framework_TestCase { /** - * @expectedException \RuntimeException + * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException */ public function testProcess() { @@ -36,7 +36,7 @@ class CheckCircularReferencesPassTest extends \PHPUnit_Framework_TestCase } /** - * @expectedException \RuntimeException + * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException */ public function testProcessWithAliases() { @@ -49,7 +49,7 @@ class CheckCircularReferencesPassTest extends \PHPUnit_Framework_TestCase } /** - * @expectedException \RuntimeException + * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException */ public function testProcessDetectsIndirectCircularReference() { @@ -61,6 +61,19 @@ class CheckCircularReferencesPassTest extends \PHPUnit_Framework_TestCase $this->process($container); } + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException + */ + public function testDeepCircularReference() + { + $container = new ContainerBuilder(); + $container->register('a')->addArgument(new Reference('b')); + $container->register('b')->addArgument(new Reference('c')); + $container->register('c')->addArgument(new Reference('b')); + + $this->process($container); + } + public function testProcessIgnoresMethodCalls() { $container = new ContainerBuilder(); From 54bcf5c697f33b888c6b210f846a7d47c98a8ec2 Mon Sep 17 00:00:00 2001 From: Benjamin Paap Date: Wed, 17 Apr 2013 15:11:40 +0200 Subject: [PATCH 3/9] [Translator] added additional conversion for encodings other than utf-8 --- .../Translation/Loader/XliffFileLoader.php | 54 ++++++++++++++++++- .../Tests/Loader/XliffFileLoaderTest.php | 9 ++++ .../Translation/Tests/fixtures/encoding.xlf | 15 ++++++ 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf diff --git a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php index ba01fe3c65..e1b31a80f8 100644 --- a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php +++ b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php @@ -23,6 +23,37 @@ use Symfony\Component\Config\Resource\FileResource; */ class XliffFileLoader implements LoaderInterface { + + /** + * Encoding specified in xlf file + * + * @var string + */ + protected $encoding = null; + + /** + * Get $encoding + * + * @return string + */ + public function getEncoding() + { + return $this->encoding; + } + + /** + * Set $encoding + * + * @param string $encoding + * @return \Symfony\Component\Translation\Loader\XliffFileLoader + */ + public function setEncoding($encoding) + { + $this->encoding = strtoupper($encoding); + + return $this; + } + /** * {@inheritdoc} * @@ -37,6 +68,8 @@ class XliffFileLoader implements LoaderInterface $xml = $this->parseFile($resource); $xml->registerXPathNamespace('xliff', 'urn:oasis:names:tc:xliff:document:1.2'); + $encoding = $this->getEncoding(); + $catalogue = new MessageCatalogue($locale); foreach ($xml->xpath('//xliff:trans-unit') as $translation) { $attributes = $translation->attributes(); @@ -46,7 +79,21 @@ class XliffFileLoader implements LoaderInterface } $source = isset($attributes['resname']) && $attributes['resname'] ? $attributes['resname'] : $translation->source; - $catalogue->set((string) $source, (string) $translation->target, $domain); + $target = (string) $translation->target; + + // If the xlf file has another encoding specified try to convert it here because + // simple_xml will always return utf-8 encoded values + if ($encoding !== null) { + if (function_exists('mb_convert_encoding')) { + $target = mb_convert_encoding($target, $encoding, 'UTF-8'); + } elseif (function_exists('iconv')) { + $target = iconv('UTF-8', $encoding, $target); + } else { + throw new \RuntimeException('No suitable convert encoding function (use UTF-8 as your encoding or install the iconv or mbstring extension).'); + } + } + + $catalogue->set((string) $translation->source, $target, $domain); } $catalogue->addResource(new FileResource($resource)); @@ -76,6 +123,11 @@ class XliffFileLoader implements LoaderInterface throw new \RuntimeException(implode("\n", $this->getXmlErrors($internalErrors))); } + $encoding = strtoupper($dom->encoding); + if (!empty($encoding) && $encoding != 'UTF-8') { + $this->setEncoding($encoding); + } + libxml_disable_entity_loader($disableEntities); foreach ($dom->childNodes as $child) { diff --git a/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php b/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php index 5fad58617a..cf88d366e2 100644 --- a/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php +++ b/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php @@ -50,6 +50,15 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertFalse($catalogue->has('extra', 'domain1')); } + public function testEncoding() + { + $loader = $this->createLoader(); + $catalogue = $loader->load(__DIR__.'/../fixtures/encoding.xlf', 'en', 'domain1'); + + $this->assertEquals(utf8_decode('föö'), $catalogue->get('bar', 'domain1')); + $this->assertEquals(utf8_decode('bär'), $catalogue->get('foo', 'domain1')); + } + /** * @expectedException \RuntimeException */ diff --git a/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf b/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf new file mode 100644 index 0000000000..be656f9374 --- /dev/null +++ b/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf @@ -0,0 +1,15 @@ + + + + + + foo + bär + + + bar + föö + + + + From ad4624cd04957f43b7712f9f157223b69896707b Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 20 Apr 2013 09:44:07 +0200 Subject: [PATCH 4/9] [Translation] removed unneeded getter/setter --- .../Translation/Loader/XliffFileLoader.php | 48 +++---------------- .../Translation/Tests/fixtures/encoding.xlf | 6 +-- 2 files changed, 9 insertions(+), 45 deletions(-) diff --git a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php index e1b31a80f8..4ad5087099 100644 --- a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php +++ b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php @@ -23,37 +23,6 @@ use Symfony\Component\Config\Resource\FileResource; */ class XliffFileLoader implements LoaderInterface { - - /** - * Encoding specified in xlf file - * - * @var string - */ - protected $encoding = null; - - /** - * Get $encoding - * - * @return string - */ - public function getEncoding() - { - return $this->encoding; - } - - /** - * Set $encoding - * - * @param string $encoding - * @return \Symfony\Component\Translation\Loader\XliffFileLoader - */ - public function setEncoding($encoding) - { - $this->encoding = strtoupper($encoding); - - return $this; - } - /** * {@inheritdoc} * @@ -68,8 +37,6 @@ class XliffFileLoader implements LoaderInterface $xml = $this->parseFile($resource); $xml->registerXPathNamespace('xliff', 'urn:oasis:names:tc:xliff:document:1.2'); - $encoding = $this->getEncoding(); - $catalogue = new MessageCatalogue($locale); foreach ($xml->xpath('//xliff:trans-unit') as $translation) { $attributes = $translation->attributes(); @@ -81,19 +48,19 @@ class XliffFileLoader implements LoaderInterface $source = isset($attributes['resname']) && $attributes['resname'] ? $attributes['resname'] : $translation->source; $target = (string) $translation->target; - // If the xlf file has another encoding specified try to convert it here because + // If the xlf file has another encoding specified, try to convert it because // simple_xml will always return utf-8 encoded values - if ($encoding !== null) { + if ('UTF-8' !== $this->encoding && !empty($this->encoding)) { if (function_exists('mb_convert_encoding')) { - $target = mb_convert_encoding($target, $encoding, 'UTF-8'); + $target = mb_convert_encoding($target, $this->encoding, 'UTF-8'); } elseif (function_exists('iconv')) { - $target = iconv('UTF-8', $encoding, $target); + $target = iconv('UTF-8', $this->encoding, $target); } else { throw new \RuntimeException('No suitable convert encoding function (use UTF-8 as your encoding or install the iconv or mbstring extension).'); } } - $catalogue->set((string) $translation->source, $target, $domain); + $catalogue->set((string) $source, $target, $domain); } $catalogue->addResource(new FileResource($resource)); @@ -123,10 +90,7 @@ class XliffFileLoader implements LoaderInterface throw new \RuntimeException(implode("\n", $this->getXmlErrors($internalErrors))); } - $encoding = strtoupper($dom->encoding); - if (!empty($encoding) && $encoding != 'UTF-8') { - $this->setEncoding($encoding); - } + $this->encoding = strtoupper($dom->encoding); libxml_disable_entity_loader($disableEntities); diff --git a/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf b/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf index be656f9374..a6c33e084b 100644 --- a/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf +++ b/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf @@ -4,11 +4,11 @@ foo - bär + bär - bar - föö + bar + föö From 4c4a0c4ef128a2d8fe4881e85f8b9897aea1de6f Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 20 Apr 2013 10:25:59 +0200 Subject: [PATCH 5/9] [Translation] removed an uneeded class property --- .../Component/Translation/Loader/XliffFileLoader.php | 12 +++++------- .../Translation/Tests/Loader/XliffFileLoaderTest.php | 4 ++++ .../Translation/Tests/fixtures/encoding.xlf | 4 ++-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php index 4ad5087099..9c97233c77 100644 --- a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php +++ b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php @@ -34,7 +34,7 @@ class XliffFileLoader implements LoaderInterface throw new \InvalidArgumentException(sprintf('This is not a local file "%s".', $resource)); } - $xml = $this->parseFile($resource); + list($xml, $encoding) = $this->parseFile($resource); $xml->registerXPathNamespace('xliff', 'urn:oasis:names:tc:xliff:document:1.2'); $catalogue = new MessageCatalogue($locale); @@ -50,11 +50,11 @@ class XliffFileLoader implements LoaderInterface // If the xlf file has another encoding specified, try to convert it because // simple_xml will always return utf-8 encoded values - if ('UTF-8' !== $this->encoding && !empty($this->encoding)) { + if ('UTF-8' !== $encoding && !empty($encoding)) { if (function_exists('mb_convert_encoding')) { - $target = mb_convert_encoding($target, $this->encoding, 'UTF-8'); + $target = mb_convert_encoding($target, $encoding, 'UTF-8'); } elseif (function_exists('iconv')) { - $target = iconv('UTF-8', $this->encoding, $target); + $target = iconv('UTF-8', $encoding, $target); } else { throw new \RuntimeException('No suitable convert encoding function (use UTF-8 as your encoding or install the iconv or mbstring extension).'); } @@ -90,8 +90,6 @@ class XliffFileLoader implements LoaderInterface throw new \RuntimeException(implode("\n", $this->getXmlErrors($internalErrors))); } - $this->encoding = strtoupper($dom->encoding); - libxml_disable_entity_loader($disableEntities); foreach ($dom->childNodes as $child) { @@ -125,7 +123,7 @@ class XliffFileLoader implements LoaderInterface libxml_use_internal_errors($internalErrors); - return simplexml_import_dom($dom); + return array(simplexml_import_dom($dom), strtoupper($dom->encoding)); } /** diff --git a/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php b/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php index cf88d366e2..f84a62ee22 100644 --- a/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php +++ b/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php @@ -52,6 +52,10 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase public function testEncoding() { + if (!function_exists('iconv') && !function_exists('mb_convert_encoding')) { + $this->markTestSkipped('The iconv and mbstring extensions are not available.'); + } + $loader = $this->createLoader(); $catalogue = $loader->load(__DIR__.'/../fixtures/encoding.xlf', 'en', 'domain1'); diff --git a/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf b/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf index a6c33e084b..6be901bd75 100644 --- a/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf +++ b/src/Symfony/Component/Translation/Tests/fixtures/encoding.xlf @@ -4,11 +4,11 @@ foo - bär + bär bar - föö + föö From c2bc707a4de91f9ca609a170176eb30e3e45cb10 Mon Sep 17 00:00:00 2001 From: Andrew Coulton Date: Sun, 14 Apr 2013 00:07:46 +0100 Subject: [PATCH 6/9] fixed detection of secure cookies received over https BrowserKit's cookie handling only recognises a secure cookie if the cookie option is set and the cookie was set over an https request. The client was not passing the url into the cookiejar update code, causing Cookie::isSecure() to always return false for every cookie. Fixes symfony/symfony#7666 --- src/Symfony/Component/BrowserKit/Client.php | 2 +- src/Symfony/Component/BrowserKit/Tests/ClientTest.php | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/BrowserKit/Client.php b/src/Symfony/Component/BrowserKit/Client.php index c57b9e37a1..8664f31ab0 100644 --- a/src/Symfony/Component/BrowserKit/Client.php +++ b/src/Symfony/Component/BrowserKit/Client.php @@ -266,7 +266,7 @@ abstract class Client $response = $this->filterResponse($this->response); - $this->cookieJar->updateFromResponse($response); + $this->cookieJar->updateFromResponse($response, $uri); $this->redirect = $response->getHeader('Location'); diff --git a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php index e03b73cf0b..49b0c04129 100644 --- a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php +++ b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php @@ -205,6 +205,15 @@ class ClientTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array('foo' => 'bar'), $client->getCookieJar()->allValues('http://www.example.com/foo/foobar'), '->request() updates the CookieJar'); } + public function testRequestSecureCookies() + { + $client = new TestClient(); + $client->setNextResponse(new Response('foo', 200, array('Set-Cookie' => 'foo=bar; path=/; secure'))); + $client->request('GET', 'https://www.example.com/foo/foobar'); + + $this->assertTrue($client->getCookieJar()->get('foo','/','www.example.com')->isSecure()); + } + public function testClick() { if (!class_exists('Symfony\Component\DomCrawler\Crawler')) { From 495d0e366e0d9feea6c4873e567ea919093ff179 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 20 Apr 2013 19:50:53 +0200 Subject: [PATCH 7/9] [HttpFoundation] fixed empty domain= in Cookie::__toString() --- src/Symfony/Component/HttpFoundation/Cookie.php | 2 +- src/Symfony/Component/HttpFoundation/Tests/CookieTest.php | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Cookie.php b/src/Symfony/Component/HttpFoundation/Cookie.php index fe3a4cff42..43bf87dc23 100644 --- a/src/Symfony/Component/HttpFoundation/Cookie.php +++ b/src/Symfony/Component/HttpFoundation/Cookie.php @@ -95,7 +95,7 @@ class Cookie $str .= '; path='.$this->path; } - if (null !== $this->getDomain()) { + if ($this->getDomain()) { $str .= '; domain='.$this->getDomain(); } diff --git a/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php b/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php index 6a9948d62a..bfedd27fe1 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php @@ -117,11 +117,12 @@ class CookieTest extends \PHPUnit_Framework_TestCase public function testToString() { $cookie = new Cookie('foo', 'bar', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true); - $this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; domain=.myfoodomain.com; secure; httponly', $cookie->__toString(), '->__toString() returns string representation of the cookie'); $cookie = new Cookie('foo', null, 1, '/admin/', '.myfoodomain.com'); - $this->assertEquals('foo=deleted; expires=' . gmdate("D, d-M-Y H:i:s T", time()-31536001) . '; path=/admin/; domain=.myfoodomain.com; httponly', $cookie->__toString(), '->__toString() returns string representation of a cleared cookie if value is NULL'); + + $cookie = new Cookie('foo', 'bar', 0, '/', ''); + $this->assertEquals('foo=bar; httponly', $cookie->__toString()); } } From c884151e1656d382507c4a682b605d24d14b4266 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 20 Apr 2013 20:04:45 +0200 Subject: [PATCH 8/9] [BrowserKit] removed dead code --- src/Symfony/Component/BrowserKit/Cookie.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/BrowserKit/Cookie.php b/src/Symfony/Component/BrowserKit/Cookie.php index 6e1cdbf3c2..9feeb06dc9 100644 --- a/src/Symfony/Component/BrowserKit/Cookie.php +++ b/src/Symfony/Component/BrowserKit/Cookie.php @@ -145,10 +145,9 @@ class Cookie if ((false === $urlParts = parse_url($url)) || !isset($urlParts['host']) || !isset($urlParts['path'])) { throw new \InvalidArgumentException(sprintf('The URL "%s" is not valid.', $url)); } - $parts = array_merge($urlParts, $parts); - $values['domain'] = $parts['host']; - $values['path'] = substr($parts['path'], 0, strrpos($parts['path'], '/')); + $values['domain'] = $urlParts['host']; + $values['path'] = substr($urlParts['path'], 0, strrpos($urlParts['path'], '/')); } foreach ($parts as $part) { From 36d057b69efe67d84cbf73a3bafce283c1fc4c3b Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 20 Apr 2013 20:04:59 +0200 Subject: [PATCH 9/9] [HttpFoundation][BrowserKit] fixed path when converting a cookie to a string An empty path has a different meaning than a /; it means that the path is the one from the current URI. --- src/Symfony/Component/BrowserKit/Cookie.php | 2 +- .../Component/BrowserKit/Tests/ClientTest.php | 2 +- .../Component/BrowserKit/Tests/CookieTest.php | 24 +++++++++---------- .../Component/HttpFoundation/Cookie.php | 2 +- .../HttpFoundation/Tests/CookieTest.php | 4 ++-- .../Tests/ResponseHeaderBagTest.php | 6 ++--- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/Symfony/Component/BrowserKit/Cookie.php b/src/Symfony/Component/BrowserKit/Cookie.php index 9feeb06dc9..48a5cfd5bd 100644 --- a/src/Symfony/Component/BrowserKit/Cookie.php +++ b/src/Symfony/Component/BrowserKit/Cookie.php @@ -95,7 +95,7 @@ class Cookie $cookie .= '; domain='.$this->domain; } - if ('/' !== $this->path) { + if ($this->path) { $cookie .= '; path='.$this->path; } diff --git a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php index 49b0c04129..9235fd97b8 100644 --- a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php +++ b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php @@ -211,7 +211,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase $client->setNextResponse(new Response('foo', 200, array('Set-Cookie' => 'foo=bar; path=/; secure'))); $client->request('GET', 'https://www.example.com/foo/foobar'); - $this->assertTrue($client->getCookieJar()->get('foo','/','www.example.com')->isSecure()); + $this->assertTrue($client->getCookieJar()->get('foo', '/', 'www.example.com')->isSecure()); } public function testClick() diff --git a/src/Symfony/Component/BrowserKit/Tests/CookieTest.php b/src/Symfony/Component/BrowserKit/Tests/CookieTest.php index be53eea417..606b2e2e91 100644 --- a/src/Symfony/Component/BrowserKit/Tests/CookieTest.php +++ b/src/Symfony/Component/BrowserKit/Tests/CookieTest.php @@ -26,14 +26,14 @@ class CookieTest extends \PHPUnit_Framework_TestCase public function getTestsForToFromString() { return array( - array('foo=bar'), + array('foo=bar; path=/'), array('foo=bar; path=/foo'), - array('foo=bar; domain=google.com'), - array('foo=bar; domain=example.com; secure', 'https://example.com/'), - array('foo=bar; httponly'), + array('foo=bar; domain=google.com; path=/'), + array('foo=bar; domain=example.com; path=/; secure', 'https://example.com/'), + array('foo=bar; path=/; httponly'), array('foo=bar; domain=google.com; path=/foo; secure; httponly', 'https://google.com/'), - array('foo=bar=baz'), - array('foo=bar%3Dbaz'), + array('foo=bar=baz; path=/'), + array('foo=bar%3Dbaz; path=/'), ); } @@ -67,17 +67,17 @@ class CookieTest extends \PHPUnit_Framework_TestCase public function testFromStringWithCapitalization() { - $this->assertEquals('Foo=Bar', (string) Cookie::fromString('Foo=Bar')); - $this->assertEquals('foo=bar; expires=Fri, 31 Dec 2010 23:59:59 GMT', (string) Cookie::fromString('foo=bar; Expires=Fri, 31 Dec 2010 23:59:59 GMT')); - $this->assertEquals('foo=bar; domain=www.example.org; httponly', (string) Cookie::fromString('foo=bar; DOMAIN=www.example.org; HttpOnly')); + $this->assertEquals('Foo=Bar; path=/', (string) Cookie::fromString('Foo=Bar')); + $this->assertEquals('foo=bar; expires=Fri, 31 Dec 2010 23:59:59 GMT; path=/', (string) Cookie::fromString('foo=bar; Expires=Fri, 31 Dec 2010 23:59:59 GMT')); + $this->assertEquals('foo=bar; domain=www.example.org; path=/; httponly', (string) Cookie::fromString('foo=bar; DOMAIN=www.example.org; HttpOnly')); } public function testFromStringWithUrl() { - $this->assertEquals('foo=bar; domain=www.example.com', (string) Cookie::FromString('foo=bar', 'http://www.example.com/')); + $this->assertEquals('foo=bar; domain=www.example.com; path=/', (string) Cookie::FromString('foo=bar', 'http://www.example.com/')); $this->assertEquals('foo=bar; domain=www.example.com; path=/foo', (string) Cookie::FromString('foo=bar', 'http://www.example.com/foo/bar')); - $this->assertEquals('foo=bar; domain=www.example.com', (string) Cookie::FromString('foo=bar; path=/', 'http://www.example.com/foo/bar')); - $this->assertEquals('foo=bar; domain=www.myotherexample.com', (string) Cookie::FromString('foo=bar; domain=www.myotherexample.com', 'http://www.example.com/')); + $this->assertEquals('foo=bar; domain=www.example.com; path=/', (string) Cookie::FromString('foo=bar; path=/', 'http://www.example.com/foo/bar')); + $this->assertEquals('foo=bar; domain=www.myotherexample.com; path=/', (string) Cookie::FromString('foo=bar; domain=www.myotherexample.com', 'http://www.example.com/')); } public function testFromStringThrowsAnExceptionIfCookieIsNotValid() diff --git a/src/Symfony/Component/HttpFoundation/Cookie.php b/src/Symfony/Component/HttpFoundation/Cookie.php index 43bf87dc23..f8fbe67a6e 100644 --- a/src/Symfony/Component/HttpFoundation/Cookie.php +++ b/src/Symfony/Component/HttpFoundation/Cookie.php @@ -91,7 +91,7 @@ class Cookie } } - if ('/' !== $this->path) { + if ($this->path) { $str .= '; path='.$this->path; } diff --git a/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php b/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php index bfedd27fe1..3249ba3251 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php @@ -117,12 +117,12 @@ class CookieTest extends \PHPUnit_Framework_TestCase public function testToString() { $cookie = new Cookie('foo', 'bar', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true); - $this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; domain=.myfoodomain.com; secure; httponly', $cookie->__toString(), '->__toString() returns string representation of the cookie'); + $this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; path=/; domain=.myfoodomain.com; secure; httponly', $cookie->__toString(), '->__toString() returns string representation of the cookie'); $cookie = new Cookie('foo', null, 1, '/admin/', '.myfoodomain.com'); $this->assertEquals('foo=deleted; expires=' . gmdate("D, d-M-Y H:i:s T", time()-31536001) . '; path=/admin/; domain=.myfoodomain.com; httponly', $cookie->__toString(), '->__toString() returns string representation of a cleared cookie if value is NULL'); $cookie = new Cookie('foo', 'bar', 0, '/', ''); - $this->assertEquals('foo=bar; httponly', $cookie->__toString()); + $this->assertEquals('foo=bar; path=/; httponly', $cookie->__toString()); } } diff --git a/src/Symfony/Component/HttpFoundation/Tests/ResponseHeaderBagTest.php b/src/Symfony/Component/HttpFoundation/Tests/ResponseHeaderBagTest.php index fc3d908cb1..89b3e872df 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/ResponseHeaderBagTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/ResponseHeaderBagTest.php @@ -69,11 +69,11 @@ class ResponseHeaderBagTest extends \PHPUnit_Framework_TestCase $bag = new ResponseHeaderBag(array()); $bag->setCookie(new Cookie('foo', 'bar')); - $this->assertContains("Set-Cookie: foo=bar; httponly", explode("\r\n", $bag->__toString())); + $this->assertContains("Set-Cookie: foo=bar; path=/; httponly", explode("\r\n", $bag->__toString())); $bag->clearCookie('foo'); - $this->assertContains("Set-Cookie: foo=deleted; expires=".gmdate("D, d-M-Y H:i:s T", time() - 31536001)."; httponly", explode("\r\n", $bag->__toString())); + $this->assertContains("Set-Cookie: foo=deleted; expires=".gmdate("D, d-M-Y H:i:s T", time() - 31536001)."; path=/; httponly", explode("\r\n", $bag->__toString())); } public function testReplace() @@ -113,7 +113,7 @@ class ResponseHeaderBagTest extends \PHPUnit_Framework_TestCase $this->assertContains("Set-Cookie: foo=bar; path=/path/foo; domain=foo.bar; httponly", $headers); $this->assertContains("Set-Cookie: foo=bar; path=/path/foo; domain=foo.bar; httponly", $headers); $this->assertContains("Set-Cookie: foo=bar; path=/path/bar; domain=bar.foo; httponly", $headers); - $this->assertContains("Set-Cookie: foo=bar; httponly", $headers); + $this->assertContains("Set-Cookie: foo=bar; path=/; httponly", $headers); $cookies = $bag->getCookies(ResponseHeaderBag::COOKIES_ARRAY); $this->assertTrue(isset($cookies['foo.bar']['/path/foo']['foo']));