Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:78559
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain basereality.com from 74.125.82.52 cause and error)
MIME-Version: 1.0
In-Reply-To: <CABRjmp8rdVmCPCA5xdfgUirCKg=kF3NHXYK0xmPgOxS+4sKc9w@mail.gmail.com>
References: <CABRjmp_2DzhrGGoknZvSZ6fM=okjz+Od+2t0TVrNarc2fRLWiA@mail.gmail.com>
	<CAHMUw2HVmJD9syo8Rbc1=bA5S3U9umS0_XHFzwzWsY9Ee7UneQ@mail.gmail.com>
	<CAL+t6cEfbd7KbB=EbhdqgLKG6tBURgj+Ls9Y62tjOHL1Y+r-SA@mail.gmail.com>
	<CAH-PCH7tDQCtmAbPNOF4ONkeSQz5wr89XJ81a0USmzCqKi=26g@mail.gmail.com>
	<CAGAGxbYuimpj+phbNXn_3e_17vpGEkes7Pn5jCh9j27uZoOT8Q@mail.gmail.com>
	<CABRjmp8rdVmCPCA5xdfgUirCKg=kF3NHXYK0xmPgOxS+4sKc9w@mail.gmail.com>
Date: Sun, 2 Nov 2014 13:43:59 +0000
Message-ID: <CA+kxMuQR8_Bnugg4O3WqUHDMTMNrTLKsxCoJgqWgmCQOAmDgpQ@mail.gmail.com>
To: Florian Margaine <florian@margaine.com>
Cc: Chris Wright <cw@daverandom.com>, Ferenc Kovacs <tyra3l@gmail.com>, 
	Sherif Ramadan <theanomaly.is@gmail.com>, Tjerk Meesters <tjerk.meesters@gmail.com>, 
	PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] setcookie() minor BC break - fixes issue #67736
From: danack@basereality.com (Dan Ackroyd)

Florian wrote:
> On the PR <https://github.com/php/php-src/pull/795/> of the setcookie pat=
ch
> 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 s=
ame 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 =3D 'bar1';
$fooCookie =3D '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 <florian@margaine.com> wrote:
> Hi list,
>
> On the PR <https://github.com/php/php-src/pull/795/> of the setcookie pat=
ch
> 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 cooki=
es
> (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 bre=
ak.
> - 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 <cw@daverandom.com> wrote:
>
>> On 8 September 2014 09:09, Ferenc Kovacs <tyra3l@gmail.com> wrote:
>> > On Mon, Sep 8, 2014 at 9:15 AM, Sherif Ramadan <theanomaly.is@gmail.co=
m>
>> > 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 s=
ame
>> >> 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 discoura=
ges
>> > 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 =3D true
>> option
>> > to the function signature, but setcookie already has 7 arguments, so
>> > probably isn't a great idea.
>>
>> --
>> > Ferenc Kov=C3=A1cs
>> > @Tyr43l - http://tyrael.hu
>>
>
> Regards,
>
> --
> Florian Margaine