Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix set cookie constructor validation #3133

Merged
merged 8 commits into from
May 14, 2023
Merged

Fix set cookie constructor validation #3133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
433788d
Fix set cookie constructor validation
GrahamCampbell May 13, 2023
90e8aa9
fixes
GrahamCampbell May 13, 2023
fa9676d
Update SetCookie.php
GrahamCampbell May 13, 2023
a625b9a
Update SetCookieTest.php
GrahamCampbell May 13, 2023
42efa2e
Update SetCookieTest.php
GrahamCampbell May 13, 2023
535cf21
Update SetCookieTest.php
GrahamCampbell May 13, 2023
b4b154c
Update SetCookieTest.php
GrahamCampbell May 13, 2023
e30e22e
Use data providers
GrahamCampbell May 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions src/Cookie/SetCookie.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,49 @@ public static function fromString(string $cookie): self
*/
public function __construct(array $data = [])
{
/** @var array|null $replaced will be null in case of replace error */
$replaced = \array_replace(self::$defaults, $data);
if ($replaced === null) {
throw new \InvalidArgumentException('Unable to replace the default values for the Cookie.');
$this->data = self::$defaults;

if (isset($data['Name'])) {
$this->setName($data['Name']);
}

if (isset($data['Value'])) {
$this->setValue($data['Value']);
}

if (isset($data['Domain'])) {
$this->setDomain($data['Domain']);
}

if (isset($data['Path'])) {
$this->setPath($data['Path']);
}

if (isset($data['Max-Age'])) {
$this->setMaxAge($data['Max-Age']);
}

if (isset($data['Expires'])) {
$this->setExpires($data['Expires']);
}

if (isset($data['Secure'])) {
$this->setSecure($data['Secure']);
}

if (isset($data['Discard'])) {
$this->setDiscard($data['Discard']);
}

if (isset($data['HttpOnly'])) {
$this->setHttpOnly($data['HttpOnly']);
}

// Set the remaining values that don't have extra validation logic
foreach (array_diff(array_keys($data), array_keys(self::$defaults)) as $key) {
$this->data[$key] = $data[$key];
}

$this->data = $replaced;
// Extract the Expires value and turn it into a UNIX timestamp if needed
if (!$this->getExpires() && $this->getMaxAge()) {
// Calculate the Expires date
Expand Down
141 changes: 99 additions & 42 deletions tests/Cookie/CookieJarTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,55 +128,112 @@ public function testRemovesSelectively()
self::assertCount(0, $this->jar);
}

public function testDoesNotAddIncompleteCookies()
public static function providesIncompleteCookies(): array
{
self::assertFalse($this->jar->setCookie(new SetCookie()));
self::assertFalse($this->jar->setCookie(new SetCookie([
'Name' => 'foo'
])));
self::assertFalse($this->jar->setCookie(new SetCookie([
'Name' => false
])));
self::assertFalse($this->jar->setCookie(new SetCookie([
'Name' => true
])));
self::assertFalse($this->jar->setCookie(new SetCookie([
'Name' => 'foo',
'Domain' => 'foo.com'
])));
return [
[
[],
],
[
[
'Name' => 'foo',
],
],
[
[
'Name' => false,
],
],
[
[
'Name' => true,
],
],
[
[
'Name' => 'foo',
'Domain' => 'foo.com',
],
],
];
}

public function testDoesNotAddEmptyCookies()
/**
* @dataProvider providesIncompleteCookies
*/
public function testDoesNotAddIncompleteCookies(array $cookie)
{
self::assertFalse($this->jar->setCookie(new SetCookie([
'Name' => '',
'Domain' => 'foo.com',
'Value' => 0
])));
self::assertFalse($this->jar->setCookie(new SetCookie($cookie)));
}

public function testDoesAddValidCookies()
public static function providesEmptyCookies(): array
{
return [
[
[
'Name' => '',
'Domain' => 'foo.com',
'Value' => 0,
],
],
[
[
'Name' => null,
'Domain' => 'foo.com',
'Value' => 0,
],
],
];
}

/**
* @dataProvider providesEmptyCookies
*/
public function testDoesNotAddEmptyCookies(array $cookie)
{
self::assertFalse($this->jar->setCookie(new SetCookie($cookie)));
}

public static function providesValidCookies(): array
{
return [
[
[
'Name' => '0',
'Domain' => 'foo.com',
'Value' => 0,
],
],
[
[
'Name' => 'foo',
'Domain' => 'foo.com',
'Value' => 0,
],
],
[
[
'Name' => 'foo',
'Domain' => 'foo.com',
'Value' => 0.0,
],
],
[
[
'Name' => 'foo',
'Domain' => 'foo.com',
'Value' => '0',
],
],
];
}

/**
* @dataProvider providesValidCookies
*/
public function testDoesAddValidCookies(array $cookie)
{
self::assertTrue($this->jar->setCookie(new SetCookie([
'Name' => '0',
'Domain' => 'foo.com',
'Value' => 0
])));
self::assertTrue($this->jar->setCookie(new SetCookie([
'Name' => 'foo',
'Domain' => 'foo.com',
'Value' => 0
])));
self::assertTrue($this->jar->setCookie(new SetCookie([
'Name' => 'foo',
'Domain' => 'foo.com',
'Value' => 0.0
])));
self::assertTrue($this->jar->setCookie(new SetCookie([
'Name' => 'foo',
'Domain' => 'foo.com',
'Value' => '0'
])));
self::assertTrue($this->jar->setCookie(new SetCookie($cookie)));
}

public function testOverwritesCookiesThatAreOlderOrDiscardable()
Expand Down
55 changes: 35 additions & 20 deletions tests/Cookie/SetCookieTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ public function cookieParserDataProvider()
'Discard' => null,
'Name' => 'ASIHTTPRequestTestCookie',
'Value' => 'This+is+the+value',
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
['', []],
['foo', []],
Expand All @@ -241,8 +241,8 @@ public function cookieParserDataProvider()
'Max-Age' => null,
'Path' => '/',
'Secure' => null,
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
// Test setting a blank value for a cookie
[[
Expand All @@ -256,8 +256,8 @@ public function cookieParserDataProvider()
'Max-Age' => null,
'Path' => '/',
'Secure' => null,
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
// Test setting a value and removing quotes
[[
Expand All @@ -271,8 +271,8 @@ public function cookieParserDataProvider()
'Max-Age' => null,
'Path' => '/',
'Secure' => null,
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
// Some of the following tests are based on https://github.com/zendframework/zf1/blob/master/tests/Zend/Http/CookieTest.php
[
Expand All @@ -286,8 +286,8 @@ public function cookieParserDataProvider()
'Max-Age' => null,
'Path' => '/',
'Secure' => null,
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
[
'expires=tomorrow; secure; path=/Space Out/; expires=Tue, 21-Nov-2006 08:33:44 GMT; domain=.example.com',
Expand All @@ -300,8 +300,8 @@ public function cookieParserDataProvider()
'Discard' => null,
'Secure' => true,
'Max-Age' => null,
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
[
'domain=unittests; expires=Tue, 21-Nov-2006 08:33:44 GMT; domain=example.com; path=/some value/',
Expand All @@ -314,8 +314,8 @@ public function cookieParserDataProvider()
'Secure' => false,
'Discard' => null,
'Max-Age' => null,
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
[
'path=indexAction; path=/; domain=.foo.com; expires=Tue, 21-Nov-2006 08:33:44 GMT',
Expand All @@ -328,8 +328,8 @@ public function cookieParserDataProvider()
'Secure' => false,
'Discard' => null,
'Max-Age' => null,
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
[
'secure=sha1; secure; SECURE; domain=some.really.deep.domain.com; version=1; Max-Age=86400',
Expand All @@ -343,8 +343,8 @@ public function cookieParserDataProvider()
'Expires' => \time() + 86400,
'Max-Age' => 86400,
'HttpOnly' => false,
'version' => '1'
]
'version' => '1',
],
],
[
'PHPSESSID=123456789+abcd%2Cef; secure; discard; domain=.localdomain; path=/foo/baz; expires=Tue, 21-Nov-2006 08:33:44 GMT;',
Expand All @@ -357,8 +357,23 @@ public function cookieParserDataProvider()
'Secure' => true,
'Discard' => true,
'Max-Age' => null,
'HttpOnly' => false
]
'HttpOnly' => false,
],
],
[
'fr=synced; Max-Age=604800 Expires=Mon, 12 Dec 2022 13:27:50 GMT; Domain=.example.com; Path=/; SameSite=None; Secure; HttpOnly',
[
'Name' => 'fr',
'Value' => 'synced',
'Domain' => '.example.com',
'Path' => '/',
'Expires' => \time() + 604800,
'Secure' => true,
'Discard' => false,
'Max-Age' => 604800,
'HttpOnly' => true,
'SameSite' => 'None',
],
],
];
}
Expand Down