Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119277
MIME-Version: 1.0
References: <CAFPFaM+d_1pV__2u+rkTEOF8mPM_CSXRk3udmG-+G1oVuKbQmg@mail.gmail.com>
 <CAOWwgpnJrHfx83wkd35J_9HwqzkB-1iRfTyqYbZ=7HB9nUhrhQ@mail.gmail.com>
In-Reply-To: <CAOWwgpnJrHfx83wkd35J_9HwqzkB-1iRfTyqYbZ=7HB9nUhrhQ@mail.gmail.com>
Date: Mon, 16 Jan 2023 13:39:29 +0000
Message-ID: <CAFPFaMKTaQDSJXFfos=3sdL7ddSrje_aHWcg67GA9jXPe8uhew@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Cc: Christian Schneider <cschneid@cschneid.com>, Nicolas Grekas <nicolas.grekas+php@gmail.com>
Content-Type: multipart/alternative; boundary="0000000000001576d105f261b779"
Subject: Re: [PHP-DEV] [RFC] Add SameSite cookie attribute parameter
From: george.banyard@gmail.com ("G. P. B.")

--0000000000001576d105f261b779
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sun, 15 Jan 2023 at 16:40, Nicolas Grekas <nicolas.grekas+php@gmail.com>
wrote:

> Hi George,
>
> There's quite some activity on the HTTP cookies side.
> I read about SameParty and Partitioned attributes recently, see:
> - https://developer.chrome.com/docs/privacy-sandbox/chips/
> - https://github.com/cfredric/sameparty
>
> Maybe we should have a plan that works for these too?
>
> Nicolas
>

Considering those cookie attributes are still in an experimental design
phase and are not universally agreed upon by user agents, I wouldn't want
to add a specific parameter for them.
However, a more generic solution by overloading the options array to
accepts arbitrary key:value pairs might be something to pursue, but this is
out of scope for this RFC as just adding this to the current option array
forces people to use that signature which is less type safe.


On Sun, 15 Jan 2023 at 20:58, Christian Schneider <cschneid@cschneid.com>
wrote:

> Some comments:
> - I am not convinced that we should introduce a third way of providing
> parameters to setcookie(). I don't think this function is used often enou=
gh
> in common code to add yet another iteration of the API. Assuming there is=
 1
> to 2 places in your framework using this I don't think many bugs will go
> unnoticed. Adding a warning to illegal 'samesite' values in $options woul=
d
> IMHO be enough if stricter checking is wished for.
>

How is this providing a third way of providing parameters to setcookie()?
As it is, if I want to use named arguments and the typed parameters, I
cannot set the SameSite attribute.
I've heard multiple people waste time due to a SameSite bug because they
made a typo, and it took them way too long to figure out the issue as they
thought they had a server configuration problem.
Adding a warning for invalid values is effectively turning that option into
an Enum, which at this point one might as well have a proper Enum.


> - I don't like the camelCase of $sameSite as this is different from all
> the other parameters, e.g. $expires_or_options (yes, this is a
> pseudo-parameter name, I know) and $httponly. Looking at a couple of
> functions in the standard PHP set I didn't see any $camelCase.
>

ACK, it should probably be in snake case and be $same_site


> - A more generic question: How are Enums handled concerning future
> additions of values vs. BC compatibility? What is the migration plan ther=
e
> if one wants to support both old and new PHP versions?
>

The parameter is optional, so I don't understand what migration plan you
need?
If in the, unlikely, event that a new value is added this should be added
as a case in the enum in the next minor version of PHP.
In the, again unlikely, event that a value is deprecated, the corresponding
enum case should also be deprecated following the normal PHP deprecation
process.

I only decided to make this an enum because I deem it very unlikely for a
new valid attribute value to be introduced, the new IETF RFC clarifies and
amends the behaviour of the 3 valid attribute values that have always been
the same since 2016.


> Regards,
> - Chris


On Sun, 15 Jan 2023 at 21:08, Claude Pache <claude.pache@gmail.com> wrote:

> Hi,
>
> Some technical remarks:
> * The new parameter name should be `$samesite` (all lowercase), in order
> to match with the casing of the corresponding key in `$options`.
>

I don't agree with this, the name $samesite all lowercase is far from
optimal, however it should be in snake case ($same_site) in accordance with
the other parameters.


> * I think that you should add the case `SameSite::Omit` (which is the
> current default). This is not only for BC, but also for FC if, for some
> reason, `SameSite: Lax` is replaced by some attribute that supersedes it.
> Or if `SameSite: Lax` becomes the default, and therefore redundant. =E2=
=80=94
> Having `SameSite::Omit` instead of `null` would mean that it would be
> difficult to miss it by accident.
>

Same idea as in my reply to Chris, it is extremely unlikely that a new
attribute value will be added, moreover the point of using SameSite::Lax as
the default is that it will become the default (it already is in Chrome,
and is gated between a flag in Firefox) as currently in any case you *must*
provide a SameSite values for your cookies as the behaviour end-users are
going to have depends on their browser (e.g. Chrome defaulting to Lax,
while Firefox and Safari still default to None currently) and thus omitting
the value is, IMHO, a bug waiting to happen.


> That said, I am much more interested in being able to add custom cookie
> attributes. Back when SameSite was introduced (on the web, not in PHP), I
> recall that I had to use some hack in order to include them in my session
> cookie (before upgrading to PHP 7.3). The new cookie attributes mentioned
> by Nicolas in the other mail are probably too experimental in order to
> support them officially, but it could be interesting to be able to includ=
e
> them nonetheless, e.g. using some `customAttributes` parameter.
>

As said to Nicolas, this is definitely interesting, but out of scope for
this RFC.


> =E2=80=94Claude


Best regards,

George P. Banyard

--0000000000001576d105f261b779--