Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 03 Nov 2014, at 02:21, Dan Ackroyd dan...@basereality.com wrote: On 2 November 2014 15:10, Rowan Collins rowan.coll...@gmail.com wrote: 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 02 11 2014, at 01:33, Rowan Collins rowan.coll...@gmail.com wrote: On 01/11/2014 22:24, Andrea Faulds 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. 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
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
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
On 1 November 2014 17:09, Florian Margaine flor...@margaine.com wrote:
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 Tue, Sep 9, 2014 at 4:53 PM, Chris Wright c...@daverandom.com wrote:
On 8 September 2014 09:09, Ferenc Kovacs tyr...@gmail.com wrote:
On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly...@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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
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 On 1 November 2014 22:24, Andrea Faulds a...@ajf.me wrote: On 1 Nov 2014, at 21:47, Rowan Collins rowan.coll...@gmail.com wrote: On 01/11/2014 21:10, Alexander Kurilo wrote: 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/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 2 November 2014 13:49:15 GMT, Dan Ackroyd dan...@basereality.com wrote: 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] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
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/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 2 November 2014 15:10, Rowan Collins rowan.coll...@gmail.com wrote: 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
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 Tue, Sep 9, 2014 at 4:53 PM, Chris Wright c...@daverandom.com wrote:
On 8 September 2014 09:09, Ferenc Kovacs tyr...@gmail.com wrote:
On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly...@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
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 01/11/14 13:09, Florian Margaine wrote:
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 Tue, Sep 9, 2014 at 4:53 PM, Chris Wright c...@daverandom.com wrote:
On 8 September 2014 09:09, Ferenc Kovacs tyr...@gmail.com wrote:
On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly...@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,
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 01/11/2014 21:10, Alexander Kurilo wrote: 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] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 1 Nov 2014, at 21:47, Rowan Collins rowan.coll...@gmail.com wrote: On 01/11/2014 21:10, Alexander Kurilo wrote: 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/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 01/11/2014 22:24, Andrea Faulds 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. 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] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
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 On 8 September 2014 07:50, Tjerk Meesters tjerk.meest...@gmail.com wrote: Hi! On Sat, Sep 6, 2014 at 5:38 AM, Florian Margaine flor...@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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On 8 September 2014 09:09, Ferenc Kovacs tyr...@gmail.com wrote: On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly...@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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
Hi! On Sat, Sep 6, 2014 at 5:38 AM, Florian Margaine flor...@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
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
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.meest...@gmail.com wrote: Hi! On Sat, Sep 6, 2014 at 5:38 AM, Florian Margaine flor...@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
Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan theanomaly...@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
[PHP-DEV] setcookie() minor BC break - fixes issue #67736
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*
