tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

FIX stabilise typed APIs (#10740)

Browse Source 
Since 4.12 the use of typehints and return types has caused issues with
values fetched directly from config without validation. This has lead to
upgrade woes in a minor version (#10721) with no immediate recourse
other than manual system intervention.

To use types, we should ensure types, leaving a stable API that won't
error on a bad value - or should give a thoughtful and directive error
message if so.

Issue #10721 summary:
SessionMiddleware runs before FlushMiddleware
SessionMiddleware causes a PHP fatal error passing `null` to a `string`
parameter.
`null` comes from config, because default string value doesn't exist. We
need flush for this - but system execution never makes it that far.
This commit is contained in:
Dylan Wagstaff 2023-04-11 10:52:41 +12:00 committed by GitHub
parent 403f924d22
commit 92061a3ba6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 15 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/Control/Cookie.php
View File

@ -14,6 +14,12 @@ class Cookie
{ {
use Configurable; use Configurable;
public const SAMESITE_LAX = 'Lax';
public const SAMESITE_STRICT = 'Strict';
public const SAMESITE_NONE = 'None';
/** /**
* @config * @config
* *
@ -25,7 +31,7 @@ class Cookie
* Must be "Strict", "Lax", or "None" * Must be "Strict", "Lax", or "None"
* @config * @config
*/ */
private static string $default_samesite = 'Lax'; private static string $default_samesite = self::SAMESITE_LAX;
/** /**
* Fetch the current instance of the cookie backend. * Fetch the current instance of the cookie backend.
@ -110,14 +116,14 @@ class Cookie
public static function validateSameSite(string $sameSite): void public static function validateSameSite(string $sameSite): void
{ {
$validValues = [ $validValues = [
'Strict', self::SAMESITE_STRICT,
'Lax', self::SAMESITE_LAX,
'None', self::SAMESITE_NONE,
]; ];
if (!in_array($sameSite, $validValues)) { if (!in_array($sameSite, $validValues)) {
throw new LogicException('Cookie samesite must be "Strict", "Lax", or "None"'); throw new LogicException('Cookie samesite must be "Strict", "Lax", or "None"');
} }
if ($sameSite === 'None' && !Director::is_https(self::getRequest())) { if ($sameSite === self::SAMESITE_NONE && !Director::is_https(self::getRequest())) {
Injector::inst()->get(LoggerInterface::class)->warning('Cookie samesite cannot be "None" for non-https requests.'); Injector::inst()->get(LoggerInterface::class)->warning('Cookie samesite cannot be "None" for non-https requests.');
} }
} }

4
src/Control/CookieJar.php
View File

@ -207,8 +207,8 @@ class CookieJar implements Cookie_Backend
{ {
Deprecation::notice('4.12.0', 'The relevant methods will include a `$sameSite` parameter instead.'); Deprecation::notice('4.12.0', 'The relevant methods will include a `$sameSite` parameter instead.');
if ($name === session_name()) { if ($name === session_name()) {
return Session::config()->get('cookie_samesite'); return Session::config()->get('cookie_samesite') ?? Cookie::SAMESITE_LAX;
} }
return Cookie::config()->get('default_samesite'); return Cookie::config()->get('default_samesite') ?? Cookie::SAMESITE_LAX;
} }
} }

4
src/Control/Session.php
View File

@ -139,7 +139,7 @@ class Session
* Must be "Strict", "Lax", or "None". * Must be "Strict", "Lax", or "None".
* @config * @config
*/ */
private static string $cookie_samesite = 'Lax'; private static string $cookie_samesite = Cookie::SAMESITE_LAX;
/** /**
* Name of session cache limiter to use. * Name of session cache limiter to use.
@ -374,7 +374,7 @@ class Session
} }
} }
$sameSite = static::config()->get('cookie_samesite'); $sameSite = static::config()->get('cookie_samesite') ?? Cookie::SAMESITE_LAX;
Cookie::validateSameSite($sameSite); Cookie::validateSameSite($sameSite);
$secure = $this->isCookieSecure($sameSite, Director::is_https($request)); $secure = $this->isCookieSecure($sameSite, Director::is_https($request));