Hi Yasuo,

I still have an issue with that. I believe the correct behaviour here is 
(assuming the `replace` argument to header() is honoured) what you’re seeing. 
Yes, it might be *unexpected* for new users, but its also *expected* by 
millions of current users/projects.

I would suggest perhaps a warning on the header() docs page, and perhaps an 
example to avoid the issue on the Session handling page.

Leaving it as-is, with improved docs means all functionality is *possible* with 
the right arguments.

Changing to your proposal means advanced use-cases are *impossible* with any 
arguments.


I realise you’re trying to remove WTF cases, but I don’t think removing 
advanced capabilities is the way to do that.


Cheers

Stephen


> On 19 Oct 2016, at 08:00, Yasuo Ohgaki <yohg...@ohgaki.net> wrote:
> 
> Hi all,
> 
> On Tue, Oct 18, 2016 at 4:31 PM, Yasuo Ohgaki <yohg...@ohgaki.net> wrote:
>> I understand why header() is made to remove all headers of the same
>> name. This is needed in some cases, but it does not work well for some
>> cases.
>> 
>> We need to decide what to do with
>> https://bugs.php.net/bug.php?id=72997
>> 
>> There is 2 issues.
>>  - header() removes all headers of the same name including 'Set-Cookie'
>>  - header() ignores replace flag. (This one is easy to fix)
>> 
>> Since header() enables 'replace flag' by default, it removes all
>> 'Set-Cookie' headers sent previously by default. It can easily disturb
>> security related cookies to work. i.e. Session ID cookie, Auto Login
>> cookie. This bug would be very hard to find for normal users, too.
>> 
>> Restoring older behavior (Removing only one header) cannot be a
>> resolution because it can still disturb security related cookies.
>> 
>> Possible resolutions:
>> 
>> - Prohibit 'Set-Cookie' for header() and force users to use setcookie()
>> - Mitigate by disabling replace flag by default. (This is not a good idea, 
>> IMO)
>> 
>> Both resolution requires BC, but this is better to be fixed ASAP.
>> 
>> Non-BC resolution could be:
>>  - "Ask users to use setcookie() always for 'Set-Cookie'".
>> 
>> I would like to prohibit 'Set-Cookie' by header() because it may
>> remove session ID cookie as well as auto login cookie, etc. If we
>> leave released version as it is now, I would like to prohibit
>> 'Set-Cookie' by header() in PHP 7.1.
>> 
>> Problem with this may be that user cannot modify 'Set-Cookie' header
>> line as user want.
>> 
>> $ php -r 'setcookie("REMEMBERME=value; expires=Sat, 03-Sep-2020
>> 05:38:43 GMT; path=/; domain=aaa");'
>> PHP Warning:  Cookie names cannot contain any of the following '=,;
>> \t\r\n\013\014' in Command line code on line 1
>> 
>> 
>> Comments?
> 
> An idea for session ID protection.
> 
> Following code results in lost session always.
> 
> <?php
> session_start();
> session_regenerate_id();
> header('Set-Cookie: something');
> ?>
> 
> header() function removes all header of the same name, e.g.
> Set-Cookie, Expires, etc, by sapi_remove_header(). This could be very
> hard to find the cause.
> 
> 
> This risk can be removed w/o much BC. Only BC is when user is
> intentionally trying to delete session ID cookie manually. This would
> be very rare. We can add code that excludes session ID cookie in
> sapi_remove_header().
> http://lxr.php.net/xref/PHP-MASTER/main/SAPI.c#593
> 
> To do that, we can search string like following
> Set-Cookie: PHPSESSID=xxxxxxx
> 
> The only issue is we need session global, i.e. PS(session_name), at
> least. It's not nice to have dependency from SAPI.c to session, but it
> protects session ID from removed by users by mistake.
> 
> Any comments?
> 
> Regards,
> 
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
> 
> -- 
> 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

Reply via email to