Skip to content

Commit 5f89740

Browse files
authored
refactor: lazily load CSP and Cookie only when needed by Response (#10121)
* refactor: lazily load CSP and Cookie only when needed by Response * address review
1 parent 80f8cd8 commit 5f89740

6 files changed

Lines changed: 187 additions & 41 deletions

File tree

system/HTTP/Response.php

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@
1414
namespace CodeIgniter\HTTP;
1515

1616
use CodeIgniter\Cookie\Cookie;
17-
use CodeIgniter\Cookie\CookieStore;
1817
use CodeIgniter\HTTP\Exceptions\HTTPException;
19-
use Config\Cookie as CookieConfig;
2018

2119
/**
2220
* Representation of an outgoing, server-side response.
@@ -38,7 +36,7 @@ class Response extends Message implements ResponseInterface
3836
/**
3937
* HTTP status codes
4038
*
41-
* @var array
39+
* @var array<int, string>
4240
*/
4341
protected static $statusCodes = [
4442
// 1xx: Informational
@@ -140,23 +138,14 @@ class Response extends Message implements ResponseInterface
140138
*/
141139
protected $pretend = false;
142140

141+
/**
142+
* Construct a non-caching response with a default content type of `text/html`.
143+
*/
143144
public function __construct()
144145
{
145-
// Default to a non-caching page.
146-
// Also ensures that a Cache-control header exists.
147-
$this->noCache();
148-
149-
// We need CSP object even if not enabled to avoid calls to non existing methods
150-
$this->CSP = service('csp');
151-
152-
$this->cookieStore = new CookieStore([]);
153-
154-
$cookie = config(CookieConfig::class);
155-
156-
Cookie::setDefaults($cookie);
146+
Cookie::setDefaults(config('Cookie'));
157147

158-
// Default to an HTML Content-Type. Devs can override if needed.
159-
$this->setContentType('text/html');
148+
$this->noCache()->setContentType('text/html');
160149
}
161150

162151
/**
@@ -168,7 +157,6 @@ public function __construct()
168157
* @return $this
169158
*
170159
* @internal For testing purposes only.
171-
* @testTag only available to test code
172160
*/
173161
public function pretend(bool $pretend = true)
174162
{

system/HTTP/ResponseTrait.php

Lines changed: 82 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace CodeIgniter\HTTP;
1515

16+
use CodeIgniter\Config\Services;
1617
use CodeIgniter\Cookie\Cookie;
1718
use CodeIgniter\Cookie\CookieStore;
1819
use CodeIgniter\Cookie\Exceptions\CookieException;
@@ -21,6 +22,8 @@
2122
use CodeIgniter\I18n\Time;
2223
use CodeIgniter\Pager\PagerInterface;
2324
use CodeIgniter\Security\Exceptions\SecurityException;
25+
use Config\App;
26+
use Config\ContentSecurityPolicy as ContentSecurityPolicyConfig;
2427
use Config\Cookie as CookieConfig;
2528
use DateTime;
2629
use DateTimeZone;
@@ -31,21 +34,30 @@
3134
* Additional methods to make a PSR-7 Response class
3235
* compliant with the framework's own ResponseInterface.
3336
*
37+
* @property array<int, string> $statusCodes
38+
* @property string|null $body
39+
*
3440
* @see https://github.com/php-fig/http-message/blob/master/src/ResponseInterface.php
3541
*/
3642
trait ResponseTrait
3743
{
3844
/**
39-
* Content security policy handler
45+
* Content security policy handler.
46+
*
47+
* Lazily instantiated on first use via `self::getCSP()` so that the
48+
* ContentSecurityPolicy class is not loaded on requests that do not use CSP.
4049
*
41-
* @var ContentSecurityPolicy
50+
* @var ContentSecurityPolicy|null
4251
*/
4352
protected $CSP;
4453

4554
/**
4655
* CookieStore instance.
4756
*
48-
* @var CookieStore
57+
* Lazily instantiated on first cookie-related call so that the Cookie and
58+
* CookieStore classes are not loaded on requests that do not use cookies.
59+
*
60+
* @var CookieStore|null
4961
*/
5062
protected $cookieStore;
5163

@@ -77,19 +89,17 @@ trait ResponseTrait
7789
*/
7890
public function setStatusCode(int $code, string $reason = '')
7991
{
80-
// Valid range?
8192
if ($code < 100 || $code > 599) {
8293
throw HTTPException::forInvalidStatusCode($code);
8394
}
8495

85-
// Unknown and no message?
86-
if (! array_key_exists($code, static::$statusCodes) && ($reason === '')) {
96+
if (! array_key_exists($code, static::$statusCodes) && $reason === '') {
8797
throw HTTPException::forUnkownStatusCode($code);
8898
}
8999

90100
$this->statusCode = $code;
91101

92-
$this->reason = ($reason !== '') ? $reason : static::$statusCodes[$code];
102+
$this->reason = $reason !== '' ? $reason : static::$statusCodes[$code];
93103

94104
return $this;
95105
}
@@ -366,8 +376,10 @@ public function setLastModified($date)
366376
public function send()
367377
{
368378
// If we're enforcing a Content Security Policy,
369-
// we need to give it a chance to build out it's headers.
370-
$this->CSP->finalize($this);
379+
// we need to give it a chance to build out its headers.
380+
if ($this->shouldFinalizeCsp()) {
381+
$this->getCSP()->finalize($this);
382+
}
371383

372384
$this->sendHeaders();
373385
$this->sendCookies();
@@ -376,6 +388,44 @@ public function send()
376388
return $this;
377389
}
378390

391+
/**
392+
* Decides whether {@see ContentSecurityPolicy::finalize()} should run for
393+
* this response. Keeping the CSP class unloaded on requests that do not
394+
* need it avoids the cost of constructing a 1000+ line service on every
395+
* request.
396+
*/
397+
private function shouldFinalizeCsp(): bool
398+
{
399+
// Developer already touched CSP through getCSP(); respect it.
400+
if ($this->CSP !== null) {
401+
return true;
402+
}
403+
404+
// A CSP instance has been registered (e.g., via Services::injectMock()
405+
// or any earlier service('csp') call) — reuse it instead of skipping.
406+
if (Services::has('csp')) {
407+
return true;
408+
}
409+
410+
if (config(App::class)->CSPEnabled) {
411+
return true;
412+
}
413+
414+
// Placeholders in the body still need to be stripped even when CSP
415+
// is disabled, so the body is scanned for the configured nonce tags
416+
// before committing to loading the full CSP class.
417+
$body = (string) $this->body;
418+
419+
if ($body === '') {
420+
return false;
421+
}
422+
423+
$cspConfig = config(ContentSecurityPolicyConfig::class);
424+
425+
return str_contains($body, $cspConfig->scriptNonceTag)
426+
|| str_contains($body, $cspConfig->styleNonceTag);
427+
}
428+
379429
/**
380430
* Sends the headers of this HTTP response to the browser.
381431
*
@@ -518,8 +568,10 @@ public function setCookie(
518568
$httponly = null,
519569
$samesite = null,
520570
) {
571+
$store = $this->getCookieStore();
572+
521573
if ($name instanceof Cookie) {
522-
$this->cookieStore = $this->cookieStore->put($name);
574+
$this->cookieStore = $store->put($name);
523575

524576
return $this;
525577
}
@@ -553,18 +605,23 @@ public function setCookie(
553605
'samesite' => $samesite ?? '',
554606
]);
555607

556-
$this->cookieStore = $this->cookieStore->put($cookie);
608+
$this->cookieStore = $store->put($cookie);
557609

558610
return $this;
559611
}
560612

561613
/**
562614
* Returns the `CookieStore` instance.
563615
*
616+
* Lazily instantiates the `CookieStore` on first call, so that the Cookie and
617+
* CookieStore classes are not loaded on requests that do not use cookies.
618+
*
564619
* @return CookieStore
565620
*/
566621
public function getCookieStore()
567622
{
623+
$this->cookieStore ??= new CookieStore([]);
624+
568625
return $this->cookieStore;
569626
}
570627

@@ -573,9 +630,10 @@ public function getCookieStore()
573630
*/
574631
public function hasCookie(string $name, ?string $value = null, string $prefix = ''): bool
575632
{
633+
$store = $this->getCookieStore();
576634
$prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC
577635

578-
return $this->cookieStore->has($name, $prefix, $value);
636+
return $store->has($name, $prefix, $value);
579637
}
580638

581639
/**
@@ -588,14 +646,16 @@ public function hasCookie(string $name, ?string $value = null, string $prefix =
588646
*/
589647
public function getCookie(?string $name = null, string $prefix = '')
590648
{
649+
$store = $this->getCookieStore();
650+
591651
if ((string) $name === '') {
592-
return $this->cookieStore->display();
652+
return $store->display();
593653
}
594654

595655
try {
596656
$prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC
597657

598-
return $this->cookieStore->get($name, $prefix);
658+
return $store->get($name, $prefix);
599659
} catch (CookieException $e) {
600660
log_message('error', (string) $e);
601661

@@ -614,10 +674,10 @@ public function deleteCookie(string $name = '', string $domain = '', string $pat
614674
return $this;
615675
}
616676

677+
$store = $this->getCookieStore();
617678
$prefix = $prefix !== '' ? $prefix : Cookie::setDefaults()['prefix']; // to retain BC
618679

619680
$prefixed = $prefix . $name;
620-
$store = $this->cookieStore;
621681
$found = false;
622682

623683
/** @var Cookie $cookie */
@@ -653,6 +713,10 @@ public function deleteCookie(string $name = '', string $domain = '', string $pat
653713
*/
654714
public function getCookies()
655715
{
716+
if ($this->cookieStore === null) {
717+
return [];
718+
}
719+
656720
return $this->cookieStore->display();
657721
}
658722

@@ -663,7 +727,7 @@ public function getCookies()
663727
*/
664728
protected function sendCookies()
665729
{
666-
if ($this->pretend) {
730+
if ($this->pretend || $this->cookieStore === null) {
667731
return;
668732
}
669733

@@ -753,6 +817,8 @@ public function download(string $filename = '', $data = '', bool $setMime = fals
753817

754818
public function getCSP(): ContentSecurityPolicy
755819
{
820+
$this->CSP ??= service('csp');
821+
756822
return $this->CSP;
757823
}
758824
}

0 commit comments

Comments
 (0)