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
