Skip to content

Commit

Permalink
Add SameSite parameter to cookie setting functions
Browse files Browse the repository at this point in the history
  • Loading branch information
Girgias committed Jan 14, 2023
1 parent a493da7 commit 233c984
Show file tree
Hide file tree
Showing 11 changed files with 201 additions and 17 deletions.
25 changes: 24 additions & 1 deletion ext/session/session.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "ext/standard/basic_functions.h"
#include "ext/standard/head.h"
#include "ext/random/php_random.h"
#include "zend_enum.h"

#include "mod_files.h"
#include "mod_user.h"
Expand Down Expand Up @@ -1667,6 +1668,7 @@ PHP_FUNCTION(session_set_cookie_params)
zend_string *lifetime = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
bool secure = 0, secure_null = 1;
bool httponly = 0, httponly_null = 1;
zend_object *same_site_enum = NULL;
zend_string *ini_name;
zend_result result;
int found = 0;
Expand All @@ -1675,13 +1677,14 @@ PHP_FUNCTION(session_set_cookie_params)
return;
}

ZEND_PARSE_PARAMETERS_START(1, 5)
ZEND_PARSE_PARAMETERS_START(1, 6)
Z_PARAM_ARRAY_HT_OR_LONG(options_ht, lifetime_long)
Z_PARAM_OPTIONAL
Z_PARAM_STR_OR_NULL(path)
Z_PARAM_STR_OR_NULL(domain)
Z_PARAM_BOOL_OR_NULL(secure, secure_null)
Z_PARAM_BOOL_OR_NULL(httponly, httponly_null)
Z_PARAM_OBJ_OF_CLASS(same_site_enum, SameSite_ce)
ZEND_PARSE_PARAMETERS_END();

if (PS(session_status) == php_session_active) {
Expand Down Expand Up @@ -1717,6 +1720,12 @@ PHP_FUNCTION(session_set_cookie_params)
zend_argument_value_error(5, "must be null when argument #1 ($lifetime_or_options) is an array");
RETURN_THROWS();
}
/* Use ArgumentCount error trick similar to base setcookie() function */
if (same_site_enum) {
zend_argument_count_error("session_set_cookie_params(): Expects exactly 1 arguments when argument #1 "
"($lifetime_or_options) is an array");
RETURN_THROWS();
}
ZEND_HASH_FOREACH_STR_KEY_VAL(options_ht, key, value) {
if (key) {
ZVAL_DEREF(value);
Expand Down Expand Up @@ -1806,6 +1815,20 @@ PHP_FUNCTION(session_set_cookie_params)
goto cleanup;
}
}
if (same_site_enum) {
zval *case_name = zend_enum_fetch_case_name(same_site_enum);
samesite = zend_string_copy(Z_STR_P(case_name));

/* 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")) {
zend_string_release(samesite);
zend_argument_value_error(6, "can only be SameSite::None if argument #6 ($secure) is true");
RETVAL_FALSE;
goto cleanup;
/* RETURN_THROWS(); */
}
}
if (samesite) {
ini_name = zend_string_init("session.cookie_samesite", sizeof("session.cookie_samesite") - 1, 0);
result = zend_alter_ini_entry(ini_name, samesite, PHP_INI_USER, PHP_INI_STAGE_RUNTIME);
Expand Down
2 changes: 1 addition & 1 deletion ext/session/session.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ function session_cache_limiter(?string $value = null): string|false {}

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 {}

function session_start(array $options = []): bool {}

Expand Down
3 changes: 2 additions & 1 deletion ext/session/session_arginfo.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

81 changes: 81 additions & 0 deletions ext/session/tests/session_set_cookie_params_SameSite_param.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
--TEST--
session_set_cookie_params() SameSite parameter tests
--EXTENSIONS--
session
--SKIPIF--
<?php include('skipif.inc'); ?>
--FILE--
<?php

ob_start();

// Check type errors work
try {
session_set_cookie_params(20, sameSite: new stdClass());
} catch (\TypeError $e) {
echo $e->getMessage(), \PHP_EOL;
}

// Check ValueError when using SameSite::None and the cookie is not secure, see
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#samesitenone_requires_secure
try {
session_set_cookie_params(20, sameSite: SameSite::None);
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
session_set_cookie_params(20, sameSite: SameSite::None, secure: false);
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}

// Check get argument count error when using an option array
try {
session_set_cookie_params(['secure' => false], sameSite: SameSite::None);
} catch (\ArgumentCountError $e) {
echo $e->getMessage(), \PHP_EOL;
}

echo "*** Testing session_set_cookie_params(20, sameSite:) ***\n";
var_dump(session_set_cookie_params(20, sameSite: SameSite::None, secure: true));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_set_cookie_params(20, sameSite: SameSite::Lax, secure: true));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_set_cookie_params(20, sameSite: SameSite::Strict, secure: true));
var_dump(ini_get("session.cookie_samesite"));
// After session started
var_dump(session_start());
var_dump(session_set_cookie_params(20, sameSite: SameSite::None, secure: true));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_set_cookie_params(20, sameSite: SameSite::Lax, secure: true));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_set_cookie_params(20, sameSite: SameSite::Strict, secure: true));

echo "Done";
ob_end_flush();
?>
--EXPECTF--
session_set_cookie_params(): Argument #6 ($sameSite) must be of type SameSite, stdClass given
session_set_cookie_params(): Argument #6 ($sameSite) can only be SameSite::None if argument #6 ($secure) is true
session_set_cookie_params(): Argument #6 ($sameSite) can only be SameSite::None if argument #6 ($secure) is true
session_set_cookie_params(): Expects exactly 1 arguments when argument #1 ($lifetime_or_options) is an array
*** Testing session_set_cookie_params(20, sameSite:) ***
bool(true)
string(4) "None"
bool(true)
string(3) "Lax"
bool(true)
string(6) "Strict"
bool(true)

Warning: session_set_cookie_params(): Session cookie parameters cannot be changed when a session is active in %s on line %d
bool(false)
string(6) "Strict"

Warning: session_set_cookie_params(): Session cookie parameters cannot be changed when a session is active in %s on line %d
bool(false)
string(6) "Strict"

Warning: session_set_cookie_params(): Session cookie parameters cannot be changed when a session is active in %s on line %d
bool(false)
Done
2 changes: 2 additions & 0 deletions ext/standard/basic_functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "php_ext_syslog.h"
#include "ext/standard/info.h"
#include "ext/session/php_session.h"
#include "zend_enum.h"
#include "zend_exceptions.h"
#include "zend_attributes.h"
#include "zend_ini.h"
Expand Down Expand Up @@ -306,6 +307,7 @@ PHP_MINIT_FUNCTION(basic) /* {{{ */
php_register_incomplete_class_handlers();

assertion_error_ce = register_class_AssertionError(zend_ce_error);
SameSite_ce = register_class_SameSite();

BASIC_MINIT_SUBMODULE(var)
BASIC_MINIT_SUBMODULE(file)
Expand Down
10 changes: 8 additions & 2 deletions ext/standard/basic_functions.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -2191,9 +2191,15 @@ function header(string $header, bool $replace = true, int $response_code = 0): v

function header_remove(?string $name = null): void {}

function setrawcookie(string $name, string $value = "", array|int $expires_or_options = 0, string $path = "", string $domain = "", bool $secure = false, bool $httponly = false): bool {}
enum SameSite {
case None;
case Lax;
case Strict;
}

function setrawcookie(string $name, string $value = "", array|int $expires_or_options = 0, string $path = "", string $domain = "", bool $secure = false, bool $httponly = false, SameSite $sameSite = SameSite::Lax): bool {}

function setcookie(string $name, string $value = "", array|int $expires_or_options = 0, string $path = "", string $domain = "", bool $secure = false, bool $httponly = false): bool {}
function setcookie(string $name, string $value = "", array|int $expires_or_options = 0, string $path = "", string $domain = "", bool $secure = false, bool $httponly = false, SameSite $sameSite = SameSite::Lax): bool {}

function http_response_code(int $response_code = 0): int|bool {}

Expand Down
21 changes: 20 additions & 1 deletion ext/standard/basic_functions_arginfo.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 18 additions & 1 deletion ext/standard/head.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@

#include "php_globals.h"
#include "zend_smart_str.h"
#include "zend_enum.h"

PHPAPI zend_class_entry *SameSite_ce;

/* Implementation of the language Header() function */
/* {{{ Sends a raw HTTP header */
Expand Down Expand Up @@ -227,9 +229,10 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
HashTable *options = NULL;
zend_long expires = 0;
zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
zend_object *same_site_enum = NULL;
bool secure = 0, httponly = 0;

ZEND_PARSE_PARAMETERS_START(1, 7)
ZEND_PARSE_PARAMETERS_START(1, 8)
Z_PARAM_STR(name)
Z_PARAM_OPTIONAL
Z_PARAM_STR(value)
Expand All @@ -238,6 +241,7 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
Z_PARAM_STR(domain)
Z_PARAM_BOOL(secure)
Z_PARAM_BOOL(httponly)
Z_PARAM_OBJ_OF_CLASS(same_site_enum, SameSite_ce)
ZEND_PARSE_PARAMETERS_END();

if (options) {
Expand All @@ -254,6 +258,19 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
}
}

if (same_site_enum) {
zval *case_name = zend_enum_fetch_case_name(same_site_enum);
samesite = zend_string_copy(Z_STR_P(case_name));

/* 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")) {
zend_string_release(samesite);
zend_argument_value_error(8, "can only be SameSite::None if argument #6 ($secure) is true");
RETURN_THROWS();
}
}

if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
RETVAL_TRUE;
} else {
Expand Down
2 changes: 2 additions & 0 deletions ext/standard/head.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,6 @@ PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t e
zend_string *path, zend_string *domain, bool secure, bool httponly,
zend_string *samesite, bool url_encode);

extern PHPAPI zend_class_entry *SameSite_ce;

#endif
20 changes: 10 additions & 10 deletions ext/standard/tests/network/setcookie.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ setcookie('name', 'value', 0, '', '', FALSE, TRUE);

setcookie('name', 'value', ['expires' => $tsp]);
setcookie('name', 'value', ['expires' => $tsn, 'path' => '/path/', 'domain' => 'domain.tld', 'secure' => true, 'httponly' => true, 'samesite' => 'Strict']);
setcookie('name', 'value', /*expires:*/ $tsp, path: '/path/', domain: 'domain.tld', secure: true, httponly: true, sameSite: SameSite::None);
setcookie('name', 'value', /*expires:*/ $tsp, path: '/path/', domain: 'domain.tld', secure: true, httponly: true, sameSite: SameSite::Lax);
setcookie('name', 'value', /*expires:*/ $tsp, path: '/path/', domain: 'domain.tld', secure: true, httponly: true, sameSite: SameSite::Strict);

$expected = array(
'Set-Cookie: name=deleted; expires='.date('D, d M Y H:i:s', 1).' GMT; Max-Age=0',
Expand All @@ -34,7 +37,10 @@ $expected = array(
'Set-Cookie: name=value; secure',
'Set-Cookie: name=value; HttpOnly',
'Set-Cookie: name=value; expires='.date('D, d M Y H:i:s', $tsp).' GMT; Max-Age=5',
'Set-Cookie: name=value; expires='.date('D, d M Y H:i:s', $tsn).' GMT; Max-Age=0; path=/path/; domain=domain.tld; secure; HttpOnly; SameSite=Strict'
'Set-Cookie: name=value; expires='.date('D, d M Y H:i:s', $tsn).' GMT; Max-Age=0; path=/path/; domain=domain.tld; secure; HttpOnly; SameSite=Strict',
'Set-Cookie: name=value; expires='.date('D, d M Y H:i:s', $tsp).' GMT; Max-Age=5; path=/path/; domain=domain.tld; secure; HttpOnly; SameSite=None',
'Set-Cookie: name=value; expires='.date('D, d M Y H:i:s', $tsp).' GMT; Max-Age=5; path=/path/; domain=domain.tld; secure; HttpOnly; SameSite=Lax',
'Set-Cookie: name=value; expires='.date('D, d M Y H:i:s', $tsp).' GMT; Max-Age=5; path=/path/; domain=domain.tld; secure; HttpOnly; SameSite=Strict',
);

$headers = headers_list();
Expand All @@ -44,26 +50,20 @@ if (($i = count($expected)) > count($headers))
return;
}

do {
$header = current($headers);
if (strncmp($header, 'Set-Cookie:', 11) !== 0) {
foreach ($headers as $header) {
if (!str_starts_with($header, 'Set-Cookie:')) {
continue;
}

// If the second rolls over between the time() call and the internal time determination by
// setcookie(), we might get Max-Age=4 instead of Max-Age=5.
$header = str_replace('Max-Age=4', 'Max-Age=5', $header);
if ($header === current($expected)) {
$i--;
} else {
echo "Header mismatch:\n\tExpected: "
.current($expected)
."\n\tReceived: ".current($headers)."\n";
echo "Header mismatch:\n\tExpected: ", current($expected), "\n\tReceived: ", $header, "\n";
}

next($expected);
}
while (next($headers) !== FALSE);

echo ($i === 0)
? 'OK'
Expand Down
33 changes: 33 additions & 0 deletions ext/standard/tests/network/setcookie_samesite_param.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
--TEST--
setcookie() SameSite parameter errors
--FILE--
<?php

// Check type errors work
try {
setcookie('name', sameSite: new stdClass());
} catch (\TypeError $e) {
echo $e->getMessage(), \PHP_EOL;
}

// Check ValueError when using SameSite::None and the cookie is not secure, see
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#samesitenone_requires_secure
try {
setcookie('name', sameSite: SameSite::None);
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
setcookie('name', sameSite: SameSite::None, secure: false);
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}

// Sanity enum check
var_dump(SameSite::Strict);
?>
--EXPECT--
setcookie(): Argument #8 ($sameSite) must be of type SameSite, stdClass given
setcookie(): Argument #8 ($sameSite) can only be SameSite::None if argument #6 ($secure) is true
setcookie(): Argument #8 ($sameSite) can only be SameSite::None if argument #6 ($secure) is true
enum(SameSite::Strict)

0 comments on commit 233c984

Please sign in to comment.