Hello Internals,
I would like to start the discussion about the Add SameSite cookie
attribute parameter RFC:
https://wiki.php.net/rfc/same-site-parameter
This proposes to add an optional same site parameter to the setrawcooki(),
setcookie() and session_set_cookie_params() that takes a a value a new
SameSite enum:
enum SameSite {
case None;
case Lax;
case Strict;}
Best regards,
George P. Banyard
Hi George,
Hello Internals,
I would like to start the discussion about the Add SameSite cookie
attribute parameter RFC:
https://wiki.php.net/rfc/same-site-parameterThis proposes to add an optional same site parameter to the setrawcooki(),
setcookie()andsession_set_cookie_params()that takes a a value a new
SameSite enum:enum SameSite {
case None;
case Lax;
case Strict;}
There's quite some activity on the HTTP cookies side.
I read about SameParty and Partitioned attributes recently, see:
Maybe we should have a plan that works for these too?
Nicolas
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: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 tosetcookie(). 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.
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 there
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
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: Laxis replaced by some attribute that supersedes it.
Or ifSameSite: Laxbecomes the default, and therefore redundant. —
HavingSameSite::Omitinstead ofnullwould 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 include
them nonetheless, e.g. using somecustomAttributesparameter.
As said to Nicolas, this is definitely interesting, but out of scope for
this RFC.
—Claude
Best regards,
George P. Banyard
Am 16.01.2023 um 14:39 schrieb G. P. B. george.banyard@gmail.com:
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.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.
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?
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.
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
On Mon, 16 Jan 2023 at 15:36, Christian Schneider cschneid@cschneid.com
wrote:
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 tosetcookie(). 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.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.
I personally don't plan on deprecating the array $options API. If two APIs
that provide the same functionality are too confusing, then the current
incomplete way of passing parameters should be deprecated at some point.
However, until then, having symmetry between both APIs is better for people
who prefer to have parameters and use named arguments than the $options
array.
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 :-)
That is actually a good idea to add support for the array version to accept
the new enum, I'll amend the implementation.
Side-Note: Isn't SameSite a very generic name in the global namespace? I'm
not sure what the PHP policy is here.
AFAIK the global namespace is "owned" by PHP so that shouldn't be an issue
per the usual policy.
- 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
Looking at $httponly I would have expected $samesite.
I suppose it can be either or, but trying to improve the name of a not yet
existing parameter seems better than following the weird naming scheme. But
I'm not hard set on the name here.
- 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?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.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 :-)
I might be again misunderstanding, but one cannot extend an enum as they
are final classes under the hood.
Currently, the only other native enum is the one that was added with the
Randomizer Additions RFC [1] so this topic hasn't come up yet as the enum
for ext-random is definetely complete.
Best,
George P. Banyard
Am 17.01.2023 um 15:59 schrieb G. P. B. george.banyard@gmail.com:
Side-Note: Isn't SameSite a very generic name in the global namespace? I'm not sure what the PHP policy is here.
AFAIK the global namespace is "owned" by PHP so that shouldn't be an issue per the usual policy.
In a quick check I could not find any other Enums defined in the core. Would this be the first one?
I guess I was comparing it to constants where in most cases the constant name reflects where it is used, e.g. STR_PAD_LEFT and I was therefore wondering whether the name SameSite should somehow contain cookie in one form or another. Or is this frowned upon for Enums?
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 :-)
I might be again misunderstanding, but one cannot extend an enum as they are final classes under the hood.
Currently, the only other native enum is the one that was added with the Randomizer Additions RFC [1] so this topic hasn't come up yet as the enum for ext-random is definetely complete.
I'm talking about adding new values in later PHP versions, let's for example assume they would add a SameSite mode "Stricter" and PHP wants to support that.
How would one write code to use "Stricter" in code meant to work for both old and new PHP versions?
- Chris
Am 17.01.2023 um 15:59 schrieb G. P. B. george.banyard@gmail.com:
I might be again misunderstanding, but one cannot extend an enum as
they are final classes under the hood. Currently, the only other
native enum is the one that was added with the Randomizer Additions
RFC [1] so this topic hasn't come up yet as the enum for ext-random
is definetely complete.I'm talking about adding new values in later PHP versions, let's for
example assume they would add a SameSite mode "Stricter" and PHP wants
to support that. How would one write code to use "Stricter" in code
meant to work for both old and new PHP versions?
if (version_compare(phpversion(), "8.4.0", ">")) {
setcookie("test", "value", samesite: SameSite::Stricter);
} else {
setcookie("test", "value", samesite: SameSite::Strict);
}
These are run time checks, not compile time:
8 > JMPZ $1, ->18
...
13 FETCH_CLASS_CONSTANT ~2 'SameSite', 'Stricter'
...
17 > JMP ->26
18 > EXT_STMT
...
22 FETCH_CLASS_CONSTANT ~4 'SameSite', 'Strict'
cheers,
Derick
Le 18 janv. 2023 à 16:20, Derick Rethans derick@php.net a écrit :
if (version_compare(phpversion(), "8.4.0", ">")) {
setcookie("test", "value", samesite: SameSite::Stricter);
} else {
setcookie("test", "value", samesite: SameSite::Strict);
}
Or even, replace version_compare(...) with SameSite::tryFrom(...) !== null:
setcookie("test", "value", samesite: SameSite::tryFrom('Stricter') ?? SameSite::Strict);
—Claude
Am 18.01.2023 um 16:26 schrieb Claude Pache claude.pache@gmail.com:
Le 18 janv. 2023 à 16:20, Derick Rethans derick@php.net a écrit :
if (version_compare(phpversion(), "8.4.0", ">")) {
setcookie("test", "value", samesite: SameSite::Stricter);
} else {
setcookie("test", "value", samesite: SameSite::Strict);
}Or even, replace
version_compare(...)withSameSite::tryFrom(...) !== null:
setcookie("test", "value", samesite: SameSite::tryFrom('Stricter') ?? SameSite::Strict);
Thanks for your replies, I like the second option as it is a feature instead of a version check.
Now my only itch is that the support for SameSite=Stricter is actually depending on the browser, not the server so assuming all browser are already supporting this new mode I should not send a less strict mode just because I'm using an old PHP version. This is currently possible since setcookie() does not validate the content of the samesite options.
But as this is somewhat of a special case (most function options do not depend on something external) and you seem to be confident that the list of SameSite options will not change any time soon I'll shut up now :-)
Regards,
- Chris
Hi
Side-Note: Isn't SameSite a very generic name in the global namespace? I'm
not sure what the PHP policy is here.AFAIK the global namespace is "owned" by PHP so that shouldn't be an issue
per the usual policy.
It's still pretty generic even if it does not collide with any userland
code. It would not be unlikely to have something named "SameSite" in the
future that is not related to cookies, e.g. for some other HTTP-related
thing.
Best regards
Tim Düsterhus
Am 14.01.2023 um 16:14 schrieb G. P. B. george.banyard@gmail.com:
I would like to start the discussion about the Add SameSite cookie
attribute parameter RFC:
https://wiki.php.net/rfc/same-site-parameterThis proposes to add an optional same site parameter to the setrawcooki(),
setcookie()andsession_set_cookie_params()that takes a a value a new
SameSite enum:enum SameSite {
case None;
case Lax;
case Strict;}
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. - 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.
- 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?
Regards,
- Chris
Le 14 janv. 2023 à 16:14, G. P. B. george.banyard@gmail.com a écrit :
Hello Internals,
I would like to start the discussion about the Add SameSite cookie
attribute parameter RFC:
https://wiki.php.net/rfc/same-site-parameterThis proposes to add an optional same site parameter to the setrawcooki(),
setcookie()andsession_set_cookie_params()that takes a a value a new
SameSite enum:enum SameSite {
case None;
case Lax;
case Strict;}Best regards,
George P. Banyard
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 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: Laxis replaced by some attribute that supersedes it. Or ifSameSite: Laxbecomes the default, and therefore redundant. — HavingSameSite::Omitinstead ofnullwould mean that it would be difficult to miss it by accident.
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 include them nonetheless, e.g. using some customAttributes parameter.
—Claude