From 92061a3ba66d2e677e4b3f1a5a4cc269480f6534 Mon Sep 17 00:00:00 2001
From: Dylan Wagstaff <NightJar@users.noreply.github.com>
Date: Tue, 11 Apr 2023 10:52:41 +1200
Subject: [PATCH] FIX stabilise typed APIs (#10740)

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.
---
 src/Control/Cookie.php    | 16 +++++++++++-----
 src/Control/CookieJar.php |  4 ++--
 src/Control/Session.php   |  4 ++--
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/Control/Cookie.php b/src/Control/Cookie.php
index aafefb82b..2995e844f 100644
--- a/src/Control/Cookie.php
+++ b/src/Control/Cookie.php
@@ -14,6 +14,12 @@ class Cookie
 {
     use Configurable;
 
+    public const SAMESITE_LAX = 'Lax';
+
+    public const SAMESITE_STRICT = 'Strict';
+
+    public const SAMESITE_NONE = 'None';
+
     /**
      * @config
      *
@@ -25,7 +31,7 @@ class Cookie
      * Must be "Strict", "Lax", or "None"
      * @config
      */
-    private static string $default_samesite = 'Lax';
+    private static string $default_samesite = self::SAMESITE_LAX;
 
     /**
      * Fetch the current instance of the cookie backend.
@@ -110,14 +116,14 @@ class Cookie
     public static function validateSameSite(string $sameSite): void
     {
         $validValues = [
-            'Strict',
-            'Lax',
-            'None',
+            self::SAMESITE_STRICT,
+            self::SAMESITE_LAX,
+            self::SAMESITE_NONE,
         ];
         if (!in_array($sameSite, $validValues)) {
             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.');
         }
     }
diff --git a/src/Control/CookieJar.php b/src/Control/CookieJar.php
index 73467526c..04b5ee74b 100644
--- a/src/Control/CookieJar.php
+++ b/src/Control/CookieJar.php
@@ -207,8 +207,8 @@ class CookieJar implements Cookie_Backend
     {
         Deprecation::notice('4.12.0', 'The relevant methods will include a `$sameSite` parameter instead.');
         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;
     }
 }
diff --git a/src/Control/Session.php b/src/Control/Session.php
index 01623d87f..31ed9b426 100644
--- a/src/Control/Session.php
+++ b/src/Control/Session.php
@@ -139,7 +139,7 @@ class Session
      * Must be "Strict", "Lax", or "None".
      * @config
      */
-    private static string $cookie_samesite = 'Lax';
+    private static string $cookie_samesite = Cookie::SAMESITE_LAX;
 
     /**
      * 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);
         $secure = $this->isCookieSecure($sameSite, Director::is_https($request));