Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119280
Content-Type: text/plain;
	charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.300.101.1.3\))
Date: Mon, 16 Jan 2023 16:36:37 +0100
References: <CAFPFaM+d_1pV__2u+rkTEOF8mPM_CSXRk3udmG-+G1oVuKbQmg@mail.gmail.com>
 <CAOWwgpnJrHfx83wkd35J_9HwqzkB-1iRfTyqYbZ=7HB9nUhrhQ@mail.gmail.com>
 <CAFPFaMKTaQDSJXFfos=3sdL7ddSrje_aHWcg67GA9jXPe8uhew@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
In-Reply-To: <CAFPFaMKTaQDSJXFfos=3sdL7ddSrje_aHWcg67GA9jXPe8uhew@mail.gmail.com>
Message-ID: <22D992B7-62B3-4A4F-934F-17D53058FD35@cschneid.com>
Subject: Re: [PHP-DEV] [RFC] Add SameSite cookie attribute parameter
From: cschneid@cschneid.com (Christian Schneider)

Am 16.01.2023 um 14:39 schrieb G. P. B. <george.banyard@gmail.com>:
> 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 =
enough 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 would IMHO be enough if stricter checking is wished for.
>=20
> 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.

Maybe third API was misleading, what I mean is that we had the (old) way =
of positional arguments for options which did not include something like =
samesite and the (new) way with the options array. So I would assume =
most code was migrated to $options.
Now you are suggesting of updating the old API with an additional =
positional argument which would mean changing the array-form back to =
positional.
Is your plan to deprecate the $options-API at some point in the future? =
Having two APIs offering the same functionality seems a bit confusing to =
me.

Alternatively the 'samesite' option in $options could accept =
SameSite::Lax. This would accomplish (almost) the same with a smaller =
change. Disclosure: I like arrays for options as they allow for array =
addition etc. Something similar can be done with ...$options, true :-)

Side-Note: Isn't SameSite a very generic name in the global namespace? =
I'm not sure what the PHP policy is here.

>   - 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.
>=20
> ACK, it should probably be in snake case and be $same_site

Looking at $httponly I would have expected $samesite.

>   - A more generic question: How are Enums handled concerning future =
additions of values vs. BC compatibility? What is the migration plan =
there if one wants to support both old and new PHP versions?
>=20
> 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.
>=20
> 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.

I was talking about extending Enums, not the parameter and not SameSite =
specifically.
Are there any Enums in core PHP APIs where new values could be added in =
the future and how is the plan for code supporting multiple PHP version =
there? This was just something which crossed my mind when thinking about =
APIs with Enums. This is not really related to this RFC so I understand =
if you want to ignore this part :-)

Regards,
- Chris