Hotfix/5118 #5286

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

ClemensSahs commented Oct 17, 2013

fix for #5118

Member

Ocramius commented Oct 17, 2013

@ClemensSahs seems broken to me. What if a cookie contains ;?

Contributor

basz commented Oct 18, 2013

Did you merge or just close?

Contributor

ClemensSahs commented Oct 18, 2013

@basz
oh ... no I delete the wrong branch in my REPO

@ClemensSahs ClemensSahs reopened this Oct 18, 2013

@ghost ghost assigned Maks3w Oct 20, 2013

Member

Maks3w commented Oct 20, 2013

Following the history of changes resume in #5118 (thank you to @kevinpapst) I suggest add an argument to getFieldValue for return a quoted or non quoted value

library/Zend/Http/Header/SetCookie.php
@@ -208,7 +208,7 @@ public function getFieldValue()
$value = urlencode($this->getValue());
- $fieldValue = $this->getName() . '="' . $value . '"';
+ $fieldValue = $this->getName() . '=' . $value . '';
@Maks3w

Maks3w Oct 20, 2013

Member

I guess last quotes are unnecessary :)

Contributor

ClemensSahs commented Oct 20, 2013

@Maks3w
hm... parameter was not a bad idea but we have than a problem with toString and toStringMultipleHeaders. Both of this method call the getFieldValue and must forward the parameter. In my mind this is not practicable.

Perhaps this is a possible Solutions for both cases? Some objections? If not I write tomorrow a test for that and push it in to this PR.

Set false by default

Sorry, true for preserve the BC

Remove the default value

cast to boolean before assign the value

Return the value of the property. Due all previous feedback the value is always a boolean

Member

Maks3w commented Oct 21, 2013

@ClemensSahs add your solution and the feedback.

Contributor

ClemensSahs commented Oct 23, 2013

I fine her if nobody have feedback we can merge this.

Contributor

basz commented Oct 23, 2013

+1

weierophinney added a commit that referenced this pull request Oct 23, 2013

weierophinney added a commit that referenced this pull request Oct 23, 2013

weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment