Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96460
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain koalephant.com designates 206.123.115.54 as permitted sender)
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
In-Reply-To: <CAGa2bXYShMMU964gtq4VZfkBssJftJX-k2j+hRh6_YAyxm9mUw@mail.gmail.com>
Date: Wed, 19 Oct 2016 10:18:20 +0700
Cc: "internals@lists.php.net" <internals@lists.php.net>,
 Davey Shafik <davey@php.net>,
 Xinchen Hui <laruence@php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <1E4CA882-D433-413A-A369-E6340DD0078A@koalephant.com>
References: <CAGa2bXb0CKP6=eZZk2atR5XGtR8JmQ3FG756wn0J7kkPGYFyZg@mail.gmail.com> <CAGa2bXYShMMU964gtq4VZfkBssJftJX-k2j+hRh6_YAyxm9mUw@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Subject: Re: [PHP-DEV] Re: header() removes all header of the same name.
From: php-lists@koalephant.com (Stephen Reay)

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=E2=80=99=
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=E2=80=99re trying to remove WTF cases, but I don=E2=80=99t =
think removing advanced capabilities is the way to do that.


Cheers

Stephen


> On 19 Oct 2016, at 08:00, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>=20
> Hi all,
>=20
> On Tue, Oct 18, 2016 at 4:31 PM, Yasuo Ohgaki <yohgaki@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.
>>=20
>> We need to decide what to do with
>> https://bugs.php.net/bug.php?id=3D72997
>>=20
>> 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)
>>=20
>> 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.
>>=20
>> Restoring older behavior (Removing only one header) cannot be a
>> resolution because it can still disturb security related cookies.
>>=20
>> Possible resolutions:
>>=20
>> - 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)
>>=20
>> Both resolution requires BC, but this is better to be fixed ASAP.
>>=20
>> Non-BC resolution could be:
>>  - "Ask users to use setcookie() always for 'Set-Cookie'".
>>=20
>> 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.
>>=20
>> Problem with this may be that user cannot modify 'Set-Cookie' header
>> line as user want.
>>=20
>> $ php -r 'setcookie("REMEMBERME=3Dvalue; expires=3DSat, 03-Sep-2020
>> 05:38:43 GMT; path=3D/; domain=3Daaa");'
>> PHP Warning:  Cookie names cannot contain any of the following '=3D,;
>> \t\r\n\013\014' in Command line code on line 1
>>=20
>>=20
>> Comments?
>=20
> An idea for session ID protection.
>=20
> Following code results in lost session always.
>=20
> <?php
> session_start();
> session_regenerate_id();
> header('Set-Cookie: something');
> ?>
>=20
> 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.
>=20
>=20
> 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
>=20
> To do that, we can search string like following
> Set-Cookie: PHPSESSID=3Dxxxxxxx
>=20
> 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.
>=20
> Any comments?
>=20
> Regards,
>=20
> --
> Yasuo Ohgaki
> yohgaki@ohgaki.net
>=20
> --=20
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php