Skip to content

Loading…

hotfix/4508 and make Zend\Http\Header\SetCookie RFC conform #5078

Closed
wants to merge 9 commits into from

3 participants

@ClemensSahs

hotfix for the issues #4508

@ClemensSahs

last thing to do is fix the bug in fromString.
quotes will be urlencoded

@weierophinney
Zend Framework member

@ClemensSahs Please see https://travis-ci.org/ClemensSahs/zf2/jobs/10994478#L475 - looks like either the test expectation is incorrect, or the urlencoding has gone awry.

@ClemensSahs

@weierophinney
yes i kown this error, but yesterday I has not time to fix this, too :(

the reason behind this error:

  • the test gives a string with qoutes
  • and current the fromString methode don't detect this right and encode this with urlencode

I think this is near this line perhaps we muss refactor the logic. Currently i have no finial solution.

@ClemensSahs

so I'm ready with this PR

but I found that Zend\Http\Header\SetCookie is not RFC conform...
in the RFC multiple cookie's was been defined by a new line

// current implementation
Set-Cookie: SID=31d4d96e407aad42; Path=/; Secure; HttpOnly, lang=en-US; Path=/; Domain=example.com

// standart conform
Set-Cookie: SID=31d4d96e407aad42; Path=/; Secure; HttpOnly
Set-Cookie: lang=en-US; Path=/; Domain=example.com
@ClemensSahs

ok I'm fine here, any annotations?

@weierophinney weierophinney added a commit that referenced this pull request
@weierophinney weierophinney Merge branch 'hotfix/5078' into develop
Forward port #5078
71610d7
@weierophinney weierophinney added a commit that closed this pull request
@weierophinney weierophinney Merge branch 'hotfix/5078'
Close #5078
fc33b0f
@weierophinney weierophinney was assigned
@weierophinney weierophinney referenced this pull request
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney referenced this pull request
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney referenced this pull request
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this pull request
@weierophinney weierophinney Merge pull request zendframework/zf2#5078 from ClemensSahs/hotfix/4508
hotfix/4508 and make Zend\Http\Header\SetCookie RFC conform
dc23b89
@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this pull request
@weierophinney weierophinney Merge branch 'hotfix/5078' 4c82c64
@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this pull request
@weierophinney weierophinney Merge branch 'hotfix/5078' into develop 2558f0c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 60 additions and 36 deletions.
  1. +7 −10 library/Zend/Http/Header/SetCookie.php
  2. +53 −26 tests/ZendTest/Http/Header/SetCookieTest.php
View
17 library/Zend/Http/Header/SetCookie.php
@@ -100,8 +100,9 @@ public static function fromString($headerLine, $bypassHeaderFieldName = false)
$keyValuePairs = preg_split('#;\s*#', $headerLine);
foreach ($keyValuePairs as $keyValue) {
- if (strpos($keyValue, '=')) {
- list($headerKey, $headerValue) = preg_split('#=\s*#', $keyValue, 2);
+ if (preg_match('#^(?<headerKey>[^=]+)=\s*("?)(?<headerValue>[^"]+)\2#',$keyValue,$matches)) {
+ $headerKey = $matches['headerKey'];
+ $headerValue= $matches['headerValue'];
} else {
$headerKey = $keyValue;
$headerValue = null;
@@ -205,13 +206,9 @@ public function getFieldValue()
return '';
}
- $value = $this->getValue();
- if (strpos($value, '"')!==false) {
- $value = '"'.urlencode(str_replace('"', '', $value)).'"';
- } else {
- $value = urlencode($value);
- }
- $fieldValue = $this->getName() . '=' . $value;
+ $value = urlencode($this->getValue());
+
+ $fieldValue = $this->getName() . '="' . $value . '"';
$version = $this->getVersion();
if ($version!==null) {
@@ -604,7 +601,7 @@ public function toStringMultipleHeaders(array $headers)
'The SetCookie multiple header implementation can only accept an array of SetCookie headers'
);
}
- $headerLine .= ', ' . $header->getFieldValue();
+ $headerLine .= "\n" . $header->toString();
}
return $headerLine;
}
View
79 tests/ZendTest/Http/Header/SetCookieTest.php
@@ -108,7 +108,7 @@ public function testSetCookieGetFieldValueReturnsProperValue()
$setCookieHeader->setSecure(true);
$setCookieHeader->setHttponly(true);
- $target = 'myname=myvalue; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
+ $target = 'myname="myvalue"; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
. ' Domain=docs.foo.com; Path=/accounts;'
. ' Secure; HttpOnly';
@@ -126,7 +126,7 @@ public function testSetCookieToStringReturnsHeaderFormattedString()
$setCookieHeader->setSecure(true);
$setCookieHeader->setHttponly(true);
- $target = 'Set-Cookie: myname=myvalue; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
+ $target = 'Set-Cookie: myname="myvalue"; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
. ' Domain=docs.foo.com; Path=/accounts;'
. ' Secure; HttpOnly';
@@ -147,9 +147,16 @@ public function testSetCookieCanAppendOtherHeadersInWhenCreatingString()
$appendCookie = new SetCookie('othername', 'othervalue');
$headerLine = $setCookieHeader->toStringMultipleHeaders(array($appendCookie));
- $target = 'Set-Cookie: myname=myvalue; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
+ $target = 'Set-Cookie: myname="myvalue"; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
. ' Domain=docs.foo.com; Path=/accounts;'
- . ' Secure; HttpOnly, othername=othervalue';
+ . ' Secure; HttpOnly, othername="othervalue"';
+ $this->assertNotEquals($target, $headerLine);
+
+ $target = 'Set-Cookie: myname="myvalue"; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
+ . ' Domain=docs.foo.com; Path=/accounts;'
+ . ' Secure; HttpOnly';
+ $target.= "\n";
+ $target.= 'Set-Cookie: othername="othervalue"';
$this->assertEquals($target, $headerLine);
}
@@ -164,7 +171,7 @@ public function testSetCookieAttributesAreUnsettable()
$setCookieHeader->setSecure(true);
$setCookieHeader->setHttponly(true);
- $target = 'myname=myvalue; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
+ $target = 'myname="myvalue"; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
. ' Domain=docs.foo.com; Path=/accounts;'
. ' Secure; HttpOnly';
$this->assertSame($target, $setCookieHeader->getFieldValue()); // attributes set
@@ -174,10 +181,10 @@ public function testSetCookieAttributesAreUnsettable()
$setCookieHeader->setPath(NULL);
$setCookieHeader->setSecure(NULL);
$setCookieHeader->setHttponly(NULL);
- $this->assertSame('myname=myvalue', $setCookieHeader->getFieldValue()); // attributes unset
+ $this->assertSame('myname="myvalue"', $setCookieHeader->getFieldValue()); // attributes unset
$setCookieHeader->setValue(NULL);
- $this->assertSame('myname=', $setCookieHeader->getFieldValue());
+ $this->assertSame('myname=""', $setCookieHeader->getFieldValue());
$this->assertNull($setCookieHeader->getValue());
$this->assertNull($setCookieHeader->getExpires());
$this->assertNull($setCookieHeader->getDomain());
@@ -199,7 +206,7 @@ public function testSetCookieFieldValueIsEmptyStringWhenNameIsUnset()
$setCookieHeader->setSecure(true);
$setCookieHeader->setHttponly(true);
- $target = 'myname=myvalue; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
+ $target = 'myname="myvalue"; Expires=Wed, 13-Jan-2021 22:23:01 GMT;'
. ' Domain=docs.foo.com; Path=/accounts;'
. ' Secure; HttpOnly';
$this->assertSame($target, $setCookieHeader->getFieldValue()); // not empty
@@ -220,7 +227,7 @@ public function testSetCookieSetExpiresWithZeroTimeStamp()
$setCookieHeader->setExpires(0);
$this->assertSame('Thu, 01-Jan-1970 00:00:00 GMT', $setCookieHeader->getExpires());
- $target = 'myname=myvalue; Expires=Thu, 01-Jan-1970 00:00:00 GMT';
+ $target = 'myname="myvalue"; Expires=Thu, 01-Jan-1970 00:00:00 GMT';
$this->assertSame($target, $setCookieHeader->getFieldValue());
}
@@ -237,7 +244,7 @@ public function testSetCookieSetExpiresWithUnixEpochString()
$this->assertSame('Thu, 01-Jan-1970 00:00:00 GMT', $setCookieHeader->getExpires());
$this->assertSame(0, $setCookieHeader->getExpires(true));
- $target = 'myname=myvalue; Expires=Thu, 01-Jan-1970 00:00:00 GMT';
+ $target = 'myname="myvalue"; Expires=Thu, 01-Jan-1970 00:00:00 GMT';
$this->assertSame($target, $setCookieHeader->getFieldValue());
}
@@ -306,6 +313,26 @@ public function testToString($cStr, $info, $expected)
$this->assertEquals($cookie->getFieldName() . ': ' . $expected, $cookie->toString());
}
+ public function testSetJsonValue()
+ {
+ $cookieName ="fooCookie";
+ $jsonData = json_encode(array('foo'=>'bar'));
+
+ $cookie= new SetCookie($cookieName,$jsonData);
+
+ $regExp = sprintf('#^%s="%s"#',$cookieName,urlencode($jsonData));
+ $this->assertRegExp($regExp,$cookie->getFieldValue());
+
+ $cookieName ="fooCookie";
+ $jsonData = json_encode(array('foo'=>'bar'));
+
+ $cookie= new SetCookie($cookieName,$jsonData);
+ $cookie->setDomain('example.org');
+
+ $regExp = sprintf('#^%s="%s"; Domain=#',$cookieName,urlencode($jsonData));
+ $this->assertRegExp($regExp,$cookie->getFieldValue());
+ }
+
/**
* Provide valid cookie strings with information about them
*
@@ -318,7 +345,7 @@ public static function validCookieWithInfoProvider()
return array(
array(
- 'Set-Cookie: justacookie=foo; domain=example.com',
+ 'Set-Cookie: justacookie="foo"; domain=example.com',
array(
'name' => 'justacookie',
'value' => 'foo',
@@ -328,10 +355,10 @@ public static function validCookieWithInfoProvider()
'secure' => false,
'httponly'=> false
),
- 'justacookie=foo; Domain=example.com'
+ 'justacookie="foo"; Domain=example.com'
),
array(
- 'Set-Cookie: expires=tomorrow; secure; path=/Space Out/; expires=Tue, 21-Nov-2006 08:33:44 GMT; domain=.example.com',
+ 'Set-Cookie: expires="tomorrow"; secure; path=/Space Out/; expires=Tue, 21-Nov-2006 08:33:44 GMT; domain=.example.com',
array(
'name' => 'expires',
'value' => 'tomorrow',
@@ -341,10 +368,10 @@ public static function validCookieWithInfoProvider()
'secure' => true,
'httponly'=> false
),
- 'expires=tomorrow; Expires=Tue, 21-Nov-2006 08:33:44 GMT; Domain=.example.com; Path=/Space Out/; Secure'
+ 'expires="tomorrow"; Expires=Tue, 21-Nov-2006 08:33:44 GMT; Domain=.example.com; Path=/Space Out/; Secure'
),
array(
- 'Set-Cookie: domain=unittests; expires=' . gmdate('D, d-M-Y H:i:s', $now) . ' GMT; domain=example.com; path=/some%20value/',
+ 'Set-Cookie: domain="unittests"; expires=' . gmdate('D, d-M-Y H:i:s', $now) . ' GMT; domain=example.com; path=/some%20value/',
array(
'name' => 'domain',
'value' => 'unittests',
@@ -354,10 +381,10 @@ public static function validCookieWithInfoProvider()
'secure' => false,
'httponly'=> false
),
- 'domain=unittests; Expires=' . gmdate('D, d-M-Y H:i:s', $now) . ' GMT; Domain=example.com; Path=/some%20value/'
+ 'domain="unittests"; Expires=' . gmdate('D, d-M-Y H:i:s', $now) . ' GMT; Domain=example.com; Path=/some%20value/'
),
array(
- 'Set-Cookie: path=indexAction; path=/; domain=.foo.com; expires=' . gmdate('D, d-M-Y H:i:s', $yesterday) . ' GMT',
+ 'Set-Cookie: path="indexAction"; path=/; domain=.foo.com; expires=' . gmdate('D, d-M-Y H:i:s', $yesterday) . ' GMT',
array(
'name' => 'path',
'value' => 'indexAction',
@@ -367,11 +394,11 @@ public static function validCookieWithInfoProvider()
'secure' => false,
'httponly'=> false
),
- 'path=indexAction; Expires=' . gmdate('D, d-M-Y H:i:s', $yesterday) . ' GMT; Domain=.foo.com; Path=/'
+ 'path="indexAction"; Expires=' . gmdate('D, d-M-Y H:i:s', $yesterday) . ' GMT; Domain=.foo.com; Path=/'
),
array(
- 'Set-Cookie: secure=sha1; secure; SECURE; domain=some.really.deep.domain.com',
+ 'Set-Cookie: secure="sha1"; secure; SECURE; domain=some.really.deep.domain.com',
array(
'name' => 'secure',
'value' => 'sha1',
@@ -381,10 +408,10 @@ public static function validCookieWithInfoProvider()
'secure' => true,
'httponly'=> false
),
- 'secure=sha1; Domain=some.really.deep.domain.com; Secure'
+ 'secure="sha1"; Domain=some.really.deep.domain.com; Secure'
),
array(
- 'Set-Cookie: justacookie=foo; domain=example.com; httpOnly',
+ 'Set-Cookie: justacookie="foo"; domain=example.com; httpOnly',
array(
'name' => 'justacookie',
'value' => 'foo',
@@ -394,10 +421,10 @@ public static function validCookieWithInfoProvider()
'secure' => false,
'httponly'=> true
),
- 'justacookie=foo; Domain=example.com; HttpOnly'
+ 'justacookie="foo"; Domain=example.com; HttpOnly'
),
array(
- 'Set-Cookie: PHPSESSID=123456789+abcd%2Cef; secure; domain=.localdomain; path=/foo/baz; expires=Tue, 21-Nov-2006 08:33:44 GMT;',
+ 'Set-Cookie: PHPSESSID="123456789+abcd%2Cef"; secure; domain=.localdomain; path=/foo/baz; expires=Tue, 21-Nov-2006 08:33:44 GMT;',
array(
'name' => 'PHPSESSID',
'value' => '123456789+abcd%2Cef',
@@ -407,10 +434,10 @@ public static function validCookieWithInfoProvider()
'secure' => true,
'httponly'=> false
),
- 'PHPSESSID=123456789+abcd%2Cef; Expires=Tue, 21-Nov-2006 08:33:44 GMT; Domain=.localdomain; Path=/foo/baz; Secure'
+ 'PHPSESSID="123456789+abcd%2Cef"; Expires=Tue, 21-Nov-2006 08:33:44 GMT; Domain=.localdomain; Path=/foo/baz; Secure'
),
array(
- 'Set-Cookie: myname=myvalue; Domain=docs.foo.com; Path=/accounts; Expires=Wed, 13-Jan-2021 22:23:01 GMT; Secure; HttpOnly',
+ 'Set-Cookie: myname="myvalue"; Domain=docs.foo.com; Path=/accounts; Expires=Wed, 13-Jan-2021 22:23:01 GMT; Secure; HttpOnly',
array(
'name' => 'myname',
'value' => 'myvalue',
@@ -420,7 +447,7 @@ public static function validCookieWithInfoProvider()
'secure' => true,
'httponly'=> true
),
- 'myname=myvalue; Expires=Wed, 13-Jan-2021 22:23:01 GMT; Domain=docs.foo.com; Path=/accounts; Secure; HttpOnly'
+ 'myname="myvalue"; Expires=Wed, 13-Jan-2021 22:23:01 GMT; Domain=docs.foo.com; Path=/accounts; Secure; HttpOnly'
),
array(
'Set-Cookie:',
Something went wrong with that request. Please try again.