Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Hotfix/5118 #5286

Closed
wants to merge 6 commits into from
Closed

Conversation

ClemensSahs
Copy link
Contributor

fix for #5118

@Ocramius
Copy link
Member

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

@ClemensSahs
Copy link
Contributor Author

@basz
Copy link
Contributor

basz commented Oct 18, 2013

Did you merge or just close?

@ClemensSahs
Copy link
Contributor Author

@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
@Maks3w
Copy link
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

@@ -208,7 +208,7 @@ public function getFieldValue()

$value = urlencode($this->getValue());

$fieldValue = $this->getName() . '="' . $value . '"';
$fieldValue = $this->getName() . '=' . $value . '';
Copy link
Member

Choose a reason for hiding this comment

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

I guess last quotes are unnecessary :)

@ClemensSahs
Copy link
Contributor Author

@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.

@Maks3w
Copy link
Member

Maks3w commented Oct 21, 2013

@ClemensSahs add your solution and the feedback.

@ClemensSahs
Copy link
Contributor Author

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

@basz
Copy link
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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants