On Sun, 15 Jan 2023 at 16:40, Nicolas Grekas <nicolas.grekas+...@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 <cschn...@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. > 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 On Sun, 15 Jan 2023 at 21:08, Claude Pache <claude.pa...@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. — > 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 include > them nonetheless, e.g. using some `customAttributes` parameter. > As said to Nicolas, this is definitely interesting, but out of scope for this RFC. > —Claude Best regards, George P. Banyard