LS,
Just now, I opened the RFC on implementing same site cookies in PHP,
https://wiki.php.net/rfc/same-site-cookie, for voting.
It consists of two questions, depending on the implementation you would
like to see of the feature. Both questions will affect the API of four
core functions: setcookie, setrawcookie, session_set_cookie_params and
session_get_cookie_params. The first three functions have a similar
function signature. The first implementation suggestion is to add an
additional argument to these three functions. The second implementation
suggestion is to allow an array of options in which all the cookie
options will be moved into. More details are to be found in the RFC.
Hopefully, the samesite cookie flag will become a feature of the PHP
language through this RFC!
Kind regards,
Frederik Bosch
LS,
Just now, I opened the RFC on implementing same site cookies in PHP,
https://wiki.php.net/rfc/same-site-cookie, for voting.
Please be explicit:
Proposed PHP Version(s)
next PHP 7.x
It's really late in the day for 7.2. Although people might still vote
for it, the RFC needs to be explicit about which version it is for.
cheers
Dan
Hi Dan,
While I agree on your statement that it is late for 7.2, I believe the
text is explicit enough. Since features for PHP 7.2 are frozen,
according to the rules this should go for the version thereafter.
However, if a release managers wants to pick up it and embed in 7.2, I
am not going to protest. Things considered, I see no reason to change
the sentence.
Best,
Frederik
LS,
Just now, I opened the RFC on implementing same site cookies in PHP,
https://wiki.php.net/rfc/same-site-cookie, for voting.
Please be explicit:Proposed PHP Version(s)
next PHP 7.xIt's really late in the day for 7.2. Although people might still vote
for it, the RFC needs to be explicit about which version it is for.cheers
Dan
LS,
Just now, I opened the RFC on implementing same site cookies in PHP,
https://wiki.php.net/rfc/same-site-cookie, for voting.Please be explicit:
Proposed PHP Version(s)
next PHP 7.xIt's really late in the day for 7.2. Although people might still vote
for it, the RFC needs to be explicit about which version it is for.
In my opinion it's too late for 7.2 especially as it contains an ABI
break which at best will be annoying for the folks helping us test.
The primary vote should be about 7.3 and if this wants to land on 7.2
there should be a separate vote for that.
-Sara
Le 26/08/2017 à 01:10, Sara Golemon a écrit :
In my opinion it's too late for 7.2 especially as it contains an ABI
break which at best will be annoying for the folks helping us test.
The primary vote should be about 7.3 and if this wants to land on 7.2
there should be a separate vote for that.
+1
BTW, choice 1 seems awfull but preserve BC
Perhaps choice 2 could be implemented in a BC way, allowing both proto
Ex for session_set_cookie_params
Parse arg 2 as an optional zval
if arg2 is an array
if argc > 2 error
else new proto, array of options
else old proto
Remi.
Hi Sara,
Thanks for clearing this. I have no intension to have it merged in 7.2
so I updated the RFC to specifically mention it is for 7.3. If other
people want to have it in 7.2, they can start a new RFC to make that happen.
Best,
Frederik
LS,
Just now, I opened the RFC on implementing same site cookies in PHP,
https://wiki.php.net/rfc/same-site-cookie, for voting.
Please be explicit:Proposed PHP Version(s)
next PHP 7.xIt's really late in the day for 7.2. Although people might still vote
for it, the RFC needs to be explicit about which version it is for.In my opinion it's too late for 7.2 especially as it contains an ABI
break which at best will be annoying for the folks helping us test.
The primary vote should be about 7.3 and if this wants to land on 7.2
there should be a separate vote for that.-Sara
Hi Sara, hi Frederik,
Sounds good! Let's vote in getting it in first and then we can have a 2nd RFC (and vote) if it should land in 7.2
cu,
Lars
Sent from my electronic toy
Hi Sara,
Thanks for clearing this. I have no intension to have it merged in 7.2 so I updated the RFC to specifically mention it is for 7.3. If other people want to have it in 7.2, they can start a new RFC to make that happen.
Best,
FrederikLS,
Just now, I opened the RFC on implementing same site cookies in PHP,
https://wiki.php.net/rfc/same-site-cookie, for voting.
Please be explicit:Proposed PHP Version(s)
next PHP 7.xIt's really late in the day for 7.2. Although people might still vote
for it, the RFC needs to be explicit about which version it is for.In my opinion it's too late for 7.2 especially as it contains an ABI
break which at best will be annoying for the folks helping us test.
The primary vote should be about 7.3 and if this wants to land on 7.2
there should be a separate vote for that.-Sara
Sounds good! Let's vote in getting it in first and then we can have a 2nd RFC (and vote) if it should land in 7.2
Mmmm, not quite. IF you want to aim for 7.2, do it now in the same
vote. Back porting is sub-optimal and there's not a rush to land it
on 7.3. The time sensitive part is 7.2.
FTR, I'll be voting "No" on a 7.2, and I've already submitted my yes
vote for 7.3 (options array variant).
-Sara
Hi,
Sounds good! Let's vote in getting it in first and then we can have a 2nd RFC (and vote) if it should land in 7.2
Mmmm, not quite. IF you want to aim for 7.2, do it now in the same
vote. Back porting is sub-optimal and there's not a rush to land it
on 7.3. The time sensitive part is 7.2.
I second this.
In fact, there was a competing idea/RFC when the discussion started,
but the author of this one insisted that there's no time to wait for
that. To me, it doesn't make sense to rush this (compared to
alternative ideas) if it doesn't get into 7.2.
Cheers,
Andrey.
Hi Andrey,
While I agree on your statement that back-porting is suboptimal, I do
not agree on the fact that I said that there was no time to wait. I
submitted the RFC, awaited the opinions, changed the document according
to the different viewpoints and I link to the other RFC from this RFC. I
do not want to 'push' this through. I think we know the opinions, so it
is time to vote. If both suggestions from this RFC don't make, then
people can go for other solutions.
Best,
Frederik
Hi,
Sounds good! Let's vote in getting it in first and then we can have a 2nd RFC (and vote) if it should land in 7.2
Mmmm, not quite. IF you want to aim for 7.2, do it now in the same
vote. Back porting is sub-optimal and there's not a rush to land it
on 7.3. The time sensitive part is 7.2.I second this.
In fact, there was a competing idea/RFC when the discussion started,
but the author of this one insisted that there's no time to wait for
that. To me, it doesn't make sense to rush this (compared to
alternative ideas) if it doesn't get into 7.2.Cheers,
Andrey.
Hi Frederik,
On Mon, Aug 28, 2017 at 6:34 PM, Frederik Bosch | Genkgo
f.bosch@genkgo.nl wrote:
Hi Andrey,
While I agree on your statement that back-porting is suboptimal, I do not
agree on the fact that I said that there was no time to wait. I submitted
the RFC, awaited the opinions, changed the document according to the
different viewpoints and I link to the other RFC from this RFC. I do not
want to 'push' this through. I think we know the opinions, so it is time to
vote. If both suggestions from this RFC don't make, then people can go for
other solutions.
You did wait for, and adjust according to suggestions, I'm not questioning that.
I was referring to this message: https://externals.io/message/99884#99893
If you want this to land in 7.2 (i.e. not "take a while before we see
samesite cookie implemented"), then there really is no time to wait.
I'm not being negative here ... I'm using this as an argument to make
the vote for 7.2 now.
Otherwise, we have an entire year to polish all the details in time for 7.3.
Cheers,
Andrey.
Hi Andrey,
Little misunderstanding then. I agree we can better have this PHP 7.3
and take some time for it. Current votes also suggest that we should go
for the array argument implementation. Since there is only a PR for the
extra argument implementation, it will also take time to have the PR for
the array argument implementation ready. Taken that into account, we
should not want this in 7.2.
Best,
Frederik
Hi Frederik,
On Mon, Aug 28, 2017 at 6:34 PM, Frederik Bosch | Genkgo
f.bosch@genkgo.nl wrote:Hi Andrey,
While I agree on your statement that back-porting is suboptimal, I do not
agree on the fact that I said that there was no time to wait. I submitted
the RFC, awaited the opinions, changed the document according to the
different viewpoints and I link to the other RFC from this RFC. I do not
want to 'push' this through. I think we know the opinions, so it is time to
vote. If both suggestions from this RFC don't make, then people can go for
other solutions.You did wait for, and adjust according to suggestions, I'm not questioning that.
I was referring to this message: https://externals.io/message/99884#99893
If you want this to land in 7.2 (i.e. not "take a while before we see
samesite cookie implemented"), then there really is no time to wait.
I'm not being negative here ... I'm using this as an argument to make
the vote for 7.2 now.
Otherwise, we have an entire year to polish all the details in time for 7.3.Cheers,
Andrey.
--
Frederik Bosch
Partner
Genkgo logo
Mail: f.bosch@genkgo.nl mailto:f.bosch@genkgo.nl
Web: support.genkgo.com https://support.genkgo.com
Entrada 123
Amsterdam
+31 208 943 931
Genkgo B.V. staat geregistreerd bij de Kamer van Koophandel onder nummer
56501153
On Mon, Aug 28, 2017 at 12:10 PM, Frederik Bosch | Genkgo <f.bosch@genkgo.nl
wrote:
Little misunderstanding then. I agree we can better have this PHP 7.3 and
take some time for it. Current votes also suggest that we should go for the
array argument implementation. Since there is only a PR for the extra
argument implementation, it will also take time to have the PR for the
array argument implementation ready. Taken that into account, we should not
want this in 7.2.
Indeed, yes. Assuming the votes continue on this sharp lean towards the
array option, we should just forget all notions of trying to sneak this
into 7.2.
Direct calls in 7.2 and earlier can easily fall back on calling
header('Set-Cookie: ...'); manually, while sessions support is slightly
more complex, but still doable from userspace. I expect if need is deemed
high for this, a drop-in composer package can do 90% of the work
automatically.
-Sara
Hi Sara, hi Frederik,
Thinking more about this I came to change my vote (and for that reason I’ll take back the suggestion to include it into 7.2):
The array API is the better API and allows for healthier future growth so we should pursue that option
There is a (very ugly) workaround to set a same site policy by misusing the “session.cookie_path” or “session.cookie_domain” setting (e.g. set it to “/; SameSite=Strict”, you are welcome, Internet).
cu,
Lars
Little misunderstanding then. I agree we can better have this PHP 7.3 and take some time for it. Current votes also suggest that we should go for the array argument implementation. Since there is only a PR for the extra argument implementation, it will also take time to have the PR for the array argument implementation ready. Taken that into account, we should not want this in 7.2.
Indeed, yes. Assuming the votes continue on this sharp lean towards the array option, we should just forget all notions of trying to sneak this into 7.2.
Direct calls in 7.2 and earlier can easily fall back on calling header('Set-Cookie: ...'); manually, while sessions support is slightly more complex, but still doable from userspace. I expect if need is deemed high for this, a drop-in composer package can do 90% of the work automatically.
-Sara
Hi,
Frederik Bosch wrote:
LS,
Just now, I opened the RFC on implementing same site cookies in PHP,
https://wiki.php.net/rfc/same-site-cookie, for voting.
Correct me if I'm wrong, but wasn't the RFC only put to internals a week
ago? That's not a long enough discussion period before opening voting,
https://wiki.php.net/rfc/howto says it should be at least 2 weeks.
Regards.
Andrea Faulds
https://ajf.me/
Hi!
additional argument to these three functions. The second implementation
suggestion is to allow an array of options in which all the cookie
options will be moved into. More details are to be found in the RFC.
Something not clear to me on the second one - why lifetime/expiration is
a separate parameter while all others are part of $options?
--
Stas Malyshev
smalyshev@gmail.com
Hi Stanislav,
My reasoning for this is as follows.
-
The session_set_cookie_params function requires a lifetime parameter
at the moment. -
To enforce that lifetime stays required I did not want to make it
required within the optional array. That would make that optional array
not optional anymore, and even have a required key. I don't think that
is a good idea. -
To prevent that the array of options is different between the three
functions (session_set_cookie_params, setcookie, setrawcookie), I chose
to exclude lifetime from the array of options and include it in the list
of arguments.
Hence, I chose a consistent and logical API over the three functions
together rather than having logical ones per function. Hope it makes sense.
Best,
Frederik
Hi!
additional argument to these three functions. The second implementation
suggestion is to allow an array of options in which all the cookie
options will be moved into. More details are to be found in the RFC.
Something not clear to me on the second one - why lifetime/expiration is
a separate parameter while all others are part of $options?
Something not clear to me on the second one - why lifetime/expiration is
a separate parameter while all others are part of $options?
The session_set_cookie_params function requires a lifetime parameter
at the moment.To enforce that lifetime stays required I did not want to make it
required within the optional array. That would make that optional array
not optional anymore, and even have a required key. I don't think that
is a good idea.To prevent that the array of options is different between the three
functions (session_set_cookie_params, setcookie, setrawcookie), I chose
to exclude lifetime from the array of options and include it in the list
of arguments.Hence, I chose a consistent and logical API over the three functions
together rather than having logical ones per function. Hope it makes sense.
Hi all,
This reasoning doesn't make a lot of sense to me.
With regards to session_set_cookie_params, I believe that the only reason
why lifetime is required in that function is because calling it without
parameters doesn't have a purpose.
With an array of options (which keeps the first parameter mandatory), there
is no strong reason to make one of them required. An empty array or an
array without any valid key would still error.
If this is acceptable for that function, all the other functions can have
all the options in the array as that keeps the array consistent across them.
With this being said, would anyone oppose an implementation where all the
options (including lifetime) are included in the array parameter?
About the implementation, the array of options is never really defined in
the RFC. If we assume that it is identical to the proposed output
of session_get_cookie_params, the name of the first key should be lifetime.
However, for setcookie and setrawcookie, the documented parameter name is
expire. Would there be any issue if we would assume lifetime for the 4
functions?
Regards,
Pedro
Something not clear to me on the second one - why lifetime/expiration is
a separate parameter while all others are part of $options?
The session_set_cookie_params function requires a lifetime parameter
at the moment.To enforce that lifetime stays required I did not want to make it
required within the optional array. That would make that optional array
not optional anymore, and even have a required key. I don't think that
is a good idea.To prevent that the array of options is different between the three
functions (session_set_cookie_params, setcookie, setrawcookie), I chose
to exclude lifetime from the array of options and include it in the list
of arguments.Hence, I chose a consistent and logical API over the three functions
together rather than having logical ones per function. Hope it makes sense.This reasoning doesn't make a lot of sense to me.
With regards to session_set_cookie_params, I believe that the only reason
why lifetime is required in that function is because calling it without
parameters doesn't have a purpose.
With an array of options (which keeps the first parameter mandatory), there
is no strong reason to make one of them required. An empty array or an
array without any valid key would still error.
If this is acceptable for that function, all the other functions can have
all the options in the array as that keeps the array consistent across them.With this being said, would anyone oppose an implementation where all the
options (including lifetime) are included in the array parameter?
Personally, I'd even prefer this, but that's not what was voted upon, so
I'm not sure if it's okay. Anyhow, the implementation is available as
https://github.com/php/php-src/pull/3398. Thanks, Pedro!
About the implementation, the array of options is never really defined in
the RFC. If we assume that it is identical to the proposed output
of session_get_cookie_params, the name of the first key should be lifetime.
However, for setcookie and setrawcookie, the documented parameter name is
expire. Would there be any issue if we would assume lifetime for the 4
functions?
I'd prefer “expire_s_”, but I'm okay with “lifetime”.
--
Christoph M. Becker
On Sat, Jul 21, 2018 at 12:11 PM Christoph M. Becker cmbecker69@gmx.de
wrote:
Personally, I'd even prefer this, but that's not what was voted upon, so
I'm not sure if it's okay. Anyhow, the implementation is available as
https://github.com/php/php-src/pull/3398. Thanks, Pedro!
I personally believe that the intent of the vote is to have this feature
and to have it included as an alternative array of parameters. I don't mean
to step on anyone's toes with the proposed implementation, but given that
the RFC doesn't define the array of options I think most people didn't
notice this design choice and/or ultimately just wanted this feature
accepted.
About the implementation, the array of options is never really defined in
the RFC. If we assume that it is identical to the proposed output
of session_get_cookie_params, the name of the first key should be
lifetime.
However, for setcookie and setrawcookie, the documented parameter name is
expire. Would there be any issue if we would assume lifetime for the 4
functions?I'd prefer “expire_s_”, but I'm okay with “lifetime”.
Although I had suggested using the same key for all functions, when I
started the implementation I realized they have different meanings. For
session_get_cookie_params, lifetime is a relative amount of seconds, but
for setcookie and setrawcookie expire is an absolute timestamp. So I kept
that distinction.
About your suggestion, I looked again, and in fact, the only place where
the parameter is called expire is in the docs. Internally (ie. Reflection)
it is called expires. Given this and the fact that it is what actually is
used in the cookie header, I've changed the key to "expires".
Regards,
Pedro
Hi,
With this being said, would anyone oppose an implementation where all the
options (including lifetime) are included in the array parameter?
Yes.
All other "options" are actual cookie attribute names, as defined by
the various IETF RFCs, while "lifetime" is just a convenient name used
by PHP. It doesn't correspond to a particular attribute, but instead
the values for the Expires and Max-Age attributes are derived from it.
I believe during discussion I insisted that the parameter be called
"attributes", for this very reason.
On another note, I also wanted that pretty much any key/value pair to
be accepted instead of raising an error, for forward compatibility.
Unfortunately, neither of these 2 points I raised were addressed, but
it also looked like the author abandoned the RFC mid-vote as he agreed
that targeting 7.2 at the time wasn't a good idea, only he didn't
actually close the vote. So ... I'm not even sure of the status of
this RFC?
Cheers,
Andrey.
Yes.
All other "options" are actual cookie attribute names, as defined by
the various IETF RFCs, while "lifetime" is just a convenient name used
by PHP. It doesn't correspond to a particular attribute, but instead
the values for the Expires and Max-Age attributes are derived from it.
I believe during discussion I insisted that the parameter be called
"attributes", for this very reason.
Hi,
While I do understand your reasoning, I find it extremely unfriendly to the
user of the function to ask for one parameter separate from all the others
for that reason alone.
Also, keep in mind that all this function does is set the
session.cookie_*
ini entries. So all parameters are treated equally.
On another note, I also wanted that pretty much any key/value pair to
be accepted instead of raising an error, for forward compatibility.
I really believe that the user spotting errors like ['expries' =>
time()+ 3600]
faster is more valuable than FC.
Regards,
Pedro
Yes.
All other "options" are actual cookie attribute names, as defined by
the various IETF RFCs, while "lifetime" is just a convenient name used
by PHP. It doesn't correspond to a particular attribute, but instead
the values for the Expires and Max-Age attributes are derived from it.
I believe during discussion I insisted that the parameter be called
"attributes", for this very reason.Hi,
While I do understand your reasoning, I find it extremely unfriendly to the
user of the function to ask for one parameter separate from all the others
for that reason alone.
Also, keep in mind that all this function does is set the
session.cookie_*
ini entries. So all parameters are treated equally.
To add to this, session_get_cookie_params()
already returns all parameters
including "lifetime" in one array. It would be very weird if there was an
asymmetry between session_get_cookie_params()
and
session_set_cookie_params()
.
Furthermore, the way I have seen session_set_cookie_params()
used, it has
pretty much always been in conjunction with a prior
session_get_cookie_params()
call to get the current values and then only
one (or some) of them being adjusted. It makes more sense to write
$params = session_get_cookie_params()
;
// Change $params here
session_set_cookie_params($params);
than having to treat just the $params["lifetime"] case specially.
Nikita
On another note, I also wanted that pretty much any key/value pair to
be accepted instead of raising an error, for forward compatibility.
I really believe that the user spotting errors like
['expries' =>
time()+ 3600]
faster is more valuable than FC.Regards,
Pedro
Hi,
Yes.
All other "options" are actual cookie attribute names, as defined by
the various IETF RFCs, while "lifetime" is just a convenient name used
by PHP. It doesn't correspond to a particular attribute, but instead
the values for the Expires and Max-Age attributes are derived from it.
I believe during discussion I insisted that the parameter be called
"attributes", for this very reason.Hi,
While I do understand your reasoning, I find it extremely unfriendly to the
user of the function to ask for one parameter separate from all the others
for that reason alone.
Also, keep in mind that all this function does is set thesession.cookie_*
ini entries. So all parameters are treated equally.
Ok, I can see how it can be inconvenient for
session_set_cookie_params()
, though calling it "extremely" unfriendly
is some exaggeration IMO. But while I didn't quote that part of your
message, you did also suggest to apply the same decision to other
functions and so I am talking about all of them.
I'd be ok with this for session_set_cookie_params()
alone, but not for
set[raw]cookie().
On another note, I also wanted that pretty much any key/value pair to
be accepted instead of raising an error, for forward compatibility.I really believe that the user spotting errors like
['expries' =>
time()+ 3600]
faster is more valuable than FC.
Honestly, the fact that you chose "expires" for this particular
example IMO only makes a stronger case for why it needs to be
separated. :)
Cheers,
Andrey.
Am So., 22. Juli 2018 um 14:16 Uhr schrieb Andrey Andreev narf@devilix.net:
Hi,
Yes.
All other "options" are actual cookie attribute names, as defined by
the various IETF RFCs, while "lifetime" is just a convenient name used
by PHP. It doesn't correspond to a particular attribute, but instead
the values for the Expires and Max-Age attributes are derived from it.
I believe during discussion I insisted that the parameter be called
"attributes", for this very reason.Hi,
While I do understand your reasoning, I find it extremely unfriendly to the
user of the function to ask for one parameter separate from all the others
for that reason alone.
Also, keep in mind that all this function does is set thesession.cookie_*
ini entries. So all parameters are treated equally.Ok, I can see how it can be inconvenient for
session_set_cookie_params()
, though calling it "extremely" unfriendly
is some exaggeration IMO. But while I didn't quote that part of your
message, you did also suggest to apply the same decision to other
functions and so I am talking about all of them.I'd be ok with this for
session_set_cookie_params()
alone, but not for
set[raw]cookie().On another note, I also wanted that pretty much any key/value pair to
be accepted instead of raising an error, for forward compatibility.I really believe that the user spotting errors like
['expries' =>
time()+ 3600]
faster is more valuable than FC.Honestly, the fact that you chose "expires" for this particular
example IMO only makes a stronger case for why it needs to be
separated. :)
It'd be great to use an OO approach instead of "magic" array keys,
e.g. like this:
https://github.com/amphp/http/blob/9c0ba2f2ebfae482b3ad7a0475eb3d1f74d87949/src/Cookie/CookieAttributes.php
Regards, Niklas
It'd be great to use an OO approach instead of "magic" array keys,
e.g. like this:Regards, Niklas
Hi,
While I do agree with the sentiment:
- That would have been an even greater departure from the original RFC.
- This is currently a purely procedural API. If this were about an
hypotheticalResponseHeaders::setCookie
it would definitely be the way to
go.
Regards,
Pedro
Am So., 22. Juli 2018 um 18:11 Uhr schrieb Pedro Magalhães mail@pmmaga.net:
It'd be great to use an OO approach instead of "magic" array keys,
e.g. like this:Regards, Niklas
Hi,
While I do agree with the sentiment:
- That would have been an even greater departure from the original RFC.
- This is currently a purely procedural API. If this were about an
hypotheticalResponseHeaders::setCookie
it would definitely be the way to
go.Regards,
Pedro
Hey Pedro,
why does it have to be an all or nothing approach? It's perfectly fine
to have a function that accepts an object.
Regards, Niklas
Am So., 22. Juli 2018 um 18:11 Uhr schrieb Pedro Magalhães mail@pmmaga.net:
It'd be great to use an OO approach instead of "magic" array keys,
e.g. like this:While I do agree with the sentiment:
- That would have been an even greater departure from the original RFC.
- This is currently a purely procedural API. If this were about an
hypotheticalResponseHeaders::setCookie
it would definitely be the way to
go.why does it have to be an all or nothing approach? It's perfectly fine
to have a function that accepts an object.
We have an already accepted (29:3) RFC[1], which just lacks
implementation. Departing from the solution, which we agreed upon, in
some details (such as suggested by Pedro) might be okay, but using
objects instead of arrays is certainly not.
[1] https://wiki.php.net/rfc/same-site-cookie
--
Christoph M. Becker
Am So., 22. Juli 2018 um 18:52 Uhr schrieb Christoph M. Becker
cmbecker69@gmx.de:
Am So., 22. Juli 2018 um 18:11 Uhr schrieb Pedro Magalhães mail@pmmaga.net:
It'd be great to use an OO approach instead of "magic" array keys,
e.g. like this:While I do agree with the sentiment:
- That would have been an even greater departure from the original RFC.
- This is currently a purely procedural API. If this were about an
hypotheticalResponseHeaders::setCookie
it would definitely be the way to
go.why does it have to be an all or nothing approach? It's perfectly fine
to have a function that accepts an object.We have an already accepted (29:3) RFC[1], which just lacks
implementation. Departing from the solution, which we agreed upon, in
some details (such as suggested by Pedro) might be okay, but using
objects instead of arrays is certainly not.
We can always have a second RFC that changes a previous RFC. It'll
land in PHP 7.4 then, but that's not an issue for me.
Regards, Niklas
Am So., 22. Juli 2018 um 18:11 Uhr schrieb Pedro Magalhães <
mail@pmmaga.net>:It'd be great to use an OO approach instead of "magic" array keys,
e.g. like this:Regards, Niklas
Hi,
While I do agree with the sentiment:
- That would have been an even greater departure from the original RFC.
- This is currently a purely procedural API. If this were about an
hypotheticalResponseHeaders::setCookie
it would definitely be the way
to
go.Regards,
PedroHey Pedro,
why does it have to be an all or nothing approach? It's perfectly fine
to have a function that accepts an object.Regards, Niklas
While defining SessionCookieParams object and use it is ok, but
there is a thing to consider.
How it could be more useful than current procedural API. i.e. array vs
object params.
class SessionCookiePrams {
public $lifetime;
public $path;
// and so on
}
Users still can typo with this, so it may be
class SessionCookiePrams {
private $lifetime;
private $path;
// and so on
function setLifetime() {..}
function setPath() {..}
}
Defining such OO API is out of scope for this RFC.
It would be better let users to define such OO API wrapper for the time
being.
If we would like to add OO API for session, it would be better to have
session_oo. c or like and define OO APIs in it. It requires a new RFC for
this.
One thing regarding implementation.
Since the internet RFC has only 2 values for "samesite", the parameter can
be
bool rather than string so that users can avoid "broken security by a typo".
If "samesite" has more than 2 values, the INI handler can be changed so
that it can
handle both bool and string parameters.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi,
One thing regarding implementation.
Since the internet RFC has only 2 values for "samesite", the parameter can
be
bool rather than string so that users can avoid "broken security by a typo".
If "samesite" has more than 2 values, the INI handler can be changed so that
it can
handle both bool and string parameters.
The attribute has 2 possible values, but those are 2 different modes
of operation when enabled, not 2 states in total. It doesn't fit in
a boolean, and even if it did it wouldn't be forward-compatible that
way.
Cheers,
Andrey.
Hi,
One thing regarding implementation.
Since the internet RFC has only 2 values for "samesite", the parameter
can
be
bool rather than string so that users can avoid "broken security by a
typo".
If "samesite" has more than 2 values, the INI handler can be changed so
that
it can
handle both bool and string parameters.The attribute has 2 possible values, but those are 2 different modes
of operation when enabled, not 2 states in total. It doesn't fit in
a boolean, and even if it did it wouldn't be forward-compatible that
way.
What do you mean by "those are 2 different modes
of operation when enabled, not 2 states in total. "?
samesite-value = "Strict" / "Lax"
Flag is flag. It does not matter if it is used as combined values.
An INI value can be bool and string/etc. Even when 3rd value is added, it
can
be supported. Such INIs exist in PHP already.
Regards,
--
Yasuo Ohgaki
Hi,
One thing regarding implementation.
Since the internet RFC has only 2 values for "samesite", the parameter
can
be
bool rather than string so that users can avoid "broken security by a
typo".
If "samesite" has more than 2 values, the INI handler can be changed so
that
it can
handle both bool and string parameters.The attribute has 2 possible values, but those are 2 different modes
of operation when enabled, not 2 states in total. It doesn't fit in
a boolean, and even if it did it wouldn't be forward-compatible that
way.What do you mean by "those are 2 different modes
of operation when enabled, not 2 states in total. "?samesite-value = "Strict" / "Lax"
Flag is flag. It does not matter if it is used as combined values.
An INI value can be bool and string/etc. Even when 3rd value is added, it
can
be supported. Such INIs exist in PHP already.
A boolean makes sense for Secure and HTTPonly, where the flag either
exists or not. That's not what we have here, as SameSite=Lax is not
the same thing as not having SameSite at all.
bool(false) may make sense as an Off switch, yes, but that's not what
you suggested ...
Cheers,
Andrey.
Hi,
On Sun, Jul 29, 2018 at 7:22 AM, Yasuo Ohgaki yohgaki@ohgaki.net
wrote:One thing regarding implementation.
Since the internet RFC has only 2 values for "samesite", the parameter
can
be
bool rather than string so that users can avoid "broken security by a
typo".
If "samesite" has more than 2 values, the INI handler can be changed
so
that
it can
handle both bool and string parameters.The attribute has 2 possible values, but those are 2 different modes
of operation when enabled, not 2 states in total. It doesn't fit in
a boolean, and even if it did it wouldn't be forward-compatible that
way.What do you mean by "those are 2 different modes
of operation when enabled, not 2 states in total. "?samesite-value = "Strict" / "Lax"
Flag is flag. It does not matter if it is used as combined values.
An INI value can be bool and string/etc. Even when 3rd value is added, it
can
be supported. Such INIs exist in PHP already.A boolean makes sense for Secure and HTTPonly, where the flag either
exists or not. That's not what we have here, as SameSite=Lax is not
the same thing as not having SameSite at all.bool(false) may make sense as an Off switch, yes, but that's not what
you suggested ...
Bool actually have 3 values.
true/false/null(empty)
So there isn't issue being bool INI.
It's much secure than string, since current code does not have validation.
i.e. Typo breaks security setting.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
On Sun, Jul 29, 2018 at 9:27 PM Andrey Andreev narf@devilix.net
wrote:Hi,
On Sun, Jul 29, 2018 at 7:22 AM, Yasuo Ohgaki yohgaki@ohgaki.net
wrote:One thing regarding implementation.
Since the internet RFC has only 2 values for "samesite", the
parameter
can
be
bool rather than string so that users can avoid "broken security by a
typo".
If "samesite" has more than 2 values, the INI handler can be changed
so
that
it can
handle both bool and string parameters.The attribute has 2 possible values, but those are 2 different modes
of operation when enabled, not 2 states in total. It doesn't fit in
a boolean, and even if it did it wouldn't be forward-compatible that
way.What do you mean by "those are 2 different modes
of operation when enabled, not 2 states in total. "?samesite-value = "Strict" / "Lax"
Flag is flag. It does not matter if it is used as combined values.
An INI value can be bool and string/etc. Even when 3rd value is added,
it
can
be supported. Such INIs exist in PHP already.A boolean makes sense for Secure and HTTPonly, where the flag either
exists or not. That's not what we have here, as SameSite=Lax is not
the same thing as not having SameSite at all.bool(false) may make sense as an Off switch, yes, but that's not what
you suggested ...Bool actually have 3 values.
Simple INI handler can do this, precisely.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Ok, I can see how it can be inconvenient for
session_set_cookie_params()
, though calling it "extremely" unfriendly
is some exaggeration IMO.
Hi,
Right, I may have been a bit overly dramatic. :)
But while I didn't quote that part of your
message, you did also suggest to apply the same decision to other
functions and so I am talking about all of them.I'd be ok with this for
session_set_cookie_params()
alone, but not for
set[raw]cookie().
I thought your comment was about session_set_cookie_params only because
your reasoning about lifetime (as a relative amount of time) being a PHP
construct only makes sense there.
So I'm not sure why for set[raw]cookie the expires attribute would be
treated different from the others? Max-Age is derived from it, but the
value you pass to expires will be directly used in the cookie attribute
(although in a different datetime format). Some other attributes are also
not used verbatim. For instance, 'secure' being true or false also means
the secure;
attribute being present or omitted.
Thinking again from the perspective of the user, I would find it annoying
to have the expires attribute separate from the others.
Regards,
Pedro
Hi again,
But while I didn't quote that part of your
message, you did also suggest to apply the same decision to other
functions and so I am talking about all of them.I'd be ok with this for
session_set_cookie_params()
alone, but not for
set[raw]cookie().I thought your comment was about session_set_cookie_params only because
your reasoning about lifetime (as a relative amount of time) being a PHP
construct only makes sense there.
I've been unclear about this, indeed. But I don't see why you thought
I was talking about it as a relative value in particular, nor how my
arguments would only make sense for session_set_cookie_params()
... If
anything, that's the one place where my arguments aren't as strong,
because cookie-related settings are only a small part of the session
handler.
The primary motivation for all my thoughts so far has been to follow
the IETF standards as close as possible, and (as opposed to all other
parameters) cookie attributes are computed from the
lifetime/expire/expires input, regardless of whether it is an absolute
or relative value. Thus, I would like for that distinction to be
obvious in the function design. That's not all of my reasoning, but
more on that below ...
So I'm not sure why for set[raw]cookie the expires attribute would be
treated different from the others? Max-Age is derived from it, but the value
you pass to expires will be directly used in the cookie attribute (although
in a different datetime format). Some other attributes are also not used
verbatim. For instance, 'secure' being true or false also means they
secure;
attribute being present or omitted.
Thinking again from the perspective of the user, I would find it annoying to
have the expires attribute separate from the others.
Well, you only note the different format as a side thing, but it is
still a kind of derivation. As a side note touching on the absolute
vs. relative thing from earlier, I'd argue that if set[raw]cookie()
was designed today, it would accept a relative value (Max-Age wasn't a
thing until PHP 5.5 IIRC, so it wasn't a factor in the original
function design). Anyway ...
If semantic purity is unimportant, I could also ask why by the same
token you don't want $name and $value to be part of the array params?
How are these 2 parameters less of an inconvenience than $expires (or
whatever you call it)? We have to draw the line somewhere and it has
to be clear; turning $expires into an array key/value pair seems
arbitrary to me.
Last, but certainly not least, we talk about $expires here only becase
that's how it's (currently) named in either documentation and/or
reflection. But for all intents and purposes it may as well be named
$fooBar and it wouldn't matter as long as it is a concrete parameter,
whereas an associative array key name is very important. Now I'd have
to remember if it actually is "lifetime", "expire" or "expires" ... or
is it "max-age"? Not only that, but if it is either "expires" or
"max-age", I would rightfully have reasons to believe that the
expected input should be match the actual Set-Cookie attribute instead
of a PHP-specific value.
That's very unintuitive and I believe we have a general consensus on
this list that array parameters are somewhat evil. You have to
remember that the only reason we're doing this here is to avoid
parameter creep with potential for infinity, and nothing else.
And yes, Secure and HTTPonly don't go as is in the Set-Cookie header,
but those are natural edge cases that I'm sure everybody undestands.
Cheers,
Andrey.
Last, but certainly not least, we talk about $expires here only becase
that's how it's (currently) named in either documentation and/or
reflection. But for all intents and purposes it may as well be named
$fooBar and it wouldn't matter as long as it is a concrete parameter,
whereas an associative array key name is very important. Now I'd have
to remember if it actually is "lifetime", "expire" or "expires" ... or
is it "max-age"? Not only that, but if it is either "expires" or
"max-age", I would rightfully have reasons to believe that the
expected input should be match the actual Set-Cookie attribute instead
of a PHP-specific value.
That's very unintuitive and I believe we have a general consensus on
this list that array parameters are somewhat evil. You have to
remember that the only reason we're doing this here is to avoid
parameter creep with potential for infinity, and nothing else.
Hi Andrey,
Well, "expires" is what ends up in the cookie header itself so I think that
it's simple to remember. But I do understand your arguments on semantic
purity and the fact that Max-Age is derived from it but I still believe
that in this case, it's not worth the distinction. If there ever comes a
new attribute that won't be used verbatim, what would we do? Leave it
between $expires and the options array and break all existing code? Leave
it to the end of the signature to avoid the BC break but then we are left
with something really awkward?
Given that we understand each other but we just disagree on what is more
important, I'd really like to hear someone else's opinion. If we are to get
something into 7.3 (which I believe we should due to
https://github.com/php/php-src/pull/2613#issuecomment-401266510) and with
the feature freeze in one week, we should reach an agreement on what to do
very soon.
Regards,
Pedro
Hi,
Last, but certainly not least, we talk about $expires here only becase
that's how it's (currently) named in either documentation and/or
reflection. But for all intents and purposes it may as well be named
$fooBar and it wouldn't matter as long as it is a concrete parameter,
whereas an associative array key name is very important. Now I'd have
to remember if it actually is "lifetime", "expire" or "expires" ... or
is it "max-age"? Not only that, but if it is either "expires" or
"max-age", I would rightfully have reasons to believe that the
expected input should be match the actual Set-Cookie attribute instead
of a PHP-specific value.
That's very unintuitive and I believe we have a general consensus on
this list that array parameters are somewhat evil. You have to
remember that the only reason we're doing this here is to avoid
parameter creep with potential for infinity, and nothing else.Hi Andrey,
Well, "expires" is what ends up in the cookie header itself so I think that
it's simple to remember. But I do understand your arguments on semantic
purity and the fact that Max-Age is derived from it but I still believe that
in this case, it's not worth the distinction. If there ever comes a new
attribute that won't be used verbatim, what would we do? Leave it between
$expires and the options array and break all existing code? Leave it to the
end of the signature to avoid the BC break but then we are left with
something really awkward?
Look, I get it - you have your preferences and don't want to give up
on them. But now you're just speculating and aside from basically
saying "not a big deal", you haven't really addressed my arguments.
Given that we understand each other but we just disagree on what is more
important, I'd really like to hear someone else's opinion. If we are to get
something into 7.3 (which I believe we should due to
https://github.com/php/php-src/pull/2613#issuecomment-401266510) and with
the feature freeze in one week, we should reach an agreement on what to do
very soon.
Fair enough. I too would like to see more people involved in the discussion.
Although ... if the RFC is considered to be accepted (which I am still
not 100% sure if it should be, but that seems to be the case), then
technically we already have a decision made by vote. Again, I'm not
particularly happy with how it was handled, but we do have it.
Cheers,
Andrey.
There are no voting dates in the RFC, but it's open for over a month now.
I guess it can be closed.
Regards, Niklas
2017-08-25 23:19 GMT+02:00 Frederik Bosch f.bosch@genkgo.nl:
LS,
Just now, I opened the RFC on implementing same site cookies in PHP,
https://wiki.php.net/rfc/same-site-cookie, for voting.It consists of two questions, depending on the implementation you would
like to see of the feature. Both questions will affect the API of four core
functions: setcookie, setrawcookie, session_set_cookie_params and
session_get_cookie_params. The first three functions have a similar
function signature. The first implementation suggestion is to add an
additional argument to these three functions. The second implementation
suggestion is to allow an array of options in which all the cookie options
will be moved into. More details are to be found in the RFC.Hopefully, the samesite cookie flag will become a feature of the PHP
language through this RFC!Kind regards,
Frederik Bosch
Hi Niklas,
Sorry for the delay. I have my mind on totally different things these
days. Closed the voting and moved it to accepted. Thanks everyone for
voting! Now, let's implement this RFC!
Best regards,
Frederik
There are no voting dates in the RFC, but it's open for over a month now.
I guess it can be closed.
Regards, Niklas
2017-08-25 23:19 GMT+02:00 Frederik Bosch <f.bosch@genkgo.nl
mailto:f.bosch@genkgo.nl>:LS, Just now, I opened the RFC on implementing same site cookies in PHP, https://wiki.php.net/rfc/same-site-cookie <https://wiki.php.net/rfc/same-site-cookie>, for voting. It consists of two questions, depending on the implementation you would like to see of the feature. Both questions will affect the API of four core functions: setcookie, setrawcookie, session_set_cookie_params and session_get_cookie_params. The first three functions have a similar function signature. The first implementation suggestion is to add an additional argument to these three functions. The second implementation suggestion is to allow an array of options in which all the cookie options will be moved into. More details are to be found in the RFC. Hopefully, the samesite cookie flag will become a feature of the PHP language through this RFC! Kind regards, Frederik Bosch