Skip to content

Make array value cookies able to be sent. #11209

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

Merged
markstory merged 3 commits into from
Sep 19, 2017
Merged

Make array value cookies able to be sent. #11209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98f4673
Remove redundant doc blocks.
markstory Sep 19, 2017
56a56a2
Add Cookie::getStringValue()
markstory Sep 19, 2017
6cf84bc
Use new method in responses when getting cookie values.
markstory Sep 19, 2017
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
109 changes: 32 additions & 77 deletions src/Http/Cookie/Cookie.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,7 @@ public function toHeaderValue()
}

/**
* Create a cookie with an updated name
*
* @param string $name Name of the cookie
* @return static
* {@inheritDoc}
*/
public function withName($name)
{
Expand All @@ -200,11 +197,7 @@ public function withName($name)
}

/**
* Get the id for a cookie
*
* Cookies are unique across name, domain, path tuples.
*
* @return string
* {@inheritDoc}
*/
public function getId()
{
Expand All @@ -214,9 +207,7 @@ public function getId()
}

/**
* Gets the cookie name
*
* @return string
* {@inheritDoc}
*/
public function getName()
{
Expand Down Expand Up @@ -245,20 +236,27 @@ protected function validateName($name)
}

/**
* Gets the cookie value
*
* @return string|array
* {@inheritDoc}
*/
public function getValue()
{
return $this->value;
Copy link

@stefanozoffoli stefanozoffoli Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied your fix and it correctly save the cookie now.
However when I read the cookie value, it continues to be in json format.

Can be correct to modify here like this?

return $this->_expand($this->value);

But also when using ServerRequest->getCookie($cookie_name); it returns the cookie value in json format and it doesn't call Cookie->getValue().

Copy link

@stefanozoffoli stefanozoffoli Sep 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By setting

return $this->_expand($this->value);

There are warnings in the EncryptedCookieMiddleware so it's more complicated than this 😅

Also if you have the EncryptedCookieMiddleware active it already return the value in array format without the need to modify getValue()

Copy link
Member Author

@markstory markstory Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of the cookie is the JSON data though when you're reading from a request. The cookie middleware adds more behavior to cookies though. I wouldn't recommend people store JSON data in plaintext cookies as its full of 💣

}

/**
* Create a cookie with an updated value.
*
* @param string|array $value Value of the cookie to set
* @return static
* {@inheritDoc}
*/
public function getStringValue()
{
if ($this->isExpanded) {
return $this->_flatten($this->value);
}

return $this->value;
}

/**
* {@inheritDoc}
*/
public function withValue($value)
{
Expand All @@ -281,10 +279,7 @@ protected function _setValue($value)
}

/**
* Create a new cookie with an updated path
*
* @param string $path Sets the path
* @return static
* {@inheritDoc}
*/
public function withPath($path)
{
Expand All @@ -296,20 +291,15 @@ public function withPath($path)
}

/**
* Get the path attribute.
*
* @return string
* {@inheritDoc}
*/
public function getPath()
{
return $this->path;
}

/**
* Create a cookie with an updated domain
*
* @param string $domain Domain to set
* @return static
* {@inheritDoc}
*/
public function withDomain($domain)
{
Expand All @@ -321,9 +311,7 @@ public function withDomain($domain)
}

/**
* Get the domain attribute.
*
* @return string
* {@inheritDoc}
*/
public function getDomain()
{
Expand All @@ -348,20 +336,15 @@ protected function validateString($value)
}

/**
* Check if the cookie is secure
*
* @return bool
* {@inheritDoc}
*/
public function isSecure()
{
return $this->secure;
}

/**
* Create a cookie with Secure updated
*
* @param bool $secure Secure attribute value
* @return static
* {@inheritDoc}
*/
public function withSecure($secure)
{
Expand All @@ -373,10 +356,7 @@ public function withSecure($secure)
}

/**
* Create a cookie with HTTP Only updated
*
* @param bool $httpOnly HTTP Only
* @return static
* {@inheritDoc}
*/
public function withHttpOnly($httpOnly)
{
Expand Down Expand Up @@ -405,20 +385,15 @@ protected function validateBool($value)
}

/**
* Check if the cookie is HTTP only
*
* @return bool
* {@inheritDoc}
*/
public function isHttpOnly()
{
return $this->httpOnly;
}

/**
* Create a cookie with an updated expiration date
*
* @param \DateTime|\DateTimeImmutable $dateTime Date time object
* @return static
* {@inheritDoc}
*/
public function withExpiry($dateTime)
{
Expand All @@ -429,22 +404,15 @@ public function withExpiry($dateTime)
}

/**
* Get the current expiry time
*
* @return \DateTime|\DateTimeImmutable|null Timestamp of expiry or null
* {@inheritDoc}
*/
public function getExpiry()
{
return $this->expiresAt;
}

/**
* Get the timestamp from the expiration time
*
* Timestamps are strings as large timestamps can overflow MAX_INT
* in 32bit systems.
*
* @return string|null The expiry time as a string timestamp.
* {@inheritDoc}
*/
public function getExpiresTimestamp()
{
Expand All @@ -456,9 +424,7 @@ public function getExpiresTimestamp()
}

/**
* Builds the expiration value part of the header string
*
* @return string
* {@inheritDoc}
*/
public function getFormattedExpires()
{
Expand All @@ -470,12 +436,7 @@ public function getFormattedExpires()
}

/**
* Check if a cookie is expired when compared to $time
*
* Cookies without an expiration date always return false.
*
* @param \DateTime|\DateTimeImmutable $time The time to test against. Defaults to 'now' in UTC.
* @return bool
* {@inheritDoc}
*/
public function isExpired($time = null)
{
Expand All @@ -488,9 +449,7 @@ public function isExpired($time = null)
}

/**
* Create a new cookie that will virtually never expire.
*
* @return static
* {@inheritDoc}
*/
public function withNeverExpire()
{
Expand All @@ -501,11 +460,7 @@ public function withNeverExpire()
}

/**
* Create a new cookie that will expire/delete the cookie from the browser.
*
* This is done by setting the expiration time to 1 year ago
*
* @return static
* {@inheritDoc}
*/
public function withExpired()
{
Expand Down
9 changes: 9 additions & 0 deletions src/Http/Cookie/CookieInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ public function getName();
*/
public function getValue();

/**
* Gets the cookie value as a string.
*
* This will collapse any complex data in the cookie with json_encode()
*
* @return string
*/
public function getStringValue();
Copy link
Member

@jeremyharris jeremyharris Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to use __toString?

Copy link
Member Author

@markstory markstory Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cookies have more properties than just the value. Wouldn't the string value of a cookie include the Secure, HttpOnly, Path and Domain bits?

Copy link
Member

@jeremyharris jeremyharris Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you're absolutely right! Disregard :)


/**
* Create a cookie with an updated value.
*
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Response.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2146,7 +2146,7 @@ protected function convertCookieToArray(CookieInterface $cookie)
{
return [
'name' => $cookie->getName(),
'value' => $cookie->getValue(),
'value' => $cookie->getStringValue(),
'path' => $cookie->getPath(),
'domain' => $cookie->getDomain(),
'secure' => $cookie->isSecure(),
Expand Down
17 changes: 17 additions & 0 deletions tests/TestCase/Http/Cookie/CookieTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,23 @@ public function testWithValue()
$this->assertSame('new', $new->getValue());
}

/**
* Test getting the value from the cookie
*
* @return void
*/
public function testGetStringValue()
{
$cookie = new Cookie('cakephp', 'thing');
$this->assertSame('thing', $cookie->getStringValue());

$value = ['user_id' => 1, 'token' => 'abc123'];
$cookie = new Cookie('cakephp', $value);

$this->assertSame($value, $cookie->getValue());
$this->assertSame(json_encode($value), $cookie->getStringValue());
}

/**
* Test setting domain in cookies
*
Expand Down
27 changes: 27 additions & 0 deletions tests/TestCase/Http/ResponseTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1577,6 +1577,33 @@ public function testGetCookies()
$this->assertEquals($expected, $new->getCookies());
}

/**
* Test getCookies() and array data.
*
* @return void
*/
public function testGetCookiesArrayValue()
{
$response = new Response();
$cookie = (new Cookie('urmc'))
->withValue(['user_id' => 1, 'token' => 'abc123'])
->withHttpOnly(true);

$new = $response->withCookie($cookie);
$expected = [
'urmc' => [
'name' => 'urmc',
'value' => '{"user_id":1,"token":"abc123"}',
'expire' => null,
'path' => '',
'domain' => '',
'secure' => false,
'httpOnly' => true
],
];
$this->assertEquals($expected, $new->getCookies());
}

/**
* Test getCookieCollection() as array data
*
Expand Down