Skip to content

Conversation

ralt
Copy link
Contributor

@ralt ralt commented Sep 27, 2014

According to the IETF RFC 6265, only one Set-Cookie header should be sent per cookie name.

Fixes https://bugs.php.net/bug.php?id=67736

Cherry-picks and adapts 208deda (#795) for master

According to the IETF RFC 6265, only one Set-Cookie header should be sent per cookie name.

Fixes #67736

Cherry-picks and adapts 208deda for master
@@ -92,6 +93,13 @@ PHPAPI int php_setcookie(char *name, int name_len, char *value, int value_len, t
return FAILURE;
}

if (zend_hash_exists(SG(cookies), z_name) == 1) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "should not be used twice with the same name");
Copy link
Member

Choose a reason for hiding this comment

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

it's better also print the duplicated cookie name here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll see how I can phrase this.
Le 28 sept. 2014 07:38, "Xinchen Hui" notifications@github.com a écrit :

In ext/standard/head.c:

@@ -92,6 +93,13 @@ PHPAPI int php_setcookie(char *name, int name_len, char *value, int value_len, t
return FAILURE;
}

  • if (zend_hash_exists(SG(cookies), z_name) == 1) {
  •   php_error_docref(NULL TSRMLS_CC, E_WARNING, "should not be used twice with the same name");
    

it's better also print the duplicated cookie name here


Reply to this email directly or view it on GitHub
https://github.com/php/php-src/pull/849/files#r18127724.

@Danack
Copy link
Contributor

Danack commented May 24, 2015

This is still not an actual bug. Although RFC 6265 says

To maximize compatibility with user agents, servers SHOULD NOT produce two attributes with the same name in the same set-cookie-string. (See Section 5.3 for how user agents handle this case.)"

The meaning of 'Should not' explicitly means that the behaviour is not forbidden.

SHOULD NOT - This phrase, or the phrase "NOT RECOMMENDED" mean that
there may exist valid reasons in particular circumstances when the
particular behavior is acceptable or even useful, but the full
implications should be understood and the case carefully weighed
before implementing any behavior described with this label.

I don't believe giving a warning is appropriate behaviour either. If people need that safety built into their session management, they can easily build that protection in userland.

@krakjoe
Copy link
Member

krakjoe commented Jan 5, 2017

Since the author seems to have abandoned working on this (no response to very old previous comment), I'm closing this PR.

Please take this action as encouragement to open a clean PR that deals with the comments made here.

@krakjoe krakjoe closed this Jan 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants