Skip to content

[RFC] Add SameSite parameter to cookie setting functions - #10317

Open
Girgias wants to merge 1 commit into
php:masterfrom
Girgias:cookies-samesite-param
Open

[RFC] Add SameSite parameter to cookie setting functions#10317
Girgias wants to merge 1 commit into
php:masterfrom
Girgias:cookies-samesite-param

Conversation

@Girgias

Copy link
Copy Markdown
Member

Comment threadext/session/session.c

/* Verify that cookie is secure if using SameSite::None, see
* https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#samesitenone_requires_secure */
if (!secure && zend_string_equals_literal(samesite, "None")) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this passes, this would be the second native enum. Leaving this link here for reference: #9679 (comment)

function session_cache_expire(?int $value = null): int|false {}

function session_set_cookie_params(array|int $lifetime_or_options, ?string $path = null, ?string $domain = null, ?bool $secure = null, ?bool $httponly = null): bool {}
function session_set_cookie_params(array|int $lifetime_or_options, ?string $path = null, ?string $domain = null, ?bool $secure = null, ?bool $httponly = null, SameSite $sameSite = SameSite::Lax): bool {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just general feeling impression, the camel case feels out of place whereas is mostly lower case with underscore otherwise.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@jorgsowa

Copy link
Copy Markdown
Contributor

@Girgias, do you plan to put this RFC under a vote before the feature freeze?

@Girgias

Copy link
Copy Markdown
MemberAuthor

No, because at the time people didn't seem convinced, and I don't have the energy to pursue it. If you want to take it over, please feel free :)

@jorgsowa

Copy link
Copy Markdown
Contributor

I understand you. That's unfortunate to hear. I'll think about it, but I'd prefer to avoid tasks that involve a lot of persuading people.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Girgias@jorgsowa@TimWolla@devnexen