setcookie() minor BC break - fixes issue #67736

10 years ago by Florian Margaine — view source

unread

Hi,

This is a minor BC break, but still a BC break, so worth discussing on this
ML.

When a second setcookie() is done with the same name, a warning is emitted,
because the ietf rfc 6265 says it should only send one Set-Cookie header
per name.

This is fine when display_errors is set to off. When it's set to on, the
warning prevents the header from being added because "headers already sent"
(which is the minor BC break, as current PHP just sends 2 Set-Cookie
headers with the same name).

So, should it be merged? What should be done to comply with the ietf rfc
6265?

PR: https://github.com/php/php-src/pull/795/
PHP issue: https://bugs.php.net/bug.php?id=67736

Regards,

Florian Margaine

10 years ago by Tjerk Meesters — view source

unread

Hi!

On Sat, Sep 6, 2014 at 5:38 AM, Florian Margaine florian@margaine.com
wrote:

Hi,

This is a minor BC break, but still a BC break, so worth discussing on this
ML.

When a second setcookie() is done with the same name, a warning is emitted,
because the ietf rfc 6265 says it should only send one Set-Cookie header
per name.

This is fine when display_errors is set to off. When it's set to on, the
warning prevents the header from being added because "headers already sent"
(which is the minor BC break, as current PHP just sends 2 Set-Cookie
headers with the same name).

Yeah, it would prevent any header() or setcookie() following that warning
from taking place.

How about delaying that warning until the headers are sent?

So, should it be merged? What should be done to comply with the ietf rfc
6265?

PR: https://github.com/php/php-src/pull/795/
PHP issue: https://bugs.php.net/bug.php?id=67736

Regards,

Florian Margaine

--

Tjerk

10 years ago by Sherif Ramadan — view source

unread

Actually, we shouldn't be doing that all. We should simply just overwrite
the header. It wouldn't make much sense to set two headers with the same
cookie name when we can just overwrite it.

On Mon, Sep 8, 2014 at 2:50 AM, Tjerk Meesters tjerk.meesters@gmail.com
wrote:

Hi!

On Sat, Sep 6, 2014 at 5:38 AM, Florian Margaine florian@margaine.com
wrote:

Hi,

This is a minor BC break, but still a BC break, so worth discussing on
this
ML.

When a second setcookie() is done with the same name, a warning is
emitted,
because the ietf rfc 6265 says it should only send one Set-Cookie
header
per name.

This is fine when display_errors is set to off. When it's set to on, the
warning prevents the header from being added because "headers already
sent"
(which is the minor BC break, as current PHP just sends 2 Set-Cookie
headers with the same name).

Yeah, it would prevent any header() or setcookie() following that warning
from taking place.

How about delaying that warning until the headers are sent?

So, should it be merged? What should be done to comply with the ietf rfc
6265?

PR: https://github.com/php/php-src/pull/795/
PHP issue: https://bugs.php.net/bug.php?id=67736

Regards,

Florian Margaine

--

Tjerk

10 years ago by Michael Wallner — view source

unread

Actually, we shouldn't be doing that all. We should simply just overwrite
the header. It wouldn't make much sense to set two headers with the same
cookie name when we can just overwrite it.

Maybe, but the SAPI_HEADER_* only look for the header name, not its
contents.

--
Regards,
Mike

10 years ago by Ferenc Kovacs — view source

unread

On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly.is@gmail.com
wrote:

Actually, we shouldn't be doing that all. We should simply just overwrite
the header. It wouldn't make much sense to set two headers with the same
cookie name when we can just overwrite it.

that would be a bigger BC break, and without a warning, those people
affected by the change will have a harder time figuring out what went wrong.
and as was discussed both in the PR and the bugreport the rfc discourages
but doesn't prohibit this behavior, so making it impossible for the
userland to do it would be a bit of an arbitrary restriction.
maybe we could do something like introducing a new $overwrite = true option
to the function signature, but setcookie already has 7 arguments, so
probably isn't a great idea.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

10 years ago by Chris Wright — view source

unread

On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly.is@gmail.com
wrote:

Actually, we shouldn't be doing that all. We should simply just overwrite
the header. It wouldn't make much sense to set two headers with the same
cookie name when we can just overwrite it.

that would be a bigger BC break, and without a warning, those people
affected by the change will have a harder time figuring out what went wrong.
and as was discussed both in the PR and the bugreport the rfc discourages
but doesn't prohibit this behavior, so making it impossible for the
userland to do it would be a bit of an arbitrary restriction.
maybe we could do something like introducing a new $overwrite = true option
to the function signature, but setcookie already has 7 arguments, so
probably isn't a great idea.

How about changing the signature of setcookie() to:

bool setcookie (
string $name
[, string $value
[, int $expire = 0
[, string $path
[, string $domain
[, int $flags = 0
[, bool $httponly = false
]]]]]])

And creating the following constants for use as flags:

const COOKIE_SECURE = 1;
const COOKIE_HTTPONLY = 2;
const COOKIE_OVERWRITE = 4;

This will also be a small BC break, but would potentially be a
graceful way to trim an argument off setcookie() with minimal impact
to users, making the signature more user-friendly (IMO) and allowing
additional functionality to be added in the future by means of more
flags.

Setting the value of COOKIE_SECURE to 1 covers what I suspect will be
the most common case where the value of this argument is not a bool or
NULL, i.e. specifying 1/0 to reduce line length. If the $httponly arg
is specified and not NULL it can override the value for that element
of $flags. Passing this last argument could emit an E_DEPRECATED,
either immediately or in the future.

This is also not an ideal solution and everyone may hate it, just
throwing it out there.

Thanks, Chris

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

10 years ago by Florian Margaine — view source

unread

Hi list,

On the PR https://github.com/php/php-src/pull/795/ of the setcookie patch
to become compliant with the HTTP RFC, a valid use case for not throwing a
warning when running setcookie() twice with the same name: deleting cookies
(setcookie('name', '', ...);). It's thus been proposed to add an
unsetcookie() method.

I'm not set on whether it's a good thing or not. I'm also not sure how to
handle this.

There are some facts:

the current way does not follow the HTTP RFC.
we cannot have the "last come last served" cookie because it's a BC break.
the current functions cannot allow us to unset a cookie.

What should be done? Trying to keep BC at a minimum by adding an
unsetcookie() method and add warnings? Try to detect the deletion of
cookies (empty value) and add warnings to keep even more BC?

I'm unsure on what should be done and would like internals' opinion :-)

On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly.is@gmail.com
wrote:

Actually, we shouldn't be doing that all. We should simply just
overwrite
the header. It wouldn't make much sense to set two headers with the same
cookie name when we can just overwrite it.

that would be a bigger BC break, and without a warning, those people
affected by the change will have a harder time figuring out what went
wrong.
and as was discussed both in the PR and the bugreport the rfc discourages
but doesn't prohibit this behavior, so making it impossible for the
userland to do it would be a bit of an arbitrary restriction.
maybe we could do something like introducing a new $overwrite = true
option
to the function signature, but setcookie already has 7 arguments, so
probably isn't a great idea.

--

Ferenc Kovács
@Tyr43l - http://tyrael.hu

Regards,

--
Florian Margaine

10 years ago by Alexander Kurilo — view source

unread

Hi list,

On the PR https://github.com/php/php-src/pull/795/ of the setcookie patch
to become compliant with the HTTP RFC, a valid use case for not throwing a
warning when running setcookie() twice with the same name: deleting cookies
(setcookie('name', '', ...);). It's thus been proposed to add an
unsetcookie() method.

I'm not set on whether it's a good thing or not. I'm also not sure how to
handle this.

There are some facts:

the current way does not follow the HTTP RFC.

we cannot have the "last come last served" cookie because it's a BC break.

the current functions cannot allow us to unset a cookie.

What should be done? Trying to keep BC at a minimum by adding an
unsetcookie() method and add warnings? Try to detect the deletion of
cookies (empty value) and add warnings to keep even more BC?
Just in case: I believe cookies are removed not by setting an empty
value to them, but by setting their expiration date to a timestamp the past.

I'm unsure on what should be done and would like internals' opinion :-)

On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly.is@gmail.com
wrote:

Actually, we shouldn't be doing that all. We should simply just
overwrite
the header. It wouldn't make much sense to set two headers with the same
cookie name when we can just overwrite it.

that would be a bigger BC break, and without a warning, those people
affected by the change will have a harder time figuring out what went
wrong.
and as was discussed both in the PR and the bugreport the rfc discourages
but doesn't prohibit this behavior, so making it impossible for the
userland to do it would be a bit of an arbitrary restriction.
maybe we could do something like introducing a new $overwrite = true
option
to the function signature, but setcookie already has 7 arguments, so
probably isn't a great idea.

--

Ferenc Kovács
@Tyr43l - http://tyrael.hu

Regards,

10 years ago by Rowan Collins — view source

unread

What should be done? Trying to keep BC at a minimum by adding an
unsetcookie() method and add warnings? Try to detect the deletion of
cookies (empty value) and add warnings to keep even more BC?
Just in case: I believe cookies are removed not by setting an empty
value to them, but by setting their expiration date to a timestamp the
past.

My first thought on seeing unsetcookie() as a name was that it will lead
to a lot of confusion as to what it actually does; if you didn't know
about this discussion, would you expect it to:

a) remove a Set-Cookie header which has been added to the current
response (the actual behaviour required to avoid the behaviour that
SHOULD NOT be done according to the RFC), leaving any cookie the remote
UA has stored untouched?
b) add a Set-Cookie header to the response to expire an existing cookie
the remote UA might have stored (a useful feature, but unrelated to this
bug)?
c) replace any existing Set-Cookie header with that cookie name with one
which instead expires that cookie (i.e. both a and b at the same time)?

What seems to actually be wanted is more like replace_cookie() - "I
asked you to set this cookie earlier this page load, but actually set
this one instead". Or, equivalently, some change to setcookie()'s
parameter list to allow this behaviour, as others have suggested.

There is already a lot of confusion around the difference between the
cookies in $_COOKIE and those set with setcookie() (i.e. that your new
cookie won't show up in the superglobal until next request). The idea of
removing something from the list of cookies which are queued to be set
by this response is not something that's easy to explain clearly in that
context, so I think the naming and documentation of any such feature
needs to be approached very carefully.

Regards,

--
Rowan Collins
[IMSoP]

10 years ago by Andrea Faulds — view source

unread

What should be done? Trying to keep BC at a minimum by adding an
unsetcookie() method and add warnings? Try to detect the deletion of
cookies (empty value) and add warnings to keep even more BC?
Just in case: I believe cookies are removed not by setting an empty value to them, but by setting their expiration date to a timestamp the past.

My first thought on seeing unsetcookie() as a name was that it will lead to a lot of confusion as to what it actually does; if you didn't know about this discussion, would you expect it to:

a) remove a Set-Cookie header which has been added to the current response (the actual behaviour required to avoid the behaviour that SHOULD NOT be done according to the RFC), leaving any cookie the remote UA has stored untouched?
b) add a Set-Cookie header to the response to expire an existing cookie the remote UA might have stored (a useful feature, but unrelated to this bug)?
c) replace any existing Set-Cookie header with that cookie name with one which instead expires that cookie (i.e. both a and b at the same time)?

I’d expect it to do (a). But I can see your point with (b) and (c).

What seems to actually be wanted is more like replace_cookie() - "I asked you to set this cookie earlier this page load, but actually set this one instead". Or, equivalently, some change to setcookie()'s parameter list to allow this behaviour, as others have suggested.

There is already a lot of confusion around the difference between the cookies in $_COOKIE and those set with setcookie() (i.e. that your new cookie won't show up in the superglobal until next request). The idea of removing something from the list of cookies which are queued to be set by this response is not something that's easy to explain clearly in that context, so I think the naming and documentation of any such feature needs to be approached very carefully.

Perhaps it would be worth ditching any attempts to change setcookie() (just keep it around for backwards-compatibility), and to instead add a new function, function family, or indeed class for cookie handling. Some sort of sane API which would allow you to do a, b, or c, and make it clear which you wanted.

Thoughts? Any idea what such an API might look like?

Andrea Faulds
http://ajf.me/

10 years ago by Rowan Collins — view source

unread

Perhaps it would be worth ditching any attempts to change setcookie() (just keep it around for backwards-compatibility), and to instead add a new function, function family, or indeed class for cookie handling. Some sort of sane API which would allow you to do a, b, or c, and make it clear which you wanted.

Thoughts? Any idea what such an API might look like?

That rather brings us back to this week's Hot Topic: HTTP response
objects, pecl/http, and PSR-7.

pecl/httpv2 has the http\Cookie class [1], which confusingly represents
not a single cookie, but a whole set of cookies. Exactly how you attach
this to a response is not really clear from the docs I can find.

The current PSR-7 draft [2] appears to be lacking any direct support for
setting cookies, and mentions them only in the context of
Psr\Http\Message\IncomingRequestInterface. Perhaps someone who has been
following that discussion will be able to clarify why cookie handling
functions were not considered in scope for
Psr\Http\Message\ResponseInterface.

[1] http://devel-m6w6.rhcloud.com/mdref/http/Cookie
[2]
https://github.com/php-fig/fig-standards/blob/master/proposed/http-message.md

--
Rowan Collins
[IMSoP]

10 years ago by Michael Wallner — view source

unread

Perhaps it would be worth ditching any attempts to change setcookie() (just keep it around for backwards-compatibility), and to instead add a new function, function family, or indeed class for cookie handling. Some sort of sane API which would allow you to do a, b, or c, and make it clear which you wanted.

Thoughts? Any idea what such an API might look like?

That rather brings us back to this week's Hot Topic: HTTP response objects, pecl/http, and PSR-7.

pecl/httpv2 has the http\Cookie class [1], which confusingly represents not a single cookie, but a whole set of cookies. Exactly how you attach this to a response is not really clear from the docs I can find.

It it’s confusing, the docs are not good enough, and I probably have to agree here ;)
Anyway, http\Cookie can parse Cookie and Set-Cookie headers, though, looking at the RFC that might have been a bad idea to implement it that way.
Cookies never have been a main area of interest to me, so there might be a lot of room to improve in pecl/http.

Obviously, API for cookies is completely missing in the http\Env sub namespace, probably because there was $_COOKIE and set_cookie(), which may have covered my needs for it. Setting a cookie currently would mean just setting the appropriate Set-Cookie header on an http\Env\Response instance.

- - 8< - - -
    $ php -r '$r = new http\Env\Response;
    $r->setHeader("Set-Cookie",
    ((new http\Cookie("foo=bar"))->setExpires(strtotime("+1 day"))));
    $r->send(STDOUT);’

HTTP/1.1 200 OK
Set-Cookie: foo=bar; expires=Mon, 03 Nov 2014 10:21:43 GMT;
ETag: ""

- - 8 - - -

[1] http://devel-m6w6.rhcloud.com/mdref/http/Cookie http://devel-m6w6.rhcloud.com/mdref/http/Cookie

Cheers,
Mike

10 years ago by Dan Ackroyd — view source

unread

Andrea wrote:

Perhaps it would be worth ditching any attempts to change setcookie() (just keep it around for backwards-compatibility), and to instead add a new function, function family, or indeed class for cookie handling.

Thoughts? Any idea what such an API might look like?

I think this API could and should exist totally in userland.

Although it's nice that PHP ships with basic session support with it,
the exact details of session handling actually involve non-trivial
business logic

What happens when the users IP address changes?
What happens when a user tries to use session ID that has been
invalidated through session regeneration 2 seconds ago, 30 seconds.
What rules should be applied to check the session integrity e.g.
user-agent checks.

All of the above can be done 'trivially' in userland. There is
absolutely no need to attempt to put the functionality for this in
core.

cheers
Dan

What should be done? Trying to keep BC at a minimum by adding an
unsetcookie() method and add warnings? Try to detect the deletion of
cookies (empty value) and add warnings to keep even more BC?
Just in case: I believe cookies are removed not by setting an empty value to them, but by setting their expiration date to a timestamp the past.

My first thought on seeing unsetcookie() as a name was that it will lead to a lot of confusion as to what it actually does; if you didn't know about this discussion, would you expect it to:

a) remove a Set-Cookie header which has been added to the current response (the actual behaviour required to avoid the behaviour that SHOULD NOT be done according to the RFC), leaving any cookie the remote UA has stored untouched?
b) add a Set-Cookie header to the response to expire an existing cookie the remote UA might have stored (a useful feature, but unrelated to this bug)?
c) replace any existing Set-Cookie header with that cookie name with one which instead expires that cookie (i.e. both a and b at the same time)?

I’d expect it to do (a). But I can see your point with (b) and (c).

What seems to actually be wanted is more like replace_cookie() - "I asked you to set this cookie earlier this page load, but actually set this one instead". Or, equivalently, some change to setcookie()'s parameter list to allow this behaviour, as others have suggested.

There is already a lot of confusion around the difference between the cookies in $_COOKIE and those set with setcookie() (i.e. that your new cookie won't show up in the superglobal until next request). The idea of removing something from the list of cookies which are queued to be set by this response is not something that's easy to explain clearly in that context, so I think the naming and documentation of any such feature needs to be approached very carefully.

Perhaps it would be worth ditching any attempts to change setcookie() (just keep it around for backwards-compatibility), and to instead add a new function, function family, or indeed class for cookie handling. Some sort of sane API which would allow you to do a, b, or c, and make it clear which you wanted.

Thoughts? Any idea what such an API might look like?

Andrea Faulds
http://ajf.me/

10 years ago by Rowan Collins — view source

unread

Andrea wrote:

Perhaps it would be worth ditching any attempts to change setcookie()
(just keep it around for backwards-compatibility), and to instead add a
new function, function family, or indeed class for cookie handling.

Thoughts? Any idea what such an API might look like?

I think this API could and should exist totally in userland.

Although it's nice that PHP ships with basic session support with it,
the exact details of session handling actually involve non-trivial
business logic

Wait, what does session handling have to do with any of this? We're talking about setting, manipulating, and clearing cookies here, not trying to second guess what people are using them for.

Cookie-handling APIs are pretty much universally awful, from the HTTP headers themselves to JS's document.cookie voodoo. setcookie() is part of that trend, with its long list of positional parameters which need specifying every time.

If a new flexible high-level API for HTTP response logic, along the lines of pecl/http or PSR-7, were introduced, it might be nice to attempt something less ugly as part of that effort. I'd tend to agree, though, that the bug as raised should be treated as user error, or ill-advised user intention, by the existing low-level API.

Rowan Collins
[IMSoP]

10 years ago by Dan Ackroyd — view source

unread

Wait, what does session handling have to do with any of this?

Sorry, I completely failed to write what I was trying to say there.

I meant that like the session handling functions and setcookie are
similar in that:

They don't feel that nice to use.
The exact behaviour of them is currently correct but doesn't cater
for higher level logic e.g. making a script setting the same cookie be
an error rather than accepting it.

Both of them could probably do with being made nicer - but that can be
done in user-land. There's no need to fix them in the core
libraries.

cheers
Dan

10 years ago by Alexey Zakhlestin — view source

unread

Wait, what does session handling have to do with any of this?

Sorry, I completely failed to write what I was trying to say there.

I meant that like the session handling functions and setcookie are
similar in that:

They don't feel that nice to use.

The exact behaviour of them is currently correct but doesn't cater
for higher level logic e.g. making a script setting the same cookie be
an error rather than accepting it.

Both of them could probably do with being made nicer - but that can be
done in user-land. There's no need to fix them in the core
libraries.

well, one thing which could be done is decoupling building of cookie-header from adding this header to the response.

setcookie(…) = header(build_cookie_header(…))

this would be much closer to what I would call a low-level API

--
Alexey Zakhlestin
CTO at Grids.by/you
https://github.com/indeyets
PGP key: http://indeyets.ru/alexey.zakhlestin.pgp.asc

10 years ago by Dan Ackroyd — view source

unread

Florian wrote:

On the PR https://github.com/php/php-src/pull/795/ of the setcookie patch
to become compliant with the HTTP RFC,
There are some facts:

the current way does not follow the HTTP RFC.

Please stop saying this, it isn't true.

From rfc6265:

Servers SHOULD NOT include more than one Set-Cookie header field in the same response with the same cookie-name.:

From rfc2119:

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

The behaviour of PHP is ABSOLUTELY in compliance with the RFC 6265.
Setting two headers may not follow best practice but it is conformant,
and it is only doing what the users PHP script told it to do.

The problem that you're actually trying to address is that for most
people, they didn't mean to send two cookie headers twice and only did
it because they have a bug in their code, with the same header being
sent twice:

setcookie('foo', 'bar1');
setcookie('foo', 'bar2');

The correct way to fix this is not to introduce a new function that
alters global state; the correct way to it is for the user to just fix
the bug in their code, and only call setcookie once:

$fooCookie = 'bar1';
$fooCookie = 'bar2';
setcookie('foo', $fooCookie);

Adding a new function just to allow people to work round a bug in
their code sounds nuts.

we cannot have the "last come last served" cookie because it's a BC break.

Just to reiterate, it would also be incorrect behaviour. Setting is
multiple cookies with the same name is allowed, just not recommended.

I think the generating a warning on setting a duplicate header is an
ok idea. That would help most people (who presumably didn't mean to do
that) but be slightly annoying for people who have an existing
application that deliberately uses multiple setcookie with the same
name, as they would need to add error suppression.

cheers
Dan

Hi list,

On the PR https://github.com/php/php-src/pull/795/ of the setcookie patch
to become compliant with the HTTP RFC, a valid use case for not throwing a
warning when running setcookie() twice with the same name: deleting cookies
(setcookie('name', '', ...);). It's thus been proposed to add an
unsetcookie() method.

I'm not set on whether it's a good thing or not. I'm also not sure how to
handle this.

There are some facts:

the current way does not follow the HTTP RFC.

we cannot have the "last come last served" cookie because it's a BC break.

the current functions cannot allow us to unset a cookie.

What should be done? Trying to keep BC at a minimum by adding an
unsetcookie() method and add warnings? Try to detect the deletion of
cookies (empty value) and add warnings to keep even more BC?

I'm unsure on what should be done and would like internals' opinion :-)

On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly.is@gmail.com
wrote:

Actually, we shouldn't be doing that all. We should simply just
overwrite
the header. It wouldn't make much sense to set two headers with the same
cookie name when we can just overwrite it.

that would be a bigger BC break, and without a warning, those people
affected by the change will have a harder time figuring out what went
wrong.
and as was discussed both in the PR and the bugreport the rfc discourages
but doesn't prohibit this behavior, so making it impossible for the
userland to do it would be a bit of an arbitrary restriction.
maybe we could do something like introducing a new $overwrite = true
option
to the function signature, but setcookie already has 7 arguments, so
probably isn't a great idea.

--

Ferenc Kovács
@Tyr43l - http://tyrael.hu

Regards,

--
Florian Margaine

10 years ago by Stas Malyshev — view source

unread

Hi!

The behaviour of PHP is ABSOLUTELY in compliance with the RFC 6265.
Setting two headers may not follow best practice but it is conformant,
and it is only doing what the users PHP script told it to do.

I think I agree here - we're providing a low level API, and if somebody
uses this API in a manner contrary to best practices, it's on them, but
as long as it is not prohibited by the RFC, we should support this
behavior in case the users have good reasons to do this. Yes, in 99% of
cases it would not be a good reason - that's why it is not the best
practice - but as it is not prohibited, there would be valid 1% of cases
where it is required by the user.

As I see now, the behavior of following user instructions does not break
anything, right? It just allows the user to do something that the RFC
says he SHOULD NOT do. But I don't think this is a priority to enforce
best practices in a low level API, risking breaking BC and causing
trouble. If there's a case for higher level API enforcing the best
practices, fine, HTTP classes (like pecl/http ones) or frameworks can
always handle this.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/

10 years ago by Dan Ackroyd — view source

unread

How about delaying that warning until the headers are sent?

I don't think there would be any benefit to that. The only possible
scenarios are:

i) People sending duplicate headers by accident. When they see the
error, they should fix their code, so the error goes away.

ii) People who are deliberately sending duplicate headers (e.g. for a
custom client that requires them). They will be suppressing the error
and so it won't be seen anywhere.

As I said in the PR, the current behaviour isn't a PHP bug, it's doing
exactly what it was told to do by callee.

Changing the behaviour of setcookie to 'guess' what the programmer
really meant would definitely be wrong, and I'm not sure that giving a
warning is all that helpful either.

cheers
Dan

Hi!

On Sat, Sep 6, 2014 at 5:38 AM, Florian Margaine florian@margaine.com
wrote:

Hi,

This is a minor BC break, but still a BC break, so worth discussing on this
ML.

When a second setcookie() is done with the same name, a warning is emitted,
because the ietf rfc 6265 says it should only send one Set-Cookie header
per name.

This is fine when display_errors is set to off. When it's set to on, the
warning prevents the header from being added because "headers already sent"
(which is the minor BC break, as current PHP just sends 2 Set-Cookie
headers with the same name).

Yeah, it would prevent any header() or setcookie() following that warning
from taking place.

How about delaying that warning until the headers are sent?

So, should it be merged? What should be done to comply with the ietf rfc
6265?

PR: https://github.com/php/php-src/pull/795/
PHP issue: https://bugs.php.net/bug.php?id=67736

Regards,

Florian Margaine

--

Tjerk