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

Zend\Http\Header\SetCookie changed to support empty cookies #5476

Closed
wants to merge 6 commits into from
Closed

Zend\Http\Header\SetCookie changed to support empty cookies #5476

wants to merge 6 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 14, 2013

$keyValue is receiving value with '=' when the cookie is empty.

Issue #5475

$keyValue is receiving value with '=' when the cookie is empty.

Issue #5475
@Maks3w
Copy link
Member

Maks3w commented Nov 14, 2013

Please add a unit test too

@ClemensSahs
Copy link
Contributor

I write some tests for this problem, I'm sorry to say this patch is not working.

currently I'm try to fix this in every case its possible

all following case we must detect correctly

array(
'Set-Cookie: emptykey=  ; Domain=docs.foo.com;', // current failing
'Set-Cookie: emptykey=; Domain=docs.foo.com;',
'Set-Cookie: emptykey; Domain=docs.foo.com;'
);

is there a some other one?

@ClemensSahs
Copy link
Contributor

currently your patch run at this CI-Job

@ClemensSahs
Copy link
Contributor

In my mind a fix in the else branch is only a workaround, we have the if branch to cover all keyValuePairs:

  • regular values
  • with spaces
  • empty but with "="

the else branch will be for tags like "httponly" or "secure"

what is your opinion?

@ghost
Copy link
Author

ghost commented Nov 22, 2013

I agree with you. I had not realized it.

The current regex have a plus sign to match headerValue '#^(?P[^=]+)=\s*("?)(?P[^"]+)\2#'
Plus attempt to match the preceding token once or more times, so it doesnt match empty strings.

I'll revert the past changes and change the regex to '#^(?P[^=]+)=\s_("?)(?P[^"]_)\2#'
Star attempt to match the preceding token zero or more times, so it will match empty strings.

@ClemensSahs
Copy link
Contributor

@Maks3w
tests are implement and run fine
this PR is ready for merge

@jcq
Copy link

jcq commented Jan 8, 2014

Any word on this getting merged? For what it's worth, I had a similar problem, and this appears to have fixed it.

@weierophinney weierophinney added this to the 2.2.6 milestone Mar 3, 2014
@weierophinney weierophinney self-assigned this Mar 3, 2014
weierophinney added a commit that referenced this pull request Mar 3, 2014
Zend\Http\Header\SetCookie changed to support empty cookies
weierophinney added a commit that referenced this pull request Mar 3, 2014
@ralphschindler
Copy link
Member

I suppose it is fine for parsing needs that this particular class be a bit more liberal than the spec. But for the record, I don't see anywhere in the cookie spec that says values can be empty: https://www.ietf.org/rfc/rfc2109.txt

@ClemensSahs
Copy link
Contributor

@ralphschindler
The actually RFC is 6265 at this point 3.1 we see a empty definition to remove the cookie.

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

Zend\Http\Header\SetCookie changed to support empty cookies
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants