Skip to content

Commit 82a95df

Browse files
committed
bug #25348 [HttpFoundation] Send cookies using header() to fix "SameSite" ones (nicolas-grekas, cvilleger)
This PR was merged into the 3.4 branch. Discussion ---------- [HttpFoundation] Send cookies using header() to fix "SameSite" ones | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25344 | License | MIT | Doc PR | - Commits ------- 73fec23 [HttpFoundation] Add functional tests for Response::sendHeaders() e350ea0 [HttpFoundation] Send cookies using header() to fix "SameSite" ones
2 parents 9a0422c + 73fec23 commit 82a95df

19 files changed

+231
-30
lines changed

src/Symfony/Component/HttpFoundation/Cookie.php

+5-3
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,12 @@ public function __toString()
145145
$str = ($this->isRaw() ? $this->getName() : urlencode($this->getName())).'=';
146146

147147
if ('' === (string) $this->getValue()) {
148-
$str .= 'deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001).'; max-age=-31536001';
148+
$str .= 'deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001).'; Max-Age=0';
149149
} else {
150150
$str .= $this->isRaw() ? $this->getValue() : rawurlencode($this->getValue());
151151

152152
if (0 !== $this->getExpiresTime()) {
153-
$str .= '; expires='.gmdate('D, d-M-Y H:i:s T', $this->getExpiresTime()).'; max-age='.$this->getMaxAge();
153+
$str .= '; expires='.gmdate('D, d-M-Y H:i:s T', $this->getExpiresTime()).'; Max-Age='.$this->getMaxAge();
154154
}
155155
}
156156

@@ -224,7 +224,9 @@ public function getExpiresTime()
224224
*/
225225
public function getMaxAge()
226226
{
227-
return 0 !== $this->expire ? $this->expire - time() : 0;
227+
$maxAge = $this->expire - time();
228+
229+
return 0 >= $maxAge ? 0 : $maxAge;
228230
}
229231

230232
/**

src/Symfony/Component/HttpFoundation/Response.php

+1-10
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ public function sendHeaders()
327327
}
328328

329329
// headers
330-
foreach ($this->headers->allPreserveCaseWithoutCookies() as $name => $values) {
330+
foreach ($this->headers->allPreserveCase() as $name => $values) {
331331
foreach ($values as $value) {
332332
header($name.': '.$value, false, $this->statusCode);
333333
}
@@ -336,15 +336,6 @@ public function sendHeaders()
336336
// status
337337
header(sprintf('HTTP/%s %s %s', $this->version, $this->statusCode, $this->statusText), true, $this->statusCode);
338338

339-
// cookies
340-
foreach ($this->headers->getCookies() as $cookie) {
341-
if ($cookie->isRaw()) {
342-
setrawcookie($cookie->getName(), $cookie->getValue(), $cookie->getExpiresTime(), $cookie->getPath(), $cookie->getDomain(), $cookie->isSecure(), $cookie->isHttpOnly());
343-
} else {
344-
setcookie($cookie->getName(), $cookie->getValue(), $cookie->getExpiresTime(), $cookie->getPath(), $cookie->getDomain(), $cookie->isSecure(), $cookie->isHttpOnly());
345-
}
346-
}
347-
348339
return $this;
349340
}
350341

src/Symfony/Component/HttpFoundation/Tests/CookieTest.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,13 @@ public function testCookieIsCleared()
162162
public function testToString()
163163
{
164164
$cookie = new Cookie('foo', 'bar', $expire = strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true);
165-
$this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; max-age='.($expire - time()).'; path=/; domain=.myfoodomain.com; secure; httponly', (string) $cookie, '->__toString() returns string representation of the cookie');
165+
$this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; Max-Age=0; path=/; domain=.myfoodomain.com; secure; httponly', (string) $cookie, '->__toString() returns string representation of the cookie');
166166

167167
$cookie = new Cookie('foo', 'bar with white spaces', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true);
168-
$this->assertEquals('foo=bar%20with%20white%20spaces; expires=Fri, 20-May-2011 15:25:52 GMT; max-age='.($expire - time()).'; path=/; domain=.myfoodomain.com; secure; httponly', (string) $cookie, '->__toString() encodes the value of the cookie according to RFC 3986 (white space = %20)');
168+
$this->assertEquals('foo=bar%20with%20white%20spaces; expires=Fri, 20-May-2011 15:25:52 GMT; Max-Age=0; path=/; domain=.myfoodomain.com; secure; httponly', (string) $cookie, '->__toString() encodes the value of the cookie according to RFC 3986 (white space = %20)');
169169

170170
$cookie = new Cookie('foo', null, 1, '/admin/', '.myfoodomain.com');
171-
$this->assertEquals('foo=deleted; expires='.gmdate('D, d-M-Y H:i:s T', $expire = time() - 31536001).'; max-age='.($expire - time()).'; path=/admin/; domain=.myfoodomain.com; httponly', (string) $cookie, '->__toString() returns string representation of a cleared cookie if value is NULL');
171+
$this->assertEquals('foo=deleted; expires='.gmdate('D, d-M-Y H:i:s T', $expire = time() - 31536001).'; Max-Age=0; path=/admin/; domain=.myfoodomain.com; httponly', (string) $cookie, '->__toString() returns string representation of a cleared cookie if value is NULL');
172172

173173
$cookie = new Cookie('foo', 'bar', 0, '/', '');
174174
$this->assertEquals('foo=bar; path=/; httponly', (string) $cookie);
@@ -194,7 +194,7 @@ public function testGetMaxAge()
194194
$this->assertEquals($expire - time(), $cookie->getMaxAge());
195195

196196
$cookie = new Cookie('foo', 'bar', $expire = time() - 100);
197-
$this->assertEquals($expire - time(), $cookie->getMaxAge());
197+
$this->assertEquals(0, $cookie->getMaxAge());
198198
}
199199

200200
public function testFromString()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
use Symfony\Component\HttpFoundation\Response;
4+
5+
$parent = __DIR__;
6+
while (!@file_exists($parent.'/vendor/autoload.php')) {
7+
if (!@file_exists($parent)) {
8+
// open_basedir restriction in effect
9+
break;
10+
}
11+
if ($parent === dirname($parent)) {
12+
echo "vendor/autoload.php not found\n";
13+
exit(1);
14+
}
15+
16+
$parent = dirname($parent);
17+
}
18+
19+
require $parent.'/vendor/autoload.php';
20+
21+
error_reporting(-1);
22+
ini_set('html_errors', 0);
23+
ini_set('display_errors', 1);
24+
25+
header_remove('X-Powered-By');
26+
header('Content-Type: text/plain; charset=utf-8');
27+
28+
register_shutdown_function(function () {
29+
echo "\n";
30+
session_write_close();
31+
print_r(headers_list());
32+
echo "shutdown\n";
33+
});
34+
ob_start();
35+
36+
$r = new Response();
37+
$r->headers->set('Date', 'Sat, 12 Nov 1955 20:04:00 GMT');
38+
39+
return $r;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
2+
Warning: Expiry date cannot have a year greater than 9999 in %scookie_max_age.php on line 10
3+
4+
Array
5+
(
6+
[0] => Content-Type: text/plain; charset=utf-8
7+
[1] => Cache-Control: no-cache, private
8+
[2] => Date: Sat, 12 Nov 1955 20:04:00 GMT
9+
[3] => Set-Cookie: foo=bar; expires=Sat, 01-Jan-10000 02:46:40 GMT; Max-Age=%d; path=/
10+
)
11+
shutdown
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
use Symfony\Component\HttpFoundation\Cookie;
4+
5+
$r = require __DIR__.'/common.inc';
6+
7+
$r->headers->setCookie(new Cookie('foo', 'bar', 253402310800, '', null, false, false));
8+
$r->sendHeaders();
9+
10+
setcookie('foo2', 'bar', 253402310800, '/');
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
2+
Array
3+
(
4+
[0] => Content-Type: text/plain; charset=utf-8
5+
[1] => Cache-Control: no-cache, private
6+
[2] => Date: Sat, 12 Nov 1955 20:04:00 GMT
7+
[3] => Set-Cookie: ?*():@&+$/%#[]=?*():@&+$/%#[]; path=/
8+
[4] => Set-Cookie: ?*():@&+$/%#[]=?*():@&+$/%#[]; path=/
9+
)
10+
shutdown
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
use Symfony\Component\HttpFoundation\Cookie;
4+
5+
$r = require __DIR__.'/common.inc';
6+
7+
$str = '?*():@&+$/%#[]';
8+
9+
$r->headers->setCookie(new Cookie($str, $str, 0, '/', null, false, false, true));
10+
$r->sendHeaders();
11+
12+
setrawcookie($str, $str, 0, '/', null, false, false);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
2+
Array
3+
(
4+
[0] => Content-Type: text/plain; charset=utf-8
5+
[1] => Cache-Control: no-cache, private
6+
[2] => Date: Sat, 12 Nov 1955 20:04:00 GMT
7+
[3] => Set-Cookie: CookieSamesiteLaxTest=LaxValue; path=/; httponly; samesite=lax
8+
)
9+
shutdown
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
use Symfony\Component\HttpFoundation\Cookie;
4+
5+
$r = require __DIR__.'/common.inc';
6+
7+
$r->headers->setCookie(new Cookie('CookieSamesiteLaxTest', 'LaxValue', 0, '/', null, false, true, false, Cookie::SAMESITE_LAX));
8+
$r->sendHeaders();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
2+
Array
3+
(
4+
[0] => Content-Type: text/plain; charset=utf-8
5+
[1] => Cache-Control: no-cache, private
6+
[2] => Date: Sat, 12 Nov 1955 20:04:00 GMT
7+
[3] => Set-Cookie: CookieSamesiteStrictTest=StrictValue; path=/; httponly; samesite=strict
8+
)
9+
shutdown
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
use Symfony\Component\HttpFoundation\Cookie;
4+
5+
$r = require __DIR__.'/common.inc';
6+
7+
$r->headers->setCookie(new Cookie('CookieSamesiteStrictTest', 'StrictValue', 0, '/', null, false, true, false, Cookie::SAMESITE_STRICT));
8+
$r->sendHeaders();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
2+
Array
3+
(
4+
[0] => Content-Type: text/plain; charset=utf-8
5+
[1] => Cache-Control: no-cache, private
6+
[2] => Date: Sat, 12 Nov 1955 20:04:00 GMT
7+
[3] => Set-Cookie: %3F%2A%28%29%3A%40%26%2B%24%2F%25%23%5B%5D=%3F%2A%28%29%3A%40%26%2B%24%2F%25%23%5B%5D; path=/
8+
[4] => Set-Cookie: ?*():@&+$/%#[]=%3F%2A%28%29%3A%40%26%2B%24%2F%25%23%5B%5D; path=/
9+
)
10+
shutdown
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
use Symfony\Component\HttpFoundation\Cookie;
4+
5+
$r = require __DIR__.'/common.inc';
6+
7+
$str = '?*():@&+$/%#[]';
8+
9+
$r->headers->setCookie(new Cookie($str, $str, 0, '', null, false, false));
10+
$r->sendHeaders();
11+
12+
setcookie($str, $str, 0, '/');
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
The cookie name "Hello + world" contains invalid characters.
2+
Array
3+
(
4+
[0] => Content-Type: text/plain; charset=utf-8
5+
)
6+
shutdown
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
use Symfony\Component\HttpFoundation\Cookie;
4+
5+
$r = require __DIR__.'/common.inc';
6+
7+
try {
8+
$r->headers->setCookie(new Cookie('Hello + world', 'hodor'));
9+
} catch (\InvalidArgumentException $e) {
10+
echo $e->getMessage();
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\Tests;
13+
14+
use PHPUnit\Framework\TestCase;
15+
16+
/**
17+
* @requires PHP 7.0
18+
*/
19+
class ResponseFunctionalTest extends TestCase
20+
{
21+
private static $server;
22+
23+
public static function setUpBeforeClass()
24+
{
25+
$spec = array(
26+
1 => array('file', '/dev/null', 'w'),
27+
2 => array('file', '/dev/null', 'w'),
28+
);
29+
if (!self::$server = @proc_open('exec php -S localhost:8054', $spec, $pipes, __DIR__.'/Fixtures/response-functional')) {
30+
self::markTestSkipped('PHP server unable to start.');
31+
}
32+
sleep(1);
33+
}
34+
35+
public static function tearDownAfterClass()
36+
{
37+
if (self::$server) {
38+
proc_terminate(self::$server);
39+
proc_close(self::$server);
40+
}
41+
}
42+
43+
/**
44+
* @dataProvider provideCookie
45+
*/
46+
public function testCookie($fixture)
47+
{
48+
$result = file_get_contents(sprintf('http://localhost:8054/%s.php', $fixture));
49+
$this->assertStringMatchesFormatFile(__DIR__.sprintf('/Fixtures/response-functional/%s.expected', $fixture), $result);
50+
}
51+
52+
public function provideCookie()
53+
{
54+
foreach (glob(__DIR__.'/Fixtures/response-functional/*.php') as $file) {
55+
yield array(pathinfo($file, PATHINFO_FILENAME));
56+
}
57+
}
58+
}

src/Symfony/Component/HttpFoundation/Tests/ResponseHeaderBagTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public function testToStringIncludesCookieHeaders()
116116

117117
$bag->clearCookie('foo');
118118

119-
$this->assertSetCookieHeader('foo=deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001).'; max-age=-31536001; path=/; httponly', $bag);
119+
$this->assertSetCookieHeader('foo=deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001).'; Max-Age=0; path=/; httponly', $bag);
120120
}
121121

122122
public function testClearCookieSecureNotHttpOnly()
@@ -125,7 +125,7 @@ public function testClearCookieSecureNotHttpOnly()
125125

126126
$bag->clearCookie('foo', '/', null, true, false);
127127

128-
$this->assertSetCookieHeader('foo=deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001).'; max-age=-31536001; path=/; secure', $bag);
128+
$this->assertSetCookieHeader('foo=deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001).'; Max-Age=0; path=/; secure', $bag);
129129
}
130130

131131
public function testReplace()

src/Symfony/Component/HttpKernel/Tests/ClientTest.php

+6-11
Original file line numberDiff line numberDiff line change
@@ -60,22 +60,17 @@ public function testFilterResponseConvertsCookies()
6060
$m = $r->getMethod('filterResponse');
6161
$m->setAccessible(true);
6262

63-
$expected = array(
64-
'foo=bar; expires=Sun, 15-Feb-2009 20:00:00 GMT; max-age='.(strtotime('Sun, 15-Feb-2009 20:00:00 GMT') - time()).'; path=/foo; domain=http://example.com; secure; httponly',
65-
'foo1=bar1; expires=Sun, 15-Feb-2009 20:00:00 GMT; max-age='.(strtotime('Sun, 15-Feb-2009 20:00:00 GMT') - time()).'; path=/foo; domain=http://example.com; secure; httponly',
66-
);
67-
6863
$response = new Response();
69-
$response->headers->setCookie(new Cookie('foo', 'bar', \DateTime::createFromFormat('j-M-Y H:i:s T', '15-Feb-2009 20:00:00 GMT')->format('U'), '/foo', 'http://example.com', true, true));
64+
$response->headers->setCookie($cookie1 = new Cookie('foo', 'bar', \DateTime::createFromFormat('j-M-Y H:i:s T', '15-Feb-2009 20:00:00 GMT')->format('U'), '/foo', 'http://example.com', true, true));
7065
$domResponse = $m->invoke($client, $response);
71-
$this->assertEquals($expected[0], $domResponse->getHeader('Set-Cookie'));
66+
$this->assertSame((string) $cookie1, $domResponse->getHeader('Set-Cookie'));
7267

7368
$response = new Response();
74-
$response->headers->setCookie(new Cookie('foo', 'bar', \DateTime::createFromFormat('j-M-Y H:i:s T', '15-Feb-2009 20:00:00 GMT')->format('U'), '/foo', 'http://example.com', true, true));
75-
$response->headers->setCookie(new Cookie('foo1', 'bar1', \DateTime::createFromFormat('j-M-Y H:i:s T', '15-Feb-2009 20:00:00 GMT')->format('U'), '/foo', 'http://example.com', true, true));
69+
$response->headers->setCookie($cookie1 = new Cookie('foo', 'bar', \DateTime::createFromFormat('j-M-Y H:i:s T', '15-Feb-2009 20:00:00 GMT')->format('U'), '/foo', 'http://example.com', true, true));
70+
$response->headers->setCookie($cookie2 = new Cookie('foo1', 'bar1', \DateTime::createFromFormat('j-M-Y H:i:s T', '15-Feb-2009 20:00:00 GMT')->format('U'), '/foo', 'http://example.com', true, true));
7671
$domResponse = $m->invoke($client, $response);
77-
$this->assertEquals($expected[0], $domResponse->getHeader('Set-Cookie'));
78-
$this->assertEquals($expected, $domResponse->getHeader('Set-Cookie', false));
72+
$this->assertSame((string) $cookie1, $domResponse->getHeader('Set-Cookie'));
73+
$this->assertSame(array((string) $cookie1, (string) $cookie2), $domResponse->getHeader('Set-Cookie', false));
7974
}
8075

8176
public function testFilterResponseSupportsStreamedResponses()

0 commit comments

Comments
 (0)