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

Closed
wants to merge 9 commits into
from

Conversation

Projects
None yet
3 participants
@ClemensSahs
Contributor

ClemensSahs commented Sep 4, 2013

hotfix for the issues zendframework#4508

@ClemensSahs

This comment has been minimized.

Show comment
Hide comment
@ClemensSahs

ClemensSahs Sep 4, 2013

Contributor

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

Contributor

ClemensSahs commented Sep 4, 2013

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

@weierophinney

This comment has been minimized.

Show comment
Hide comment
@weierophinney

weierophinney Sep 5, 2013

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.

Member

weierophinney commented Sep 5, 2013

@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

This comment has been minimized.

Show comment
Hide comment
@ClemensSahs

ClemensSahs Sep 5, 2013

Contributor

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

Contributor

ClemensSahs commented Sep 5, 2013

@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

This comment has been minimized.

Show comment
Hide comment
@ClemensSahs

ClemensSahs Sep 5, 2013

Contributor

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
Contributor

ClemensSahs commented Sep 5, 2013

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

This comment has been minimized.

Show comment
Hide comment
@ClemensSahs

ClemensSahs Sep 6, 2013

Contributor

ok I'm fine here, any annotations?

Contributor

ClemensSahs commented Sep 6, 2013

ok I'm fine here, any annotations?

weierophinney added a commit that referenced this pull request Sep 6, 2013

Merge pull request #5078 from ClemensSahs/hotfix/4508
hotfix/4508 and make Zend\Http\Header\SetCookie RFC conform

weierophinney added a commit that referenced this pull request Sep 6, 2013

@ghost ghost assigned weierophinney Sep 6, 2013

@kevinpapst

This comment has been minimized.

Show comment
Hide comment

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

Merge pull request zendframework/zendframework#5078 from ClemensSahs/…
…hotfix/4508

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

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