diff --git a/system/HTTP/Response.php b/system/HTTP/Response.php index 0df154848f39..59474ad0938d 100644 --- a/system/HTTP/Response.php +++ b/system/HTTP/Response.php @@ -14,9 +14,7 @@ namespace CodeIgniter\HTTP; use CodeIgniter\Cookie\Cookie; -use CodeIgniter\Cookie\CookieStore; use CodeIgniter\HTTP\Exceptions\HTTPException; -use Config\Cookie as CookieConfig; /** * Representation of an outgoing, server-side response. @@ -38,7 +36,7 @@ class Response extends Message implements ResponseInterface /** * HTTP status codes * - * @var array + * @var array */ protected static $statusCodes = [ // 1xx: Informational @@ -140,23 +138,14 @@ class Response extends Message implements ResponseInterface */ protected $pretend = false; + /** + * Construct a non-caching response with a default content type of `text/html`. + */ public function __construct() { - // Default to a non-caching page. - // Also ensures that a Cache-control header exists. - $this->noCache(); - - // We need CSP object even if not enabled to avoid calls to non existing methods - $this->CSP = service('csp'); - - $this->cookieStore = new CookieStore([]); - - $cookie = config(CookieConfig::class); - - Cookie::setDefaults($cookie); + Cookie::setDefaults(config('Cookie')); - // Default to an HTML Content-Type. Devs can override if needed. - $this->setContentType('text/html'); + $this->noCache()->setContentType('text/html'); } /** @@ -168,7 +157,6 @@ public function __construct() * @return $this * * @internal For testing purposes only. - * @testTag only available to test code */ public function pretend(bool $pretend = true) { diff --git a/system/HTTP/ResponseTrait.php b/system/HTTP/ResponseTrait.php index 3063e44116fb..a2e2b01b69d3 100644 --- a/system/HTTP/ResponseTrait.php +++ b/system/HTTP/ResponseTrait.php @@ -13,6 +13,7 @@ namespace CodeIgniter\HTTP; +use CodeIgniter\Config\Services; use CodeIgniter\Cookie\Cookie; use CodeIgniter\Cookie\CookieStore; use CodeIgniter\Cookie\Exceptions\CookieException; @@ -21,6 +22,8 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Pager\PagerInterface; use CodeIgniter\Security\Exceptions\SecurityException; +use Config\App; +use Config\ContentSecurityPolicy as ContentSecurityPolicyConfig; use Config\Cookie as CookieConfig; use DateTime; use DateTimeZone; @@ -31,21 +34,30 @@ * Additional methods to make a PSR-7 Response class * compliant with the framework's own ResponseInterface. * + * @property array $statusCodes + * @property string|null $body + * * @see https://github.com/php-fig/http-message/blob/master/src/ResponseInterface.php */ trait ResponseTrait { /** - * Content security policy handler + * Content security policy handler. + * + * Lazily instantiated on first use via `self::getCSP()` so that the + * ContentSecurityPolicy class is not loaded on requests that do not use CSP. * - * @var ContentSecurityPolicy + * @var ContentSecurityPolicy|null */ protected $CSP; /** * CookieStore instance. * - * @var CookieStore + * Lazily instantiated on first cookie-related call so that the Cookie and + * CookieStore classes are not loaded on requests that do not use cookies. + * + * @var CookieStore|null */ protected $cookieStore; @@ -77,19 +89,17 @@ trait ResponseTrait */ public function setStatusCode(int $code, string $reason = '') { - // Valid range? if ($code < 100 || $code > 599) { throw HTTPException::forInvalidStatusCode($code); } - // Unknown and no message? - if (! array_key_exists($code, static::$statusCodes) && ($reason === '')) { + if (! array_key_exists($code, static::$statusCodes) && $reason === '') { throw HTTPException::forUnkownStatusCode($code); } $this->statusCode = $code; - $this->reason = ($reason !== '') ? $reason : static::$statusCodes[$code]; + $this->reason = $reason !== '' ? $reason : static::$statusCodes[$code]; return $this; } @@ -366,8 +376,10 @@ public function setLastModified($date) public function send() { // If we're enforcing a Content Security Policy, - // we need to give it a chance to build out it's headers. - $this->CSP->finalize($this); + // we need to give it a chance to build out its headers. + if ($this->shouldFinalizeCsp()) { + $this->getCSP()->finalize($this); + } $this->sendHeaders(); $this->sendCookies(); @@ -376,6 +388,44 @@ public function send() return $this; } + /** + * Decides whether {@see ContentSecurityPolicy::finalize()} should run for + * this response. Keeping the CSP class unloaded on requests that do not + * need it avoids the cost of constructing a 1000+ line service on every + * request. + */ + private function shouldFinalizeCsp(): bool + { + // Developer already touched CSP through getCSP(); respect it. + if ($this->CSP !== null) { + return true; + } + + // A CSP instance has been registered (e.g., via Services::injectMock() + // or any earlier service('csp') call) — reuse it instead of skipping. + if (Services::has('csp')) { + return true; + } + + if (config(App::class)->CSPEnabled) { + return true; + } + + // Placeholders in the body still need to be stripped even when CSP + // is disabled, so the body is scanned for the configured nonce tags + // before committing to loading the full CSP class. + $body = (string) $this->body; + + if ($body === '') { + return false; + } + + $cspConfig = config(ContentSecurityPolicyConfig::class); + + return str_contains($body, $cspConfig->scriptNonceTag) + || str_contains($body, $cspConfig->styleNonceTag); + } + /** * Sends the headers of this HTTP response to the browser. * @@ -518,8 +568,10 @@ public function setCookie( $httponly = null, $samesite = null, ) { + $store = $this->initializeCookieStore(); + if ($name instanceof Cookie) { - $this->cookieStore = $this->cookieStore->put($name); + $this->cookieStore = $store->put($name); return $this; } @@ -553,7 +605,7 @@ public function setCookie( 'samesite' => $samesite ?? '', ]); - $this->cookieStore = $this->cookieStore->put($cookie); + $this->cookieStore = $store->put($cookie); return $this; } @@ -565,7 +617,7 @@ public function setCookie( */ public function getCookieStore() { - return $this->cookieStore; + return $this->initializeCookieStore(); } /** @@ -573,9 +625,10 @@ public function getCookieStore() */ public function hasCookie(string $name, ?string $value = null, string $prefix = ''): bool { + $store = $this->initializeCookieStore(); $prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC - return $this->cookieStore->has($name, $prefix, $value); + return $store->has($name, $prefix, $value); } /** @@ -588,14 +641,16 @@ public function hasCookie(string $name, ?string $value = null, string $prefix = */ public function getCookie(?string $name = null, string $prefix = '') { + $store = $this->initializeCookieStore(); + if ((string) $name === '') { - return $this->cookieStore->display(); + return $store->display(); } try { $prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC - return $this->cookieStore->get($name, $prefix); + return $store->get($name, $prefix); } catch (CookieException $e) { log_message('error', (string) $e); @@ -614,10 +669,10 @@ public function deleteCookie(string $name = '', string $domain = '', string $pat return $this; } + $store = $this->initializeCookieStore(); $prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC $prefixed = $prefix . $name; - $store = $this->cookieStore; $found = false; /** @var Cookie $cookie */ @@ -653,6 +708,10 @@ public function deleteCookie(string $name = '', string $domain = '', string $pat */ public function getCookies() { + if ($this->cookieStore === null) { + return []; + } + return $this->cookieStore->display(); } @@ -663,7 +722,7 @@ public function getCookies() */ protected function sendCookies() { - if ($this->pretend) { + if ($this->pretend || $this->cookieStore === null) { return; } @@ -753,6 +812,18 @@ public function download(string $filename = '', $data = '', bool $setMime = fals public function getCSP(): ContentSecurityPolicy { - return $this->CSP; + return $this->CSP ??= service('csp'); + } + + /** + * Lazily initializes the cookie store and the Cookie class defaults. + * Called by every cookie-related method so cookie machinery is only + * loaded when the developer actually interacts with cookies. + */ + private function initializeCookieStore(): CookieStore + { + $this->cookieStore ??= new CookieStore([]); + + return $this->cookieStore; } } diff --git a/tests/system/HTTP/ResponseTest.php b/tests/system/HTTP/ResponseTest.php index f68047228513..0b83ab02feec 100644 --- a/tests/system/HTTP/ResponseTest.php +++ b/tests/system/HTTP/ResponseTest.php @@ -15,6 +15,8 @@ use CodeIgniter\Config\Factories; use CodeIgniter\Config\Services; +use CodeIgniter\Cookie\Cookie; +use CodeIgniter\Cookie\CookieStore; use CodeIgniter\HTTP\Exceptions\HTTPException; use CodeIgniter\Superglobals; use CodeIgniter\Test\CIUnitTestCase; @@ -24,6 +26,7 @@ use DateTimeZone; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; +use ReflectionClass; /** * @internal @@ -685,4 +688,95 @@ public function testSendRemovesPlaceholdersWhenBothCSPAndAutoNonceAreDisabled(): $this->assertStringContainsString('', $actual); $this->assertStringContainsString('', $actual); } + + /** + * @see https://github.com/codeigniter4/CodeIgniter4/issues/8201 + */ + public function testConstructorDoesNotLoadCspAndCookieClasses(): void + { + $response = (new ReflectionClass(Response::class))->newInstance(); + + $this->assertNull( + $this->getPrivateProperty($response, 'CSP'), + 'CSP must be lazily instantiated, not loaded in Response::__construct().', + ); + $this->assertNull( + $this->getPrivateProperty($response, 'cookieStore'), + 'CookieStore must be lazily instantiated, not loaded in Response::__construct().', + ); + } + + public function testSendWithoutCspOrCookiesDoesNotLoadThoseClasses(): void + { + $response = new Response(); + $response->pretend(true); + $response->setBody('No CSP or cookies here.'); + + ob_start(); + $response->send(); + ob_end_clean(); + + $this->assertNull($this->getPrivateProperty($response, 'CSP')); + $this->assertNull($this->getPrivateProperty($response, 'cookieStore')); + } + + public function testGetCspLazilyInstantiatesCsp(): void + { + $response = new Response(); + + $this->assertNull($this->getPrivateProperty($response, 'CSP')); + + $csp = $response->getCSP(); + + $this->assertInstanceOf(ContentSecurityPolicy::class, $csp); + $this->assertSame($csp, $response->getCSP(), 'Subsequent getCSP() calls must return the same instance.'); + } + + public function testSetCookieLazilyInstantiatesCookieStore(): void + { + $response = new Response(); + + $this->assertNull($this->getPrivateProperty($response, 'cookieStore')); + + $response->setCookie('foo', 'bar'); + + $this->assertInstanceOf(CookieStore::class, $this->getPrivateProperty($response, 'cookieStore')); + $this->assertTrue($response->hasCookie('foo')); + } + + public function testGetCookiesReturnsEmptyArrayWhenCookieStoreNotInitialized(): void + { + $response = new Response(); + + $this->assertSame([], $response->getCookies()); + $this->assertNull( + $this->getPrivateProperty($response, 'cookieStore'), + 'getCookies() must not instantiate the store when no cookies have been set.', + ); + } + + public function testGetCookieStillUsesSetCookieDefaultsWhenStoreNotInitialized(): void + { + $oldDefaults = Cookie::setDefaults(); + + config('Cookie')->prefix = 'test_'; + + try { + $response = new Response(); + + $this->assertNull($this->getPrivateProperty($response, 'cookieStore')); + + $response->setCookie('foo', 'bar'); + + $this->assertInstanceOf(CookieStore::class, $this->getPrivateProperty($response, 'cookieStore')); + $this->assertTrue($response->hasCookie('foo')); + + $cookie = $response->getCookie('foo'); + $this->assertSame('test_', $cookie->getPrefix()); + $this->assertSame('test_foo', $cookie->getPrefixedName()); + } finally { + Cookie::setDefaults($oldDefaults); + Factories::reset('config'); + } + } } diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index 99c0adfc8355..577f947d8b92 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -230,6 +230,9 @@ HTTP - Added ``SSEResponse`` class for streaming Server-Sent Events (SSE) over HTTP. See :ref:`server-sent-events`. - ``Response`` and its child classes no longer require ``Config\App`` passed to their constructors. Consequently, ``CURLRequest``'s ``$config`` parameter is unused and will be removed in a future release. +- ``Response`` now lazily loads the ``ContentSecurityPolicy``, ``Cookie``, and ``CookieStore`` classes only when used, + instead of instantiating them on every request in the constructor. Requests that do not touch CSP or cookies no longer + incur the cost of loading these classes. - ``CLIRequest`` now supports the ``--`` separator to mean that what follows are arguments, not options. This allows you to have arguments that start with ``-`` without them being treated as options. For example: ``php index.php command -- --myarg`` will pass ``--myarg`` as an argument instead of an option. - ``CLIRequest`` now supports options with values specified using an equals sign (e.g., ``--option=value``) in addition to the existing space-separated syntax (e.g., ``--option value``). diff --git a/utils/phpstan-baseline/loader.neon b/utils/phpstan-baseline/loader.neon index e0f08e02f8e0..7c26b843605b 100644 --- a/utils/phpstan-baseline/loader.neon +++ b/utils/phpstan-baseline/loader.neon @@ -1,4 +1,4 @@ -# total 2022 errors +# total 2021 errors includes: - argument.type.neon diff --git a/utils/phpstan-baseline/missingType.iterableValue.neon b/utils/phpstan-baseline/missingType.iterableValue.neon index 9bd09760795e..34e776f0a53f 100644 --- a/utils/phpstan-baseline/missingType.iterableValue.neon +++ b/utils/phpstan-baseline/missingType.iterableValue.neon @@ -1,4 +1,4 @@ -# total 1229 errors +# total 1228 errors parameters: ignoreErrors: @@ -2907,11 +2907,6 @@ parameters: count: 1 path: ../../system/HTTP/Response.php - - - message: '#^Property CodeIgniter\\HTTP\\Response\:\:\$statusCodes type has no value type specified in iterable type array\.$#' - count: 1 - path: ../../system/HTTP/Response.php - - message: '#^Method CodeIgniter\\HTTP\\ResponseInterface\:\:setCache\(\) has parameter \$options with no value type specified in iterable type array\.$#' count: 1