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.

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

Make array value cookies able to be sent. #11209

Merged
merged 3 commits into from Sep 19, 2017
Jump to file or symbol
Failed to load files and symbols.
+86 −78
Diff settings

Always

Just for now

View
@@ -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)
{
@@ -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()
{
@@ -214,9 +207,7 @@ public function getId()
}
/**
* Gets the cookie name
*
* @return string
* {@inheritDoc}
*/
public function getName()
{
@@ -245,20 +236,27 @@ protected function validateName($name)
}
/**
* Gets the cookie value
*
* @return string|array
* {@inheritDoc}
*/
public function getValue()
{
return $this->value;

This comment has been minimized.

@stefanozoffoli

stefanozoffoli Sep 19, 2017

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().

@stefanozoffoli

stefanozoffoli Sep 19, 2017

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().

This comment has been minimized.

@stefanozoffoli

stefanozoffoli Sep 19, 2017

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()

@stefanozoffoli

stefanozoffoli Sep 19, 2017

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()

This comment has been minimized.

@markstory

markstory Sep 19, 2017

Member

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 💣

@markstory

markstory Sep 19, 2017

Member

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)
{
@@ -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)
{
@@ -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)
{
@@ -321,9 +311,7 @@ public function withDomain($domain)
}
/**
* Get the domain attribute.
*
* @return string
* {@inheritDoc}
*/
public function getDomain()
{
@@ -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)
{
@@ -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)
{
@@ -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)
{
@@ -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()
{
@@ -456,9 +424,7 @@ public function getExpiresTimestamp()
}
/**
* Builds the expiration value part of the header string
*
* @return string
* {@inheritDoc}
*/
public function getFormattedExpires()
{
@@ -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)
{
@@ -488,9 +449,7 @@ public function isExpired($time = null)
}
/**
* Create a new cookie that will virtually never expire.
*
* @return static
* {@inheritDoc}
*/
public function withNeverExpire()
{
@@ -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()
{
@@ -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();

This comment has been minimized.

@jeremyharris

jeremyharris Sep 19, 2017

Member

Any reason not to use __toString?

@jeremyharris

jeremyharris Sep 19, 2017

Member

Any reason not to use __toString?

This comment has been minimized.

@markstory

markstory Sep 19, 2017

Member

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

@markstory

markstory Sep 19, 2017

Member

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

This comment has been minimized.

@jeremyharris

jeremyharris Sep 19, 2017

Member

Yep, you're absolutely right! Disregard :)

@jeremyharris

jeremyharris Sep 19, 2017

Member

Yep, you're absolutely right! Disregard :)

/**
* Create a cookie with an updated value.
*
View
@@ -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(),
@@ -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
*
@@ -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
*
ProTip! Use n and p to navigate between commits in a pull request.